Messages in this thread | | | Date | Wed, 23 Dec 2020 18:54:43 -0800 (PST) | Subject | Re: [PATCH] riscv: return -ENOSYS for syscall -1 | From | Palmer Dabbelt <> |
| |
On Wed, 23 Dec 2020 00:24:04 PST (-0800), Christoph Hellwig wrote: > On Tue, Dec 22, 2020 at 09:22:19AM -0700, Tycho Andersen wrote: >> On Mon, Dec 21, 2020 at 11:52:00PM +0100, Andreas Schwab wrote: >> > Properly return -ENOSYS for syscall -1 instead of leaving the return value >> > uninitialized. This fixes the strace teststuite. >> > >> > Fixes: 5340627e3fe0 ("riscv: add support for SECCOMP and SECCOMP_FILTER") >> > Signed-off-by: Andreas Schwab <schwab@suse.de> >> > --- >> > arch/riscv/kernel/entry.S | 9 +-------- >> > 1 file changed, 1 insertion(+), 8 deletions(-) >> > >> > diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S >> > index 524d918f3601..d07763001eb0 100644 >> > --- a/arch/riscv/kernel/entry.S >> > +++ b/arch/riscv/kernel/entry.S >> > @@ -186,14 +186,7 @@ check_syscall_nr: >> > * Syscall number held in a7. >> > * If syscall number is above allowed value, redirect to ni_syscall. >> > */ >> > - bge a7, t0, 1f >> > - /* >> > - * Check if syscall is rejected by tracer, i.e., a7 == -1. >> > - * If yes, we pretend it was executed. >> > - */ >> > - li t1, -1 >> > - beq a7, t1, ret_from_syscall_rejected >> > - blt a7, t1, 1f >> > + bgeu a7, t0, 1f >> >> IIUC, this is all dead code anyway for the path where seccomp actually >> rejects the syscall, since it should do the rejection directly in >> handle_syscall_trace_enter(), which is called above this hunk. So it >> seems good to me. > > That change really needs to be documented in the commit log, or even > better split into a separate patch (still documented of course!).
Unless I'm missing something, this is already how it works already? handle_syscall_trace_enter is checking the result of do_syscall_trace_enter(), which checks secure_computing(). When secure_computing() rejects the syscall we already ended up rejecting the syscall, so this code wasn't doing anything for the case it was supposed to handle.
It was, however, intercepting syscall number -1 when we weren't rejecting the syscall and directly exiting rather than calling sys_ni_syscall. That would, at a bare minimum, result in an uninitialized return value. It also breaks the pairing of trace_sys_enter() and trace_sys_exit(), which doesn't smell like a good idea.
| |