On Thu, Dec 6, 2018 at 5:49 PM Kees Cook <[email protected]> wrote: > > On Thu, Dec 6, 2018 at 7:02 AM David Abdurachmanov > <[email protected]> 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 <[email protected]> > > --- > > 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
