From: Tycho Andersen <tycho@tycho.pizza>
To: Palmer Dabbelt <palmer@dabbelt.com>
Cc: Christoph Hellwig <hch@infradead.org>,
schwab@suse.de, david.abdurachmanov@sifive.com,
linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org,
Paul Walmsley <paul.walmsley@sifive.com>
Subject: Re: [PATCH] riscv: return -ENOSYS for syscall -1
Date: Wed, 23 Dec 2020 21:38:06 -0700 [thread overview]
Message-ID: <20201224043806.GE710211@cisco> (raw)
In-Reply-To: <mhng-c15e4fab-6487-4875-a392-85669982b8b1@palmerdabbelt-glaptop>
On Wed, Dec 23, 2020 at 06:54:43PM -0800, Palmer Dabbelt wrote:
> 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?
Yes, agreed. My musing was mostly just that this is dead code that
probably should have been in removed in af33d2433b03 ("riscv: fix
seccomp reject syscall code path"), but was overlooked. Maybe this
could use a Fixes: tag for that too.
Tycho
prev parent reply other threads:[~2020-12-24 4:44 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-21 22:52 [PATCH] riscv: return -ENOSYS for syscall -1 Andreas Schwab
2020-12-22 16:22 ` Tycho Andersen
2020-12-23 4:04 ` Palmer Dabbelt
2020-12-23 8:24 ` Christoph Hellwig
2020-12-24 2:54 ` Palmer Dabbelt
2020-12-24 4:38 ` Tycho Andersen [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201224043806.GE710211@cisco \
--to=tycho@tycho.pizza \
--cc=david.abdurachmanov@sifive.com \
--cc=hch@infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=palmer@dabbelt.com \
--cc=paul.walmsley@sifive.com \
--cc=schwab@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).