Message ID | 90edd33b-6353-1228-791f-0336d94d5f8c@majoroak.me.uk (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | arm64: ptrace: Fix seccomp of traced syscall -1 (NO_SYSCALL) | expand |
On Mon, Jan 18, 2021 at 02:58:58AM +0000, Timothy Baldwin wrote: > From c047f549699d31ed91d5ac0cadbcf76a02cd801e Mon Sep 17 00:00:00 2001 > From: Timothy E Baldwin<T.E.Baldwin99@members.leeds.ac.uk> > Date: Sat, 16 Jan 2021 15:18:54 +0000 > Subject: [PATCH] arm64: ptrace: Fix seccomp of traced syscall -1 (NO_SYSCALL) > > Since commit f086f67485c5 ("arm64: ptrace: add support for syscall > emulation"), if system call number -1 is called and the process is being > traced with PTRACE_SYSCALL, for example by strace, the seccomp check is > skipped and -ENOSYS is returned unconditionally (unless altered by the > tracer) rather than carrying out action specified in the seccomp filter. > > The consequence of this is that it is not possible to reliably strace > a seccomp based implementation of a foreign system call interface in > which r7/x8 is permitted to be -1 on entry to a system call. > > Also trace_sys_enter and audit_syscall_entry are skipped if a system > call is skipped. > > Fix by removing the in_syscall(regs) check restoring the previous behaviour > which is like AArch32, x86 (which uses generic code) and everything else. > Ah, my fault. At the time of timing this I didn't test with seccomp and also for some reason IIRC I had assumed the flags SYSCALL_{EMU,TRACE} and seccomp calls are mutually exclusive and can't happen together. FWIW, Reviewed-by: Sudeep Holla <sudeep.holla@arm.com> Also I ran some minimal tests I have, so Tested-by: Sudeep Holla <sudeep.holla@arm.com> I have also asked Haibo Xu <Haibo.Xu@arm.com> who help me testing back then to test again. > Fixes: f086f67485c5 ("arm64: ptrace: add support for syscall emulation") > Signed-off-by: Timothy E Baldwin<T.E.Baldwin99@members.leeds.ac.uk> > Cc: Sudeep Holla<sudeep.holla@arm.com> > Cc: Oleg Nesterov <oleg@redhat.com> > Cc: Catalin Marinas<catalin.marinas@arm.com> > Cc: Will Deacon<will.deacon@arm.com> > Cc: Kees Cook<keescook@chromium.org> > Cc:stable@vger.kernel.org > --- > arch/arm64/kernel/ptrace.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c > index 8ac487c84e37..1d75471979cb 100644 > --- a/arch/arm64/kernel/ptrace.c > +++ b/arch/arm64/kernel/ptrace.c > @@ -1796,7 +1796,7 @@ 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)) > + if (flags & _TIF_SYSCALL_EMU) > return NO_SYSCALL; > } > -- > 2.27.0 > > The specific implementation of a seccomp based foreign system call interface > is my port of RISC OS to Linux, in the spirit User Mode Linux: > https://github.com/TimothyEBaldwin/RISC_OS_Linux_Binary > > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Wed, Feb 24, 2021 at 02:49:20PM +0000, Sudeep Holla wrote: > On Mon, Jan 18, 2021 at 02:58:58AM +0000, Timothy Baldwin wrote: > > From c047f549699d31ed91d5ac0cadbcf76a02cd801e Mon Sep 17 00:00:00 2001 > > From: Timothy E Baldwin<T.E.Baldwin99@members.leeds.ac.uk> > > Date: Sat, 16 Jan 2021 15:18:54 +0000 > > Subject: [PATCH] arm64: ptrace: Fix seccomp of traced syscall -1 (NO_SYSCALL) > > > > Since commit f086f67485c5 ("arm64: ptrace: add support for syscall > > emulation"), if system call number -1 is called and the process is being > > traced with PTRACE_SYSCALL, for example by strace, the seccomp check is > > skipped and -ENOSYS is returned unconditionally (unless altered by the > > tracer) rather than carrying out action specified in the seccomp filter. > > > > The consequence of this is that it is not possible to reliably strace > > a seccomp based implementation of a foreign system call interface in > > which r7/x8 is permitted to be -1 on entry to a system call. > > > > Also trace_sys_enter and audit_syscall_entry are skipped if a system > > call is skipped. > > > > Fix by removing the in_syscall(regs) check restoring the previous behaviour > > which is like AArch32, x86 (which uses generic code) and everything else. > > > > Ah, my fault. At the time of timing this I didn't test with seccomp and > also for some reason IIRC I had assumed the flags SYSCALL_{EMU,TRACE} > and seccomp calls are mutually exclusive and can't happen together. > > FWIW, > Reviewed-by: Sudeep Holla <sudeep.holla@arm.com> > > Also I ran some minimal tests I have, so > Tested-by: Sudeep Holla <sudeep.holla@arm.com> > > I have also asked Haibo Xu <Haibo.Xu@arm.com> who help me testing back then > to test again. Thanks for catching and fixing this! Does this pass the seccomp selftests? Reviewed-by: Kees Cook <keescook@chromium.org> Will, do you want to take this? I don't usually put the arch-specific seccomp bits through the seccomp tree. -Kees > > > > Fixes: f086f67485c5 ("arm64: ptrace: add support for syscall emulation") > > Signed-off-by: Timothy E Baldwin<T.E.Baldwin99@members.leeds.ac.uk> > > Cc: Sudeep Holla<sudeep.holla@arm.com> > > Cc: Oleg Nesterov <oleg@redhat.com> > > Cc: Catalin Marinas<catalin.marinas@arm.com> > > Cc: Will Deacon<will.deacon@arm.com> > > Cc: Kees Cook<keescook@chromium.org> > > Cc:stable@vger.kernel.org > > --- > > arch/arm64/kernel/ptrace.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c > > index 8ac487c84e37..1d75471979cb 100644 > > --- a/arch/arm64/kernel/ptrace.c > > +++ b/arch/arm64/kernel/ptrace.c > > @@ -1796,7 +1796,7 @@ 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)) > > + if (flags & _TIF_SYSCALL_EMU) > > return NO_SYSCALL; > > } > > -- > > 2.27.0 > > > > The specific implementation of a seccomp based foreign system call interface > > is my port of RISC OS to Linux, in the spirit User Mode Linux: > > https://github.com/TimothyEBaldwin/RISC_OS_Linux_Binary > > > > > > > > _______________________________________________ > > linux-arm-kernel mailing list > > linux-arm-kernel@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > > -- > Regards, > Sudeep
On Wed, Feb 24, 2021 at 02:48:05PM -0800, Kees Cook wrote: > On Wed, Feb 24, 2021 at 02:49:20PM +0000, Sudeep Holla wrote: > > On Mon, Jan 18, 2021 at 02:58:58AM +0000, Timothy Baldwin wrote: > > > From c047f549699d31ed91d5ac0cadbcf76a02cd801e Mon Sep 17 00:00:00 2001 > > > From: Timothy E Baldwin<T.E.Baldwin99@members.leeds.ac.uk> > > > Date: Sat, 16 Jan 2021 15:18:54 +0000 > > > Subject: [PATCH] arm64: ptrace: Fix seccomp of traced syscall -1 (NO_SYSCALL) > > > > > > Since commit f086f67485c5 ("arm64: ptrace: add support for syscall > > > emulation"), if system call number -1 is called and the process is being > > > traced with PTRACE_SYSCALL, for example by strace, the seccomp check is > > > skipped and -ENOSYS is returned unconditionally (unless altered by the > > > tracer) rather than carrying out action specified in the seccomp filter. > > > > > > The consequence of this is that it is not possible to reliably strace > > > a seccomp based implementation of a foreign system call interface in > > > which r7/x8 is permitted to be -1 on entry to a system call. > > > > > > Also trace_sys_enter and audit_syscall_entry are skipped if a system > > > call is skipped. > > > > > > Fix by removing the in_syscall(regs) check restoring the previous behaviour > > > which is like AArch32, x86 (which uses generic code) and everything else. > > > > > > > Ah, my fault. At the time of timing this I didn't test with seccomp and > > also for some reason IIRC I had assumed the flags SYSCALL_{EMU,TRACE} > > and seccomp calls are mutually exclusive and can't happen together. > > > > FWIW, > > Reviewed-by: Sudeep Holla <sudeep.holla@arm.com> > > > > Also I ran some minimal tests I have, so > > Tested-by: Sudeep Holla <sudeep.holla@arm.com> > > > > I have also asked Haibo Xu <Haibo.Xu@arm.com> who help me testing back then > > to test again. > > Thanks for catching and fixing this! Does this pass the seccomp selftests? > > Reviewed-by: Kees Cook <keescook@chromium.org> > > Will, do you want to take this? I don't usually put the arch-specific > seccomp bits through the seccomp tree. Sure, I'll add it to the fixes pile. Thanks! Will
On Mon, 18 Jan 2021 02:58:58 +0000, Timothy Baldwin wrote: > From c047f549699d31ed91d5ac0cadbcf76a02cd801e Mon Sep 17 00:00:00 2001 > From: Timothy E Baldwin<T.E.Baldwin99@members.leeds.ac.uk> > Date: Sat, 16 Jan 2021 15:18:54 +0000 > Subject: [PATCH] arm64: ptrace: Fix seccomp of traced syscall -1 (NO_SYSCALL) > > Since commit f086f67485c5 ("arm64: ptrace: add support for syscall > emulation"), if system call number -1 is called and the process is being > traced with PTRACE_SYSCALL, for example by strace, the seccomp check is > skipped and -ENOSYS is returned unconditionally (unless altered by the > tracer) rather than carrying out action specified in the seccomp filter. > > [...] (Note that I had to fix up whitespace errors in order for git am to apply this, so please check the resulting commit). Applied to arm64 (for-next/fixes), thanks! [1/1] arm64: ptrace: Fix seccomp of traced syscall -1 (NO_SYSCALL) https://git.kernel.org/arm64/c/df84fe947089 Cheers,
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c index 8ac487c84e37..1d75471979cb 100644 --- a/arch/arm64/kernel/ptrace.c +++ b/arch/arm64/kernel/ptrace.c @@ -1796,7 +1796,7 @@ 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)) + if (flags & _TIF_SYSCALL_EMU) return NO_SYSCALL; }