* [PATCH 0/2] riscv: add support for SECCOMP and SECCOMP_FILTER @ 2018-12-06 15:01 David Abdurachmanov 2018-12-06 15:01 ` [PATCH 1/2] riscv: add support for SECCOMP incl. filters David Abdurachmanov 2018-12-06 15:01 ` [PATCH 2/2] riscv: fix syscall_{get,set}_arguments David Abdurachmanov 0 siblings, 2 replies; 13+ messages in thread From: David Abdurachmanov @ 2018-12-06 15:01 UTC (permalink / raw) To: palmer, aou, keescook, luto, wad, green.hu, deanbo422, linux-kernel, linux-riscv Cc: David Abdurachmanov This was originally tested on 4.19 kernel with Fedora 29/RISCV. Depends on audit patches (already in linux-next). The patches are on top of linux-next next-20181206 tag. Validation was done using libseccomp (at 1e64feb5f1a9ea02687228e3073e8b784a04ce46, which is master at this date). See PR: https://github.com/seccomp/libseccomp/pull/134 Test results: # ./regression -T live Regression Test Summary tests run: 8 tests skipped: 0 tests passed: 8 tests failed: 0 tests errored: 0 # ./regression Regression Test Summary tests run: 5129 tests skipped: 104 tests passed: 5129 tests failed: 0 tests errored: 0 David Abdurachmanov (2): riscv: add support for SECCOMP incl. filters riscv: fix syscall_{get,set}_arguments arch/riscv/Kconfig | 14 ++++++++++ arch/riscv/include/asm/syscall.h | 42 +++++++++++++++++++++------- arch/riscv/include/asm/thread_info.h | 5 +++- arch/riscv/kernel/entry.S | 27 ++++++++++++++++-- arch/riscv/kernel/ptrace.c | 8 ++++++ 5 files changed, 83 insertions(+), 13 deletions(-) -- 2.19.2 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/2] riscv: add support for SECCOMP incl. filters 2018-12-06 15:01 [PATCH 0/2] riscv: add support for SECCOMP and SECCOMP_FILTER David Abdurachmanov @ 2018-12-06 15:01 ` David Abdurachmanov 2018-12-06 16:47 ` Kees Cook ` (2 more replies) 2018-12-06 15:01 ` [PATCH 2/2] riscv: fix syscall_{get,set}_arguments David Abdurachmanov 1 sibling, 3 replies; 13+ messages in thread From: David Abdurachmanov @ 2018-12-06 15:01 UTC (permalink / raw) To: palmer, aou, keescook, luto, wad, green.hu, deanbo422, linux-kernel, linux-riscv Cc: David Abdurachmanov The patch adds support for SECCOMP and SECCOMP_FILTER (BPF). Signed-off-by: David Abdurachmanov <david.abdurachmanov@gmail.com> --- arch/riscv/Kconfig | 14 ++++++++++++++ arch/riscv/include/asm/thread_info.h | 5 ++++- arch/riscv/kernel/entry.S | 27 +++++++++++++++++++++++++-- arch/riscv/kernel/ptrace.c | 8 ++++++++ 4 files changed, 51 insertions(+), 3 deletions(-) diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index a4f48f757204..49cd8e251547 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -29,6 +29,7 @@ config RISCV select GENERIC_SMP_IDLE_THREAD select GENERIC_ATOMIC64 if !64BIT || !RISCV_ISA_A select HAVE_ARCH_AUDITSYSCALL + select HAVE_ARCH_SECCOMP_FILTER select HAVE_MEMBLOCK_NODE_MAP select HAVE_DMA_CONTIGUOUS select HAVE_FUTEX_CMPXCHG if FUTEX @@ -228,6 +229,19 @@ menu "Kernel features" source "kernel/Kconfig.hz" +config SECCOMP + bool "Enable seccomp to safely compute untrusted bytecode" + help + This kernel feature is useful for number crunching applications + that may need to compute untrusted bytecode during their + execution. By using pipes or other transports made available to + the process as file descriptors supporting the read/write + syscalls, it's possible to isolate those applications in + their own address space using seccomp. Once seccomp is + enabled via prctl(PR_SET_SECCOMP), it cannot be disabled + and the task is only allowed to execute a few safe syscalls + defined by each seccomp mode. + endmenu menu "Boot options" diff --git a/arch/riscv/include/asm/thread_info.h b/arch/riscv/include/asm/thread_info.h index 1c9cc8389928..1fd6e4130cab 100644 --- a/arch/riscv/include/asm/thread_info.h +++ b/arch/riscv/include/asm/thread_info.h @@ -81,6 +81,7 @@ struct thread_info { #define TIF_MEMDIE 5 /* is terminating due to OOM killer */ #define TIF_SYSCALL_TRACEPOINT 6 /* syscall tracepoint instrumentation */ #define TIF_SYSCALL_AUDIT 7 /* syscall auditing */ +#define TIF_SECCOMP 8 /* syscall secure computing */ #define _TIF_SYSCALL_TRACE (1 << TIF_SYSCALL_TRACE) #define _TIF_NOTIFY_RESUME (1 << TIF_NOTIFY_RESUME) @@ -88,11 +89,13 @@ struct thread_info { #define _TIF_NEED_RESCHED (1 << TIF_NEED_RESCHED) #define _TIF_SYSCALL_TRACEPOINT (1 << TIF_SYSCALL_TRACEPOINT) #define _TIF_SYSCALL_AUDIT (1 << TIF_SYSCALL_AUDIT) +#define _TIF_SECCOMP (1 << TIF_SECCOMP) #define _TIF_WORK_MASK \ (_TIF_NOTIFY_RESUME | _TIF_SIGPENDING | _TIF_NEED_RESCHED) #define _TIF_SYSCALL_WORK \ - (_TIF_SYSCALL_TRACE | _TIF_SYSCALL_TRACEPOINT | _TIF_SYSCALL_AUDIT) + (_TIF_SYSCALL_TRACE | _TIF_SYSCALL_TRACEPOINT | _TIF_SYSCALL_AUDIT \ + _TIF_SECCOMP ) #endif /* _ASM_RISCV_THREAD_INFO_H */ diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S index 355166f57205..e88ccbfa61ee 100644 --- a/arch/riscv/kernel/entry.S +++ b/arch/riscv/kernel/entry.S @@ -207,8 +207,25 @@ check_syscall_nr: /* Check to make sure we don't jump to a bogus syscall number. */ li t0, __NR_syscalls la s0, sys_ni_syscall - /* Syscall number held in a7 */ - bgeu a7, t0, 1f + /* + * The tracer can change syscall number to valid/invalid value. + * We use syscall_set_nr helper in syscall_trace_enter thus we + * cannot trust the current value in a7 and have to reload from + * the current task pt_regs. + */ + REG_L a7, PT_A7(sp) + /* + * 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 or seccomp, i.e., a7 == -1. + * If yes, we pretend it was executed. + */ + li t1, -1 + beq a7, t1, ret_from_syscall_rejected + /* Call syscall */ la s0, sys_call_table slli t0, a7, RISCV_LGPTR add s0, s0, t0 @@ -219,6 +236,12 @@ check_syscall_nr: ret_from_syscall: /* Set user a0 to kernel a0 */ REG_S a0, PT_A0(sp) + /* + * We didn't execute the actual syscall. + * Seccomp already set return value for the current task pt_regs. + * (If it was configured with SECCOMP_RET_ERRNO/TRACE) + */ +ret_from_syscall_rejected: /* Trace syscalls, but only if requested by the user. */ REG_L t0, TASK_TI_FLAGS(tp) andi t0, t0, _TIF_SYSCALL_WORK diff --git a/arch/riscv/kernel/ptrace.c b/arch/riscv/kernel/ptrace.c index c1b51539c3e2..598e48b8ca2b 100644 --- a/arch/riscv/kernel/ptrace.c +++ b/arch/riscv/kernel/ptrace.c @@ -160,6 +160,14 @@ void do_syscall_trace_enter(struct pt_regs *regs) if (tracehook_report_syscall_entry(regs)) syscall_set_nr(current, regs, -1); + /* + * Do the secure computing after ptrace; failures should be fast. + * If this fails we might have return value in a0 from seccomp + * (via SECCOMP_RET_ERRNO/TRACE). + */ + if (secure_computing(NULL) == -1) + syscall_set_nr(current, regs, -1); + #ifdef CONFIG_HAVE_SYSCALL_TRACEPOINTS if (test_thread_flag(TIF_SYSCALL_TRACEPOINT)) trace_sys_enter(regs, syscall_get_nr(current, regs)); -- 2.19.2 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] riscv: add support for SECCOMP incl. filters 2018-12-06 15:01 ` [PATCH 1/2] riscv: add support for SECCOMP incl. filters David Abdurachmanov @ 2018-12-06 16:47 ` Kees Cook 2018-12-06 17:10 ` David Abdurachmanov 2018-12-06 16:51 ` Kees Cook 2018-12-06 17:06 ` Kees Cook 2 siblings, 1 reply; 13+ messages in thread From: Kees Cook @ 2018-12-06 16:47 UTC (permalink / raw) To: David Abdurachmanov Cc: Albert Ou, Will Drewry, Palmer Dabbelt, LKML, Andy Lutomirski, green.hu, linux-riscv, deanbo422 On Thu, Dec 6, 2018 at 7:02 AM David Abdurachmanov <david.abdurachmanov@gmail.com> wrote: > > The patch adds support for SECCOMP and SECCOMP_FILTER (BPF). > > Signed-off-by: David Abdurachmanov <david.abdurachmanov@gmail.com> > --- > arch/riscv/Kconfig | 14 ++++++++++++++ > arch/riscv/include/asm/thread_info.h | 5 ++++- > arch/riscv/kernel/entry.S | 27 +++++++++++++++++++++++++-- > arch/riscv/kernel/ptrace.c | 8 ++++++++ > 4 files changed, 51 insertions(+), 3 deletions(-) > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > index a4f48f757204..49cd8e251547 100644 > --- a/arch/riscv/Kconfig > +++ b/arch/riscv/Kconfig > @@ -29,6 +29,7 @@ config RISCV > select GENERIC_SMP_IDLE_THREAD > select GENERIC_ATOMIC64 if !64BIT || !RISCV_ISA_A > select HAVE_ARCH_AUDITSYSCALL > + select HAVE_ARCH_SECCOMP_FILTER > select HAVE_MEMBLOCK_NODE_MAP > select HAVE_DMA_CONTIGUOUS > select HAVE_FUTEX_CMPXCHG if FUTEX > @@ -228,6 +229,19 @@ menu "Kernel features" > > source "kernel/Kconfig.hz" > > +config SECCOMP > + bool "Enable seccomp to safely compute untrusted bytecode" > + help > + This kernel feature is useful for number crunching applications > + that may need to compute untrusted bytecode during their > + execution. By using pipes or other transports made available to > + the process as file descriptors supporting the read/write > + syscalls, it's possible to isolate those applications in > + their own address space using seccomp. Once seccomp is > + enabled via prctl(PR_SET_SECCOMP), it cannot be disabled > + and the task is only allowed to execute a few safe syscalls > + defined by each seccomp mode. > + > endmenu > > menu "Boot options" > diff --git a/arch/riscv/include/asm/thread_info.h b/arch/riscv/include/asm/thread_info.h > index 1c9cc8389928..1fd6e4130cab 100644 > --- a/arch/riscv/include/asm/thread_info.h > +++ b/arch/riscv/include/asm/thread_info.h > @@ -81,6 +81,7 @@ struct thread_info { > #define TIF_MEMDIE 5 /* is terminating due to OOM killer */ > #define TIF_SYSCALL_TRACEPOINT 6 /* syscall tracepoint instrumentation */ > #define TIF_SYSCALL_AUDIT 7 /* syscall auditing */ > +#define TIF_SECCOMP 8 /* syscall secure computing */ > > #define _TIF_SYSCALL_TRACE (1 << TIF_SYSCALL_TRACE) > #define _TIF_NOTIFY_RESUME (1 << TIF_NOTIFY_RESUME) > @@ -88,11 +89,13 @@ struct thread_info { > #define _TIF_NEED_RESCHED (1 << TIF_NEED_RESCHED) > #define _TIF_SYSCALL_TRACEPOINT (1 << TIF_SYSCALL_TRACEPOINT) > #define _TIF_SYSCALL_AUDIT (1 << TIF_SYSCALL_AUDIT) > +#define _TIF_SECCOMP (1 << TIF_SECCOMP) > > #define _TIF_WORK_MASK \ > (_TIF_NOTIFY_RESUME | _TIF_SIGPENDING | _TIF_NEED_RESCHED) > > #define _TIF_SYSCALL_WORK \ > - (_TIF_SYSCALL_TRACE | _TIF_SYSCALL_TRACEPOINT | _TIF_SYSCALL_AUDIT) > + (_TIF_SYSCALL_TRACE | _TIF_SYSCALL_TRACEPOINT | _TIF_SYSCALL_AUDIT \ > + _TIF_SECCOMP ) > > #endif /* _ASM_RISCV_THREAD_INFO_H */ > diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S > index 355166f57205..e88ccbfa61ee 100644 > --- a/arch/riscv/kernel/entry.S > +++ b/arch/riscv/kernel/entry.S > @@ -207,8 +207,25 @@ check_syscall_nr: > /* Check to make sure we don't jump to a bogus syscall number. */ > li t0, __NR_syscalls > la s0, sys_ni_syscall > - /* Syscall number held in a7 */ > - bgeu a7, t0, 1f > + /* > + * The tracer can change syscall number to valid/invalid value. > + * We use syscall_set_nr helper in syscall_trace_enter thus we > + * cannot trust the current value in a7 and have to reload from > + * the current task pt_regs. > + */ > + REG_L a7, PT_A7(sp) > + /* > + * 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 or seccomp, i.e., a7 == -1. > + * If yes, we pretend it was executed. > + */ > + li t1, -1 > + beq a7, t1, ret_from_syscall_rejected > + /* Call syscall */ > la s0, sys_call_table > slli t0, a7, RISCV_LGPTR > add s0, s0, t0 > @@ -219,6 +236,12 @@ check_syscall_nr: > ret_from_syscall: > /* Set user a0 to kernel a0 */ > REG_S a0, PT_A0(sp) > + /* > + * We didn't execute the actual syscall. > + * Seccomp already set return value for the current task pt_regs. > + * (If it was configured with SECCOMP_RET_ERRNO/TRACE) > + */ > +ret_from_syscall_rejected: > /* Trace syscalls, but only if requested by the user. */ > REG_L t0, TASK_TI_FLAGS(tp) > andi t0, t0, _TIF_SYSCALL_WORK > diff --git a/arch/riscv/kernel/ptrace.c b/arch/riscv/kernel/ptrace.c > index c1b51539c3e2..598e48b8ca2b 100644 > --- a/arch/riscv/kernel/ptrace.c > +++ b/arch/riscv/kernel/ptrace.c > @@ -160,6 +160,14 @@ void do_syscall_trace_enter(struct pt_regs *regs) > if (tracehook_report_syscall_entry(regs)) > syscall_set_nr(current, regs, -1); > > + /* > + * Do the secure computing after ptrace; failures should be fast. > + * If this fails we might have return value in a0 from seccomp > + * (via SECCOMP_RET_ERRNO/TRACE). > + */ > + if (secure_computing(NULL) == -1) > + syscall_set_nr(current, regs, -1); On a -1 return, this should return immediately -- it should not continue to process trace_sys_enter(), etc. -Kees > + > #ifdef CONFIG_HAVE_SYSCALL_TRACEPOINTS > if (test_thread_flag(TIF_SYSCALL_TRACEPOINT)) > trace_sys_enter(regs, syscall_get_nr(current, regs)); > -- > 2.19.2 > -- Kees Cook _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] riscv: add support for SECCOMP incl. filters 2018-12-06 16:47 ` Kees Cook @ 2018-12-06 17:10 ` David Abdurachmanov 0 siblings, 0 replies; 13+ messages in thread From: David Abdurachmanov @ 2018-12-06 17:10 UTC (permalink / raw) To: Kees Cook Cc: aou, Will Drewry, Palmer Dabbelt, linux-kernel, luto, Green Hu, linux-riscv, deanbo422 On Thu, Dec 6, 2018 at 5:47 PM Kees Cook <keescook@chromium.org> wrote: > > On Thu, Dec 6, 2018 at 7:02 AM David Abdurachmanov > <david.abdurachmanov@gmail.com> wrote: > > > > The patch adds support for SECCOMP and SECCOMP_FILTER (BPF). > > > > Signed-off-by: David Abdurachmanov <david.abdurachmanov@gmail.com> > > --- > > arch/riscv/Kconfig | 14 ++++++++++++++ > > arch/riscv/include/asm/thread_info.h | 5 ++++- > > arch/riscv/kernel/entry.S | 27 +++++++++++++++++++++++++-- > > arch/riscv/kernel/ptrace.c | 8 ++++++++ > > 4 files changed, 51 insertions(+), 3 deletions(-) > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > index a4f48f757204..49cd8e251547 100644 > > --- a/arch/riscv/Kconfig > > +++ b/arch/riscv/Kconfig > > @@ -29,6 +29,7 @@ config RISCV > > select GENERIC_SMP_IDLE_THREAD > > select GENERIC_ATOMIC64 if !64BIT || !RISCV_ISA_A > > select HAVE_ARCH_AUDITSYSCALL > > + select HAVE_ARCH_SECCOMP_FILTER > > select HAVE_MEMBLOCK_NODE_MAP > > select HAVE_DMA_CONTIGUOUS > > select HAVE_FUTEX_CMPXCHG if FUTEX > > @@ -228,6 +229,19 @@ menu "Kernel features" > > > > source "kernel/Kconfig.hz" > > > > +config SECCOMP > > + bool "Enable seccomp to safely compute untrusted bytecode" > > + help > > + This kernel feature is useful for number crunching applications > > + that may need to compute untrusted bytecode during their > > + execution. By using pipes or other transports made available to > > + the process as file descriptors supporting the read/write > > + syscalls, it's possible to isolate those applications in > > + their own address space using seccomp. Once seccomp is > > + enabled via prctl(PR_SET_SECCOMP), it cannot be disabled > > + and the task is only allowed to execute a few safe syscalls > > + defined by each seccomp mode. > > + > > endmenu > > > > menu "Boot options" > > diff --git a/arch/riscv/include/asm/thread_info.h b/arch/riscv/include/asm/thread_info.h > > index 1c9cc8389928..1fd6e4130cab 100644 > > --- a/arch/riscv/include/asm/thread_info.h > > +++ b/arch/riscv/include/asm/thread_info.h > > @@ -81,6 +81,7 @@ struct thread_info { > > #define TIF_MEMDIE 5 /* is terminating due to OOM killer */ > > #define TIF_SYSCALL_TRACEPOINT 6 /* syscall tracepoint instrumentation */ > > #define TIF_SYSCALL_AUDIT 7 /* syscall auditing */ > > +#define TIF_SECCOMP 8 /* syscall secure computing */ > > > > #define _TIF_SYSCALL_TRACE (1 << TIF_SYSCALL_TRACE) > > #define _TIF_NOTIFY_RESUME (1 << TIF_NOTIFY_RESUME) > > @@ -88,11 +89,13 @@ struct thread_info { > > #define _TIF_NEED_RESCHED (1 << TIF_NEED_RESCHED) > > #define _TIF_SYSCALL_TRACEPOINT (1 << TIF_SYSCALL_TRACEPOINT) > > #define _TIF_SYSCALL_AUDIT (1 << TIF_SYSCALL_AUDIT) > > +#define _TIF_SECCOMP (1 << TIF_SECCOMP) > > > > #define _TIF_WORK_MASK \ > > (_TIF_NOTIFY_RESUME | _TIF_SIGPENDING | _TIF_NEED_RESCHED) > > > > #define _TIF_SYSCALL_WORK \ > > - (_TIF_SYSCALL_TRACE | _TIF_SYSCALL_TRACEPOINT | _TIF_SYSCALL_AUDIT) > > + (_TIF_SYSCALL_TRACE | _TIF_SYSCALL_TRACEPOINT | _TIF_SYSCALL_AUDIT \ > > + _TIF_SECCOMP ) > > > > #endif /* _ASM_RISCV_THREAD_INFO_H */ > > diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S > > index 355166f57205..e88ccbfa61ee 100644 > > --- a/arch/riscv/kernel/entry.S > > +++ b/arch/riscv/kernel/entry.S > > @@ -207,8 +207,25 @@ check_syscall_nr: > > /* Check to make sure we don't jump to a bogus syscall number. */ > > li t0, __NR_syscalls > > la s0, sys_ni_syscall > > - /* Syscall number held in a7 */ > > - bgeu a7, t0, 1f > > + /* > > + * The tracer can change syscall number to valid/invalid value. > > + * We use syscall_set_nr helper in syscall_trace_enter thus we > > + * cannot trust the current value in a7 and have to reload from > > + * the current task pt_regs. > > + */ > > + REG_L a7, PT_A7(sp) > > + /* > > + * 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 or seccomp, i.e., a7 == -1. > > + * If yes, we pretend it was executed. > > + */ > > + li t1, -1 > > + beq a7, t1, ret_from_syscall_rejected > > + /* Call syscall */ > > la s0, sys_call_table > > slli t0, a7, RISCV_LGPTR > > add s0, s0, t0 > > @@ -219,6 +236,12 @@ check_syscall_nr: > > ret_from_syscall: > > /* Set user a0 to kernel a0 */ > > REG_S a0, PT_A0(sp) > > + /* > > + * We didn't execute the actual syscall. > > + * Seccomp already set return value for the current task pt_regs. > > + * (If it was configured with SECCOMP_RET_ERRNO/TRACE) > > + */ > > +ret_from_syscall_rejected: > > /* Trace syscalls, but only if requested by the user. */ > > REG_L t0, TASK_TI_FLAGS(tp) > > andi t0, t0, _TIF_SYSCALL_WORK > > diff --git a/arch/riscv/kernel/ptrace.c b/arch/riscv/kernel/ptrace.c > > index c1b51539c3e2..598e48b8ca2b 100644 > > --- a/arch/riscv/kernel/ptrace.c > > +++ b/arch/riscv/kernel/ptrace.c > > @@ -160,6 +160,14 @@ void do_syscall_trace_enter(struct pt_regs *regs) > > if (tracehook_report_syscall_entry(regs)) > > syscall_set_nr(current, regs, -1); > > > > + /* > > + * Do the secure computing after ptrace; failures should be fast. > > + * If this fails we might have return value in a0 from seccomp > > + * (via SECCOMP_RET_ERRNO/TRACE). > > + */ > > + if (secure_computing(NULL) == -1) > > + syscall_set_nr(current, regs, -1); > > On a -1 return, this should return immediately -- it should not > continue to process trace_sys_enter(), etc. Ops! No idea how I missed that. Will fix it. > -Kees > > > + > > #ifdef CONFIG_HAVE_SYSCALL_TRACEPOINTS > > if (test_thread_flag(TIF_SYSCALL_TRACEPOINT)) > > trace_sys_enter(regs, syscall_get_nr(current, regs)); > > -- > > 2.19.2 > > > > > -- > Kees Cook _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] riscv: add support for SECCOMP incl. filters 2018-12-06 15:01 ` [PATCH 1/2] riscv: add support for SECCOMP incl. filters David Abdurachmanov 2018-12-06 16:47 ` Kees Cook @ 2018-12-06 16:51 ` Kees Cook 2018-12-06 17:11 ` David Abdurachmanov 2018-12-06 18:25 ` David Abdurachmanov 2018-12-06 17:06 ` Kees Cook 2 siblings, 2 replies; 13+ messages in thread From: Kees Cook @ 2018-12-06 16:51 UTC (permalink / raw) To: David Abdurachmanov Cc: Albert Ou, Will Drewry, Palmer Dabbelt, LKML, Andy Lutomirski, green.hu, linux-riscv, deanbo422 On Thu, Dec 6, 2018 at 7:02 AM David Abdurachmanov <david.abdurachmanov@gmail.com> wrote: > The patch adds support for SECCOMP and SECCOMP_FILTER (BPF). Can you add support to tools/testing/selftests/seccomp/seccomp_bpf.c as well? That selftest finds a lot of weird corner-cases... > diff --git a/arch/riscv/include/asm/thread_info.h b/arch/riscv/include/asm/thread_info.h > index 1c9cc8389928..1fd6e4130cab 100644 > --- a/arch/riscv/include/asm/thread_info.h > +++ b/arch/riscv/include/asm/thread_info.h > @@ -81,6 +81,7 @@ struct thread_info { > #define TIF_MEMDIE 5 /* is terminating due to OOM killer */ > #define TIF_SYSCALL_TRACEPOINT 6 /* syscall tracepoint instrumentation */ > #define TIF_SYSCALL_AUDIT 7 /* syscall auditing */ > +#define TIF_SECCOMP 8 /* syscall secure computing */ Nit: extra tab needs to be removed. -- Kees Cook _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] riscv: add support for SECCOMP incl. filters 2018-12-06 16:51 ` Kees Cook @ 2018-12-06 17:11 ` David Abdurachmanov 2018-12-06 18:25 ` David Abdurachmanov 1 sibling, 0 replies; 13+ messages in thread From: David Abdurachmanov @ 2018-12-06 17:11 UTC (permalink / raw) To: Kees Cook Cc: aou, Will Drewry, Palmer Dabbelt, linux-kernel, luto, Green Hu, linux-riscv, deanbo422 On Thu, Dec 6, 2018 at 5:52 PM Kees Cook <keescook@chromium.org> wrote: > > On Thu, Dec 6, 2018 at 7:02 AM David Abdurachmanov > <david.abdurachmanov@gmail.com> wrote: > > The patch adds support for SECCOMP and SECCOMP_FILTER (BPF). > > Can you add support to tools/testing/selftests/seccomp/seccomp_bpf.c > as well? That selftest finds a lot of weird corner-cases... > > > diff --git a/arch/riscv/include/asm/thread_info.h b/arch/riscv/include/asm/thread_info.h > > index 1c9cc8389928..1fd6e4130cab 100644 > > --- a/arch/riscv/include/asm/thread_info.h > > +++ b/arch/riscv/include/asm/thread_info.h > > @@ -81,6 +81,7 @@ struct thread_info { > > #define TIF_MEMDIE 5 /* is terminating due to OOM killer */ > > #define TIF_SYSCALL_TRACEPOINT 6 /* syscall tracepoint instrumentation */ > > #define TIF_SYSCALL_AUDIT 7 /* syscall auditing */ > > +#define TIF_SECCOMP 8 /* syscall secure computing */ > > Nit: extra tab needs to be removed. Will fix it. I need to cleanup my vim configuration for tab with. david _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] riscv: add support for SECCOMP incl. filters 2018-12-06 16:51 ` Kees Cook 2018-12-06 17:11 ` David Abdurachmanov @ 2018-12-06 18:25 ` David Abdurachmanov 2018-12-06 18:32 ` Kees Cook 1 sibling, 1 reply; 13+ messages in thread From: David Abdurachmanov @ 2018-12-06 18:25 UTC (permalink / raw) To: Kees Cook Cc: aou, Will Drewry, Palmer Dabbelt, linux-kernel, luto, Green Hu, linux-riscv, deanbo422 On Thu, Dec 6, 2018 at 5:52 PM Kees Cook <keescook@chromium.org> wrote: > > On Thu, Dec 6, 2018 at 7:02 AM David Abdurachmanov > <david.abdurachmanov@gmail.com> wrote: > > The patch adds support for SECCOMP and SECCOMP_FILTER (BPF). > > Can you add support to tools/testing/selftests/seccomp/seccomp_bpf.c > as well? That selftest finds a lot of weird corner-cases... I hate it locally and will include in v2. The results see fine (tested in QEMU). TAP version 13 selftests: seccomp: seccomp_bpf ======================================== [==========] Running 64 tests from 1 test cases. [ RUN ] global.mode_strict_support [ OK ] global.mode_strict_support [ RUN ] global.mode_strict_cannot_call_prctl [ OK ] global.mode_strict_cannot_call_prctl [ RUN ] global.no_new_privs_support [ OK ] global.no_new_privs_support [ RUN ] global.mode_filter_support [ OK ] global.mode_filter_support [ RUN ] global.mode_filter_without_nnp [ OK ] global.mode_filter_without_nnp [ RUN ] global.filter_size_limits [ OK ] global.filter_size_limits [ RUN ] global.filter_chain_limits [ OK ] global.filter_chain_limits [ RUN ] global.mode_filter_cannot_move_to_strict [ OK ] global.mode_filter_cannot_move_to_strict [ RUN ] global.mode_filter_get_seccomp [ OK ] global.mode_filter_get_seccomp [ RUN ] global.ALLOW_all [ OK ] global.ALLOW_all [ RUN ] global.empty_prog [ OK ] global.empty_prog [ RUN ] global.log_all [ OK ] global.log_all [ RUN ] global.unknown_ret_is_kill_inside [ OK ] global.unknown_ret_is_kill_inside [ RUN ] global.unknown_ret_is_kill_above_allow [ OK ] global.unknown_ret_is_kill_above_allow [ RUN ] global.KILL_all [ OK ] global.KILL_all [ RUN ] global.KILL_one [ OK ] global.KILL_one [ RUN ] global.KILL_one_arg_one [ OK ] global.KILL_one_arg_one [ RUN ] global.KILL_one_arg_six [ OK ] global.KILL_one_arg_six [ RUN ] global.KILL_thread [ OK ] global.KILL_thread [ RUN ] global.KILL_process [ OK ] global.KILL_process [ RUN ] global.arg_out_of_range [ OK ] global.arg_out_of_range [ RUN ] global.ERRNO_valid [ OK ] global.ERRNO_valid [ RUN ] global.ERRNO_zero [ OK ] global.ERRNO_zero [ RUN ] global.ERRNO_capped [ OK ] global.ERRNO_capped [ RUN ] global.ERRNO_order [ OK ] global.ERRNO_order [ RUN ] TRAP.dfl [ OK ] TRAP.dfl [ RUN ] TRAP.ign [ OK ] TRAP.ign [ RUN ] TRAP.handler [ OK ] TRAP.handler [ RUN ] precedence.allow_ok [ OK ] precedence.allow_ok [ RUN ] precedence.kill_is_highest [ OK ] precedence.kill_is_highest [ RUN ] precedence.kill_is_highest_in_any_order [ OK ] precedence.kill_is_highest_in_any_order [ RUN ] precedence.trap_is_second [ OK ] precedence.trap_is_second [ RUN ] precedence.trap_is_second_in_any_order [ OK ] precedence.trap_is_second_in_any_order [ RUN ] precedence.errno_is_third [ OK ] precedence.errno_is_third [ RUN ] precedence.errno_is_third_in_any_order [ OK ] precedence.errno_is_third_in_any_order [ RUN ] precedence.trace_is_fourth [ OK ] precedence.trace_is_fourth [ RUN ] precedence.trace_is_fourth_in_any_order [ OK ] precedence.trace_is_fourth_in_any_order [ RUN ] precedence.log_is_fifth [ OK ] precedence.log_is_fifth [ RUN ] precedence.log_is_fifth_in_any_order [ OK ] precedence.log_is_fifth_in_any_order [ RUN ] TRACE_poke.read_has_side_effects [ OK ] TRACE_poke.read_has_side_effects [ RUN ] TRACE_poke.getpid_runs_normally [ OK ] TRACE_poke.getpid_runs_normally [ RUN ] TRACE_syscall.ptrace_syscall_redirected [ OK ] TRACE_syscall.ptrace_syscall_redirected [ RUN ] TRACE_syscall.ptrace_syscall_dropped [ OK ] TRACE_syscall.ptrace_syscall_dropped [ RUN ] TRACE_syscall.syscall_allowed [ OK ] TRACE_syscall.syscall_allowed [ RUN ] TRACE_syscall.syscall_redirected [ OK ] TRACE_syscall.syscall_redirected [ RUN ] TRACE_syscall.syscall_dropped [ OK ] TRACE_syscall.syscall_dropped [ RUN ] TRACE_syscall.skip_after_RET_TRACE [ OK ] TRACE_syscall.skip_after_RET_TRACE [ RUN ] TRACE_syscall.kill_after_RET_TRACE [ OK ] TRACE_syscall.kill_after_RET_TRACE [ RUN ] TRACE_syscall.skip_after_ptrace [ OK ] TRACE_syscall.skip_after_ptrace [ RUN ] TRACE_syscall.kill_after_ptrace [ OK ] TRACE_syscall.kill_after_ptrace [ RUN ] global.seccomp_syscall [ OK ] global.seccomp_syscall [ RUN ] global.seccomp_syscall_mode_lock [ OK ] global.seccomp_syscall_mode_lock [ RUN ] global.detect_seccomp_filter_flags [ OK ] global.detect_seccomp_filter_flags [ RUN ] global.TSYNC_first [ OK ] global.TSYNC_first [ RUN ] TSYNC.siblings_fail_prctl [ OK ] TSYNC.siblings_fail_prctl [ RUN ] TSYNC.two_siblings_with_ancestor [ OK ] TSYNC.two_siblings_with_ancestor [ RUN ] TSYNC.two_sibling_want_nnp [ OK ] TSYNC.two_sibling_want_nnp [ RUN ] TSYNC.two_siblings_with_no_filter [ OK ] TSYNC.two_siblings_with_no_filter [ RUN ] TSYNC.two_siblings_with_one_divergence [ OK ] TSYNC.two_siblings_with_one_divergence [ RUN ] TSYNC.two_siblings_not_under_filter [ OK ] TSYNC.two_siblings_not_under_filter [ RUN ] global.syscall_restart [ OK ] global.syscall_restart [ RUN ] global.filter_flag_log [ OK ] global.filter_flag_log [ RUN ] global.get_action_avail [ OK ] global.get_action_avail [ RUN ] global.get_metadata [ OK ] global.get_metadata [==========] 64 / 64 tests passed. [ PASSED ] ok 1..1 selftests: seccomp: seccomp_bpf [PASS] selftests: seccomp: seccomp_benchmark ======================================== Calibrating reasonable sample size... 1544120467.383132905 - 1544120467.382814604 = 318301 1544120467.384111505 - 1544120467.383931405 = 180100 1544120467.385728706 - 1544120467.384510905 = 1217801 1544120467.386858006 - 1544120467.386096506 = 761500 1544120467.388563407 - 1544120467.387171006 = 1392401 1544120467.392465908 - 1544120467.390143107 = 2322801 1544120467.397988410 - 1544120467.393666109 = 4322301 1544120467.406494614 - 1544120467.398347511 = 8147103 1544120467.427372522 - 1544120467.406955414 = 20417108 1544120467.467600338 - 1544120467.427772222 = 39828116 1544120467.542484667 - 1544120467.467954738 = 74529929 1544120467.693806026 - 1544120467.543004867 = 150801159 1544120467.970921334 - 1544120467.694244026 = 276677308 1544120468.522149049 - 1544120467.971549534 = 550599515 1544120469.637696984 - 1544120468.522606749 = 1115090235 1544120471.829467338 - 1544120469.638147084 = 2191320254 1544120476.263601568 - 1544120471.829850239 = 4433751329 1544120485.135465027 - 1544120476.263980268 = 8871484759 Benchmarking 4194304 samples... 26.716000000 - 17.812000000 = 8904000000 getpid native: 2122 ns 46.548000000 - 26.716000000 = 19832000000 getpid RET_ALLOW: 4728 ns Estimated seccomp overhead per syscall: 2606 ns ok 1..2 selftests: seccomp: seccomp_benchmark [PASS] > > > diff --git a/arch/riscv/include/asm/thread_info.h b/arch/riscv/include/asm/thread_info.h > > index 1c9cc8389928..1fd6e4130cab 100644 > > --- a/arch/riscv/include/asm/thread_info.h > > +++ b/arch/riscv/include/asm/thread_info.h > > @@ -81,6 +81,7 @@ struct thread_info { > > #define TIF_MEMDIE 5 /* is terminating due to OOM killer */ > > #define TIF_SYSCALL_TRACEPOINT 6 /* syscall tracepoint instrumentation */ > > #define TIF_SYSCALL_AUDIT 7 /* syscall auditing */ > > +#define TIF_SECCOMP 8 /* syscall secure computing */ > > Nit: extra tab needs to be removed. > > -- > Kees Cook _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] riscv: add support for SECCOMP incl. filters 2018-12-06 18:25 ` David Abdurachmanov @ 2018-12-06 18:32 ` Kees Cook 0 siblings, 0 replies; 13+ messages in thread From: Kees Cook @ 2018-12-06 18:32 UTC (permalink / raw) To: David Abdurachmanov Cc: Albert Ou, Will Drewry, Palmer Dabbelt, LKML, Andy Lutomirski, green.hu, linux-riscv, deanbo422 On Thu, Dec 6, 2018 at 10:26 AM David Abdurachmanov <david.abdurachmanov@gmail.com> wrote: > > On Thu, Dec 6, 2018 at 5:52 PM Kees Cook <keescook@chromium.org> wrote: > > > > On Thu, Dec 6, 2018 at 7:02 AM David Abdurachmanov > > <david.abdurachmanov@gmail.com> wrote: > > > The patch adds support for SECCOMP and SECCOMP_FILTER (BPF). > > > > Can you add support to tools/testing/selftests/seccomp/seccomp_bpf.c > > as well? That selftest finds a lot of weird corner-cases... > > I hate it locally and will include in v2. I hate it too. ;) But seriously (reading through the "have" typo), that's awesome. Thanks! > The results see fine (tested in QEMU). Great! -- Kees Cook _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] riscv: add support for SECCOMP incl. filters 2018-12-06 15:01 ` [PATCH 1/2] riscv: add support for SECCOMP incl. filters David Abdurachmanov 2018-12-06 16:47 ` Kees Cook 2018-12-06 16:51 ` Kees Cook @ 2018-12-06 17:06 ` Kees Cook 2018-12-06 17:12 ` David Abdurachmanov 2 siblings, 1 reply; 13+ messages in thread From: Kees Cook @ 2018-12-06 17:06 UTC (permalink / raw) To: David Abdurachmanov Cc: Albert Ou, Will Drewry, Palmer Dabbelt, LKML, Andy Lutomirski, green.hu, linux-riscv, deanbo422 On Thu, Dec 6, 2018 at 7:02 AM David Abdurachmanov <david.abdurachmanov@gmail.com> wrote: > The patch adds support for SECCOMP and SECCOMP_FILTER (BPF). I built this against linux-next but it's missing seccomp.h. Was that accidentally left out of the commit? CC arch/riscv/kernel/asm-offsets.s In file included from ./include/linux/sched.h:21:0, from arch/riscv/kernel/asm-offsets.c:18: ./include/linux/seccomp.h:14:10: fatal error: asm/seccomp.h: No such file or directory #include <asm/seccomp.h> ^~~~~~~~~~~~~~~ -- Kees Cook _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/2] riscv: add support for SECCOMP incl. filters 2018-12-06 17:06 ` Kees Cook @ 2018-12-06 17:12 ` David Abdurachmanov 0 siblings, 0 replies; 13+ messages in thread From: David Abdurachmanov @ 2018-12-06 17:12 UTC (permalink / raw) To: Kees Cook Cc: aou, Will Drewry, Palmer Dabbelt, linux-kernel, luto, Green Hu, linux-riscv, deanbo422 On Thu, Dec 6, 2018 at 6:07 PM Kees Cook <keescook@chromium.org> wrote: > > On Thu, Dec 6, 2018 at 7:02 AM David Abdurachmanov > <david.abdurachmanov@gmail.com> wrote: > > The patch adds support for SECCOMP and SECCOMP_FILTER (BPF). > > I built this against linux-next but it's missing seccomp.h. Was that > accidentally left out of the commit? > > > CC arch/riscv/kernel/asm-offsets.s > In file included from ./include/linux/sched.h:21:0, > from arch/riscv/kernel/asm-offsets.c:18: > ./include/linux/seccomp.h:14:10: fatal error: asm/seccomp.h: No such > file or directory > #include <asm/seccomp.h> > ^~~~~~~~~~~~~~~ > I forgot to add it... Will fix it. david _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/2] riscv: fix syscall_{get,set}_arguments 2018-12-06 15:01 [PATCH 0/2] riscv: add support for SECCOMP and SECCOMP_FILTER David Abdurachmanov 2018-12-06 15:01 ` [PATCH 1/2] riscv: add support for SECCOMP incl. filters David Abdurachmanov @ 2018-12-06 15:01 ` David Abdurachmanov 2018-12-06 16:48 ` Kees Cook 1 sibling, 1 reply; 13+ messages in thread From: David Abdurachmanov @ 2018-12-06 15:01 UTC (permalink / raw) To: palmer, aou, keescook, luto, wad, green.hu, deanbo422, linux-kernel, linux-riscv Cc: David Abdurachmanov Testing with libseccomp master branch revealed that testcases with filters on syscall arguments were failing due to wrong values. Seccomp uses syscall_get_argumentsi() to copy syscall arguments, and there is a bug in pointer arithmetics in memcpy() call. Two alternative implementation were tested: the one in this patch and another one based on while-break loop. Both delivered the same results. This implementation is also used in arm, arm64 and nds32 arches. Signed-off-by: David Abdurachmanov <david.abdurachmanov@gmail.com> --- arch/riscv/include/asm/syscall.h | 42 ++++++++++++++++++++++++-------- 1 file changed, 32 insertions(+), 10 deletions(-) diff --git a/arch/riscv/include/asm/syscall.h b/arch/riscv/include/asm/syscall.h index bba3da6ef157..26ceb434a433 100644 --- a/arch/riscv/include/asm/syscall.h +++ b/arch/riscv/include/asm/syscall.h @@ -70,19 +70,32 @@ static inline void syscall_set_return_value(struct task_struct *task, regs->a0 = (long) error ?: val; } +#define SYSCALL_MAX_ARGS 6 + static inline void syscall_get_arguments(struct task_struct *task, struct pt_regs *regs, unsigned int i, unsigned int n, unsigned long *args) { - BUG_ON(i + n > 6); + if (n == 0) + return; + + if (i + n > SYSCALL_MAX_ARGS) { + unsigned long *args_bad = args + SYSCALL_MAX_ARGS - i; + unsigned int n_bad = n + i - SYSCALL_MAX_ARGS; + pr_warning("%s called with max args %d, handling only %d\n", + __func__, i + n, SYSCALL_MAX_ARGS); + memset(args_bad, 0, n_bad * sizeof(args[0])); + } + if (i == 0) { args[0] = regs->orig_a0; args++; i++; n--; } - memcpy(args, ®s->a1 + i * sizeof(regs->a1), n * sizeof(args[0])); + + memcpy(args, ®s->a0 + i, n * sizeof(args[0])); } static inline void syscall_set_arguments(struct task_struct *task, @@ -90,14 +103,23 @@ static inline void syscall_set_arguments(struct task_struct *task, unsigned int i, unsigned int n, const unsigned long *args) { - BUG_ON(i + n > 6); - if (i == 0) { - regs->orig_a0 = args[0]; - args++; - i++; - n--; - } - memcpy(®s->a1 + i * sizeof(regs->a1), args, n * sizeof(regs->a0)); + if (n == 0) + return; + + if (i + n > SYSCALL_MAX_ARGS) { + pr_warning("%s called with max args %d, handling only %d\n", + __func__, i + n, SYSCALL_MAX_ARGS); + n = SYSCALL_MAX_ARGS - i; + } + + if (i == 0) { + regs->orig_a0 = args[0]; + args++; + i++; + n--; + } + + memcpy(®s->a0 + i, args, n * sizeof(args[0])); } static inline int syscall_get_arch(void) -- 2.19.2 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] riscv: fix syscall_{get,set}_arguments 2018-12-06 15:01 ` [PATCH 2/2] riscv: fix syscall_{get,set}_arguments David Abdurachmanov @ 2018-12-06 16:48 ` Kees Cook 2018-12-06 17:13 ` David Abdurachmanov 0 siblings, 1 reply; 13+ messages in thread From: Kees Cook @ 2018-12-06 16:48 UTC (permalink / raw) To: David Abdurachmanov Cc: Albert Ou, Will Drewry, Palmer Dabbelt, LKML, Andy Lutomirski, green.hu, linux-riscv, deanbo422 On Thu, Dec 6, 2018 at 7:02 AM David Abdurachmanov <david.abdurachmanov@gmail.com> wrote: > > Testing with libseccomp master branch revealed that testcases with > filters on syscall arguments were failing due to wrong values. Seccomp > uses syscall_get_argumentsi() to copy syscall arguments, and there is a > bug in pointer arithmetics in memcpy() call. > > Two alternative implementation were tested: the one in this patch and > another one based on while-break loop. Both delivered the same results. > > This implementation is also used in arm, arm64 and nds32 arches. Minor nit: can you make this the first patch? That way seccomp works correctly from the point of introduction. :) -Kees > > Signed-off-by: David Abdurachmanov <david.abdurachmanov@gmail.com> > --- > arch/riscv/include/asm/syscall.h | 42 ++++++++++++++++++++++++-------- > 1 file changed, 32 insertions(+), 10 deletions(-) > > diff --git a/arch/riscv/include/asm/syscall.h b/arch/riscv/include/asm/syscall.h > index bba3da6ef157..26ceb434a433 100644 > --- a/arch/riscv/include/asm/syscall.h > +++ b/arch/riscv/include/asm/syscall.h > @@ -70,19 +70,32 @@ static inline void syscall_set_return_value(struct task_struct *task, > regs->a0 = (long) error ?: val; > } > > +#define SYSCALL_MAX_ARGS 6 > + > static inline void syscall_get_arguments(struct task_struct *task, > struct pt_regs *regs, > unsigned int i, unsigned int n, > unsigned long *args) > { > - BUG_ON(i + n > 6); > + if (n == 0) > + return; > + > + if (i + n > SYSCALL_MAX_ARGS) { > + unsigned long *args_bad = args + SYSCALL_MAX_ARGS - i; > + unsigned int n_bad = n + i - SYSCALL_MAX_ARGS; > + pr_warning("%s called with max args %d, handling only %d\n", > + __func__, i + n, SYSCALL_MAX_ARGS); > + memset(args_bad, 0, n_bad * sizeof(args[0])); > + } > + > if (i == 0) { > args[0] = regs->orig_a0; > args++; > i++; > n--; > } > - memcpy(args, ®s->a1 + i * sizeof(regs->a1), n * sizeof(args[0])); > + > + memcpy(args, ®s->a0 + i, n * sizeof(args[0])); > } > > static inline void syscall_set_arguments(struct task_struct *task, > @@ -90,14 +103,23 @@ static inline void syscall_set_arguments(struct task_struct *task, > unsigned int i, unsigned int n, > const unsigned long *args) > { > - BUG_ON(i + n > 6); > - if (i == 0) { > - regs->orig_a0 = args[0]; > - args++; > - i++; > - n--; > - } > - memcpy(®s->a1 + i * sizeof(regs->a1), args, n * sizeof(regs->a0)); > + if (n == 0) > + return; > + > + if (i + n > SYSCALL_MAX_ARGS) { > + pr_warning("%s called with max args %d, handling only %d\n", > + __func__, i + n, SYSCALL_MAX_ARGS); > + n = SYSCALL_MAX_ARGS - i; > + } > + > + if (i == 0) { > + regs->orig_a0 = args[0]; > + args++; > + i++; > + n--; > + } > + > + memcpy(®s->a0 + i, args, n * sizeof(args[0])); > } > > static inline int syscall_get_arch(void) > -- > 2.19.2 > -- Kees Cook _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/2] riscv: fix syscall_{get,set}_arguments 2018-12-06 16:48 ` Kees Cook @ 2018-12-06 17:13 ` David Abdurachmanov 0 siblings, 0 replies; 13+ messages in thread From: David Abdurachmanov @ 2018-12-06 17:13 UTC (permalink / raw) To: Kees Cook Cc: aou, Will Drewry, Palmer Dabbelt, linux-kernel, luto, Green Hu, linux-riscv, deanbo422 On Thu, Dec 6, 2018 at 5:49 PM Kees Cook <keescook@chromium.org> wrote: > > On Thu, Dec 6, 2018 at 7:02 AM David Abdurachmanov > <david.abdurachmanov@gmail.com> wrote: > > > > Testing with libseccomp master branch revealed that testcases with > > filters on syscall arguments were failing due to wrong values. Seccomp > > uses syscall_get_argumentsi() to copy syscall arguments, and there is a > > bug in pointer arithmetics in memcpy() call. > > > > Two alternative implementation were tested: the one in this patch and > > another one based on while-break loop. Both delivered the same results. > > > > This implementation is also used in arm, arm64 and nds32 arches. > > Minor nit: can you make this the first patch? That way seccomp works > correctly from the point of introduction. :) Ok. I will do it. david > > -Kees > > > > > Signed-off-by: David Abdurachmanov <david.abdurachmanov@gmail.com> > > --- > > arch/riscv/include/asm/syscall.h | 42 ++++++++++++++++++++++++-------- > > 1 file changed, 32 insertions(+), 10 deletions(-) > > > > diff --git a/arch/riscv/include/asm/syscall.h b/arch/riscv/include/asm/syscall.h > > index bba3da6ef157..26ceb434a433 100644 > > --- a/arch/riscv/include/asm/syscall.h > > +++ b/arch/riscv/include/asm/syscall.h > > @@ -70,19 +70,32 @@ static inline void syscall_set_return_value(struct task_struct *task, > > regs->a0 = (long) error ?: val; > > } > > > > +#define SYSCALL_MAX_ARGS 6 > > + > > static inline void syscall_get_arguments(struct task_struct *task, > > struct pt_regs *regs, > > unsigned int i, unsigned int n, > > unsigned long *args) > > { > > - BUG_ON(i + n > 6); > > + if (n == 0) > > + return; > > + > > + if (i + n > SYSCALL_MAX_ARGS) { > > + unsigned long *args_bad = args + SYSCALL_MAX_ARGS - i; > > + unsigned int n_bad = n + i - SYSCALL_MAX_ARGS; > > + pr_warning("%s called with max args %d, handling only %d\n", > > + __func__, i + n, SYSCALL_MAX_ARGS); > > + memset(args_bad, 0, n_bad * sizeof(args[0])); > > + } > > + > > if (i == 0) { > > args[0] = regs->orig_a0; > > args++; > > i++; > > n--; > > } > > - memcpy(args, ®s->a1 + i * sizeof(regs->a1), n * sizeof(args[0])); > > + > > + memcpy(args, ®s->a0 + i, n * sizeof(args[0])); > > } > > > > static inline void syscall_set_arguments(struct task_struct *task, > > @@ -90,14 +103,23 @@ static inline void syscall_set_arguments(struct task_struct *task, > > unsigned int i, unsigned int n, > > const unsigned long *args) > > { > > - BUG_ON(i + n > 6); > > - if (i == 0) { > > - regs->orig_a0 = args[0]; > > - args++; > > - i++; > > - n--; > > - } > > - memcpy(®s->a1 + i * sizeof(regs->a1), args, n * sizeof(regs->a0)); > > + if (n == 0) > > + return; > > + > > + if (i + n > SYSCALL_MAX_ARGS) { > > + pr_warning("%s called with max args %d, handling only %d\n", > > + __func__, i + n, SYSCALL_MAX_ARGS); > > + n = SYSCALL_MAX_ARGS - i; > > + } > > + > > + if (i == 0) { > > + regs->orig_a0 = args[0]; > > + args++; > > + i++; > > + n--; > > + } > > + > > + memcpy(®s->a0 + i, args, n * sizeof(args[0])); > > } > > > > static inline int syscall_get_arch(void) > > -- > > 2.19.2 > > > > > -- > Kees Cook _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2018-12-06 18:33 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-12-06 15:01 [PATCH 0/2] riscv: add support for SECCOMP and SECCOMP_FILTER David Abdurachmanov 2018-12-06 15:01 ` [PATCH 1/2] riscv: add support for SECCOMP incl. filters David Abdurachmanov 2018-12-06 16:47 ` Kees Cook 2018-12-06 17:10 ` David Abdurachmanov 2018-12-06 16:51 ` Kees Cook 2018-12-06 17:11 ` David Abdurachmanov 2018-12-06 18:25 ` David Abdurachmanov 2018-12-06 18:32 ` Kees Cook 2018-12-06 17:06 ` Kees Cook 2018-12-06 17:12 ` David Abdurachmanov 2018-12-06 15:01 ` [PATCH 2/2] riscv: fix syscall_{get,set}_arguments David Abdurachmanov 2018-12-06 16:48 ` Kees Cook 2018-12-06 17:13 ` David Abdurachmanov
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).