* [PATCH] riscv: Remove unnecessary path for syscall_trace
@ 2020-05-25 14:18 guoren
2020-05-25 14:36 ` Tycho Andersen
0 siblings, 1 reply; 4+ messages in thread
From: guoren @ 2020-05-25 14:18 UTC (permalink / raw)
To: tycho, keescook, palmerdabbelt, paul.walmsley, anup
Cc: linux-riscv, linux-kernel, Guo Ren
From: Guo Ren <guoren@linux.alibaba.com>
Obviously, there is no need to recover a0-a7 in reject path.
Previous modification is from commit af33d243 by Tycho, to
fixup seccomp reject syscall code path.
Signed-off-by: Guo Ren <guoren@linux.alibaba.com>
Cc: Tycho Andersen <tycho@tycho.ws>
Cc: Kees Cook <keescook@chromium.org>
Cc: Palmer Dabbelt <palmerdabbelt@google.com>
---
arch/riscv/kernel/entry.S | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
index 56d071b..ea3444d 100644
--- a/arch/riscv/kernel/entry.S
+++ b/arch/riscv/kernel/entry.S
@@ -306,7 +306,7 @@ work_resched:
handle_syscall_trace_enter:
move a0, sp
call do_syscall_trace_enter
- move t0, a0
+ bnez a0, ret_from_syscall_rejected
REG_L a0, PT_A0(sp)
REG_L a1, PT_A1(sp)
REG_L a2, PT_A2(sp)
@@ -315,7 +315,6 @@ handle_syscall_trace_enter:
REG_L a5, PT_A5(sp)
REG_L a6, PT_A6(sp)
REG_L a7, PT_A7(sp)
- bnez t0, ret_from_syscall_rejected
j check_syscall_nr
handle_syscall_trace_exit:
move a0, sp
--
2.7.4
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] riscv: Remove unnecessary path for syscall_trace
2020-05-25 14:18 [PATCH] riscv: Remove unnecessary path for syscall_trace guoren
@ 2020-05-25 14:36 ` Tycho Andersen
2020-05-26 0:29 ` Guo Ren
0 siblings, 1 reply; 4+ messages in thread
From: Tycho Andersen @ 2020-05-25 14:36 UTC (permalink / raw)
To: guoren
Cc: keescook, palmerdabbelt, paul.walmsley, anup, linux-riscv,
linux-kernel, Guo Ren
On Mon, May 25, 2020 at 02:18:26PM +0000, guoren@kernel.org wrote:
> From: Guo Ren <guoren@linux.alibaba.com>
>
> Obviously, there is no need to recover a0-a7 in reject path.
>
> Previous modification is from commit af33d243 by Tycho, to
> fixup seccomp reject syscall code path.
Doesn't this suffer from the same problem, though? a7 is clobbered, so
the -ERESTARTSYS behavior won't work?
I haven't run the test case that was failing before.
Tycho
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] riscv: Remove unnecessary path for syscall_trace
2020-05-25 14:36 ` Tycho Andersen
@ 2020-05-26 0:29 ` Guo Ren
2020-05-26 13:38 ` Tycho Andersen
0 siblings, 1 reply; 4+ messages in thread
From: Guo Ren @ 2020-05-26 0:29 UTC (permalink / raw)
To: Tycho Andersen
Cc: keescook, palmerdabbelt, Paul Walmsley, Anup Patel, linux-riscv,
Linux Kernel Mailing List, Guo Ren
Hi Tycho,
On Mon, May 25, 2020 at 10:36 PM Tycho Andersen <tycho@tycho.ws> wrote:
>
> On Mon, May 25, 2020 at 02:18:26PM +0000, guoren@kernel.org wrote:
> > From: Guo Ren <guoren@linux.alibaba.com>
> >
> > Obviously, there is no need to recover a0-a7 in reject path.
> >
> > Previous modification is from commit af33d243 by Tycho, to
> > fixup seccomp reject syscall code path.
>
> Doesn't this suffer from the same problem, though? a7 is clobbered, so
> the -ERESTARTSYS behavior won't work?
Look, the patch only affects the path of ret_from_syscall_rejected,
and there are two possible paths:
1. ret_from_syscall_rejected->handle_syscall_trace_exit->ret_from_exception
2. ret_from_syscall_rejected->ret_from_exception
All the above skip the check_syscall_nr and ignore the current a7, in
the C function they use the pt_regs in the stack to get proper reg's
value.
For the -ERESTARTSYS, we only process it in:
ret_from_exception->resume_userspace->work_notifysig->do_notify_resume:
do_signal & handle_signal:
switch (regs->a0) {
case -ERESTARTNOHAND:
case -ERESTARTSYS:
case -ERESTARTNOINTR:
regs->a0 = regs->orig_a0;
regs->epc -= 0x4;
break;
All above are done in pt_regs and when returning to userspace, a7 will
be recovered by restore_all in entry.S.
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
--
Best Regards
Guo Ren
ML: https://lore.kernel.org/linux-csky/
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] riscv: Remove unnecessary path for syscall_trace
2020-05-26 0:29 ` Guo Ren
@ 2020-05-26 13:38 ` Tycho Andersen
0 siblings, 0 replies; 4+ messages in thread
From: Tycho Andersen @ 2020-05-26 13:38 UTC (permalink / raw)
To: Guo Ren
Cc: keescook, palmerdabbelt, Paul Walmsley, Anup Patel, linux-riscv,
Linux Kernel Mailing List, Guo Ren
On Tue, May 26, 2020 at 08:29:45AM +0800, Guo Ren wrote:
> Hi Tycho,
>
> On Mon, May 25, 2020 at 10:36 PM Tycho Andersen <tycho@tycho.ws> wrote:
> >
> > On Mon, May 25, 2020 at 02:18:26PM +0000, guoren@kernel.org wrote:
> > > From: Guo Ren <guoren@linux.alibaba.com>
> > >
> > > Obviously, there is no need to recover a0-a7 in reject path.
> > >
> > > Previous modification is from commit af33d243 by Tycho, to
> > > fixup seccomp reject syscall code path.
> >
> > Doesn't this suffer from the same problem, though? a7 is clobbered, so
> > the -ERESTARTSYS behavior won't work?
>
> Look, the patch only affects the path of ret_from_syscall_rejected,
> and there are two possible paths:
> 1. ret_from_syscall_rejected->handle_syscall_trace_exit->ret_from_exception
> 2. ret_from_syscall_rejected->ret_from_exception
>
> All the above skip the check_syscall_nr and ignore the current a7, in
> the C function they use the pt_regs in the stack to get proper reg's
> value.
>
> For the -ERESTARTSYS, we only process it in:
> ret_from_exception->resume_userspace->work_notifysig->do_notify_resume:
> do_signal & handle_signal:
>
> switch (regs->a0) {
> case -ERESTARTNOHAND:
> case -ERESTARTSYS:
> case -ERESTARTNOINTR:
> regs->a0 = regs->orig_a0;
> regs->epc -= 0x4;
> break;
>
> All above are done in pt_regs and when returning to userspace, a7 will
> be recovered by restore_all in entry.S.
Yes, thanks for that explanation.
Reviewed-by: Tycho Andersen <tycho@tycho.ws>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-05-26 13:38 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-25 14:18 [PATCH] riscv: Remove unnecessary path for syscall_trace guoren
2020-05-25 14:36 ` Tycho Andersen
2020-05-26 0:29 ` Guo Ren
2020-05-26 13:38 ` Tycho Andersen
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).