Messages in this thread | | | Subject | Re: [PATCH 2/2] ptrace: is_syscall_success: Add syscall return code handling for compat task | From | He Zhe <> | Date | Thu, 15 Apr 2021 13:12:16 +0800 |
| |
On 4/15/21 12:55 AM, Oleg Nesterov wrote: > On 04/14, David Laight wrote: >> From: Oleg Nesterov >>> Sent: 14 April 2021 16:08 >>> >>> Add audit maintainers... >>> >>> On 04/14, He Zhe wrote: >>>> When 32-bit userspace application is running on 64-bit kernel, the 32-bit >>>> syscall return code would be changed from u32 to u64 in regs_return_value >>>> and then changed to s64. Hence the negative return code would be treated >>>> as a positive number and results in a non-error in, for example, audit >>>> like below. >>> Sorry, can understand. At least on x86_64 even the 32-bit syscall returns >>> long, not u32. >>> >>> Hmm. And afaics on x86 is_compat_task() is only defined if !CONFIG_COMPAT, >>> so this patch looks wrong anyway. >> And, as with the other patch a x64_64 64bit process can make both types >> of 32bit system call - so it needs to depend on the system call entry type >> not any type of the task. > I don't understand... but iirc is_compat_task() used to check TS_COMPAT and > this is what we need to detect the 32-bit syscall. But it looks deprecated, > I think in_compat_syscall() should be used instead. > > But this doesn't matter, I still can't understand the problem.
Sorry for not enough clarification.
This was found on an arm64 kernel running with 32-bit user-space application. The arm64 version of regs_return_value returns unsigned long.
static inline unsigned long regs_return_value(struct pt_regs *regs) { return regs->regs[0]; }
But when the syscall fails, with -13 in my case, the return code has been saved as a 32 bit long negative number, 0x00000000FFFFFFF3, in regs[0] by the time regs_return_value gets called in audit_syscall_exit.
Then in audit_syscall_exit, the return value of regs_return_value is changed to a 64 bit signed long, from when on it is treated as a positive number.
Similarly in is_syscall_success, 0x00000000FFFFFFF3 would be out of error number range, resulting in a "success".
These two patches are to do the sign extension.
David, thanks, is_compat_syscall should be the right one to use. I didn't notice the difference between is_compat_syscall and is_compat_task and thought is_compat_task would be harmless to other architectures.
Zhe
> > Oleg. >
| |