I must have missed something, but I simply can't undestand it. static inline long syscall_get_error(struct task_struct *task, struct pt_regs *regs) { unsigned long error = regs->ax; #ifdef CONFIG_IA32_EMULATION /* * TS_COMPAT is set for 32-bit syscall entries and then * remains set until we return to user mode. */ if (task->thread.status & (TS_COMPAT|TS_I386_REGS_POKED)) /* * Sign-extend the value so (int)-EFOO becomes (long)-EFOO * and will match correctly in comparisons. */ error = (long) (int) error; #endif return IS_ERR_VALUE(error) ? error : 0; }
Firstly, why do we need the IS_ERR_VALUE() check? This is only used by do_signal/handle_signal, we do not care if it returns non-zero as long as the value can't be confused with -ERESTART.* codes. And why do we need the TS_ checks? IIUC only because a 32-bit debugger can change regs->ax, and we also assume that if this happens outside of syscall-exit path (so that TS_COMPAT is not set) then it should also change regs->orig_ax and this implies TS_I386_REGS_POKED. Oherwise it is not needed, even the 32-bit syscalls return long, not int. So why we can't simply change putreg32() to always sign-extend regs->ax regs->orig_ax and just do static inline long syscall_get_error(struct task_struct *task, struct pt_regs *regs) { return regs-ax; } ? Or, better, simply kill it and use syscall_get_return_value() in arch/x86/kernel/signal.c. Of course, if the tracee is 64-bit and debugger is 32-bit then the unconditional sign-extend can be wrong, but do we really care about this case? This can't really work anyway. And the current code is not right too. Say, debugger nacks the signal which interrupted syscall and sets regs->ax = -ERESTARTSYS to force the restart, this won't work because TS_COMPAT|TS_I386_REGS_POKED are not set. Oleg.