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. :) -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
