| Message ID | 20200710130702.30658-7-will@kernel.org (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | d83ee6e3e75db6f518ef2b0858f163849f2ddeb7 |
| Headers | show |
| Series | arm64: Fix single-step handling and syscall tracing | expand |
On Fri, Jul 10, 2020 at 02:07:01PM +0100, Will Deacon wrote: > Setting a system call number of -1 is special, as it indicates that the > current system call should be skipped. > > Use NO_SYSCALL instead of -1 when checking for this scenario, which is > different from the -1 returned due to a seccomp failure. I can't understand this paragraph. NO_SYSCALL is -1, so how is this "different"? arch/arm64/include/asm/ptrace.h:#define NO_SYSCALL (-1) Do you just mean "stop using a literal '-1'"? -Kees > > Cc: Mark Rutland <mark.rutland@arm.com> > Cc: Keno Fischer <keno@juliacomputing.com> > Cc: Luis Machado <luis.machado@linaro.org> > Signed-off-by: Will Deacon <will@kernel.org> > --- > arch/arm64/kernel/ptrace.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c > index 89fbee3991a2..1e02e98e68dd 100644 > --- a/arch/arm64/kernel/ptrace.c > +++ b/arch/arm64/kernel/ptrace.c > @@ -1856,12 +1856,12 @@ int syscall_trace_enter(struct pt_regs *regs) > if (flags & (_TIF_SYSCALL_EMU | _TIF_SYSCALL_TRACE)) { > tracehook_report_syscall(regs, PTRACE_SYSCALL_ENTER); > if (!in_syscall(regs) || (flags & _TIF_SYSCALL_EMU)) > - return -1; > + return NO_SYSCALL; > } > > /* Do the secure computing after ptrace; failures should be fast. */ > if (secure_computing() == -1) > - return -1; > + return NO_SYSCALL; > > if (test_thread_flag(TIF_SYSCALL_TRACEPOINT)) > trace_sys_enter(regs, regs->syscallno); > -- > 2.27.0.383.g050319c2ae-goog >
On Fri, Jul 10, 2020 at 09:04:08AM -0700, Kees Cook wrote: > On Fri, Jul 10, 2020 at 02:07:01PM +0100, Will Deacon wrote: > > Setting a system call number of -1 is special, as it indicates that the > > current system call should be skipped. > > > > Use NO_SYSCALL instead of -1 when checking for this scenario, which is > > different from the -1 returned due to a seccomp failure. > > I can't understand this paragraph. NO_SYSCALL is -1, so how is this > "different"? > > arch/arm64/include/asm/ptrace.h:#define NO_SYSCALL (-1) > > Do you just mean "stop using a literal '-1'"? Yes, I'm trying to distinguish '-1' when used as a system call number from '-1' when used as the return value of secure_computing() on failure. It's currently all mixed up, and I think it's confusing trying to realise what is a system call number and what is an error code. Will
On Fri, Jul 10, 2020 at 05:11:48PM +0100, Will Deacon wrote: > On Fri, Jul 10, 2020 at 09:04:08AM -0700, Kees Cook wrote: > > On Fri, Jul 10, 2020 at 02:07:01PM +0100, Will Deacon wrote: > > > Setting a system call number of -1 is special, as it indicates that the > > > current system call should be skipped. > > > > > > Use NO_SYSCALL instead of -1 when checking for this scenario, which is > > > different from the -1 returned due to a seccomp failure. > > > > I can't understand this paragraph. NO_SYSCALL is -1, so how is this > > "different"? > > > > arch/arm64/include/asm/ptrace.h:#define NO_SYSCALL (-1) > > > > Do you just mean "stop using a literal '-1'"? > > Yes, I'm trying to distinguish '-1' when used as a system call number > from '-1' when used as the return value of secure_computing() on failure. > It's currently all mixed up, and I think it's confusing trying to realise > what is a system call number and what is an error code. Okay, I gotcha now. Yes, that's entirely reasonable. (It's perhaps an artifact of x86's implementation that they get directly passed through.)
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c index 89fbee3991a2..1e02e98e68dd 100644 --- a/arch/arm64/kernel/ptrace.c +++ b/arch/arm64/kernel/ptrace.c @@ -1856,12 +1856,12 @@ int syscall_trace_enter(struct pt_regs *regs) if (flags & (_TIF_SYSCALL_EMU | _TIF_SYSCALL_TRACE)) { tracehook_report_syscall(regs, PTRACE_SYSCALL_ENTER); if (!in_syscall(regs) || (flags & _TIF_SYSCALL_EMU)) - return -1; + return NO_SYSCALL; } /* Do the secure computing after ptrace; failures should be fast. */ if (secure_computing() == -1) - return -1; + return NO_SYSCALL; if (test_thread_flag(TIF_SYSCALL_TRACEPOINT)) trace_sys_enter(regs, regs->syscallno);
Setting a system call number of -1 is special, as it indicates that the current system call should be skipped. Use NO_SYSCALL instead of -1 when checking for this scenario, which is different from the -1 returned due to a seccomp failure. Cc: Mark Rutland <mark.rutland@arm.com> Cc: Keno Fischer <keno@juliacomputing.com> Cc: Luis Machado <luis.machado@linaro.org> Signed-off-by: Will Deacon <will@kernel.org> --- arch/arm64/kernel/ptrace.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)