Re: [PATCH] arm64: ptrace: Fix seccomp of traced syscall -1 (NO_SYSCALL)

[Kees Cook <keescook@chromium.org>]

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

-- 
Kees Cook

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel