Re: [PATCH v1 2/4] clone3: switch to copy_struct_from_user()
Hi Aleksa, Thank you for the patch! Yet something to improve: [auto build test ERROR on linus/master] [cannot apply to v5.3 next-20190924] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982] url: https://github.com/0day-ci/linux/commits/Aleksa-Sarai/lib-introduce-copy_struct_from_user-helper/20190926-010527 config: parisc-b180_defconfig (attached as .config) compiler: hppa-linux-gcc (GCC) 7.4.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree GCC_VERSION=7.4.0 make.cross ARCH=parisc If you fix the issue, kindly add following tag Reported-by: kbuild test robot All errors (new ones prefixed by >>): lib/struct_user.o: In function `copy_struct_from_user': >> (.text+0x84): undefined reference to `is_zeroed_user' --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
Re: [PATCH v1 2/4] clone3: switch to copy_struct_from_user()
Hi Aleksa, Thank you for the patch! Yet something to improve: [auto build test ERROR on linus/master] [cannot apply to v5.3 next-20190924] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982] url: https://github.com/0day-ci/linux/commits/Aleksa-Sarai/lib-introduce-copy_struct_from_user-helper/20190926-010527 config: xtensa-common_defconfig (attached as .config) compiler: xtensa-linux-gcc (GCC) 7.4.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree GCC_VERSION=7.4.0 make.cross ARCH=xtensa If you fix the issue, kindly add following tag Reported-by: kbuild test robot All errors (new ones prefixed by >>): >> lib/struct_user.o:(.text+0x8): undefined reference to `is_zeroed_user' lib/struct_user.o: In function `copy_struct_from_user': struct_user.c:(.text+0x37): undefined reference to `is_zeroed_user' --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
Re: [PATCH v1 2/4] clone3: switch to copy_struct_from_user()
On Wed, Sep 25, 2019 at 06:59:13PM +0200, Aleksa Sarai wrote: > The change is very straightforward, and helps unify the syscall > interface for struct-from-userspace syscalls. Additionally, explicitly > define CLONE_ARGS_SIZE_VER0 to match the other users of the > struct-extension pattern. > > Signed-off-by: Aleksa Sarai > --- > include/uapi/linux/sched.h | 2 ++ > kernel/fork.c | 34 +++--- > 2 files changed, 9 insertions(+), 27 deletions(-) > > diff --git a/include/uapi/linux/sched.h b/include/uapi/linux/sched.h > index b3105ac1381a..0945805982b4 100644 > --- a/include/uapi/linux/sched.h > +++ b/include/uapi/linux/sched.h > @@ -47,6 +47,8 @@ struct clone_args { > __aligned_u64 tls; > }; > > +#define CLONE_ARGS_SIZE_VER0 64 /* sizeof first published struct */ > + > /* > * Scheduling policies > */ > diff --git a/kernel/fork.c b/kernel/fork.c > index 541fd805fb88..a86e3841ee4e 100644 > --- a/kernel/fork.c > +++ b/kernel/fork.c > @@ -2530,39 +2530,19 @@ SYSCALL_DEFINE5(clone, unsigned long, clone_flags, > unsigned long, newsp, > #ifdef __ARCH_WANT_SYS_CLONE3 > noinline static int copy_clone_args_from_user(struct kernel_clone_args > *kargs, > struct clone_args __user *uargs, > - size_t size) > + size_t usize) > { > + int err; > struct clone_args args; > > - if (unlikely(size > PAGE_SIZE)) > + if (unlikely(usize > PAGE_SIZE)) > return -E2BIG; > - > - if (unlikely(size < sizeof(struct clone_args))) > + if (unlikely(usize < CLONE_ARGS_SIZE_VER0)) > return -EINVAL; You might want to rebase this patchset after the merge window closes on rc1 since that code has changed right before the 5.3 release. But if you can't don't worry I can also fix it up. Christian
[PATCH v1 2/4] clone3: switch to copy_struct_from_user()
The change is very straightforward, and helps unify the syscall interface for struct-from-userspace syscalls. Additionally, explicitly define CLONE_ARGS_SIZE_VER0 to match the other users of the struct-extension pattern. Signed-off-by: Aleksa Sarai --- include/uapi/linux/sched.h | 2 ++ kernel/fork.c | 34 +++--- 2 files changed, 9 insertions(+), 27 deletions(-) diff --git a/include/uapi/linux/sched.h b/include/uapi/linux/sched.h index b3105ac1381a..0945805982b4 100644 --- a/include/uapi/linux/sched.h +++ b/include/uapi/linux/sched.h @@ -47,6 +47,8 @@ struct clone_args { __aligned_u64 tls; }; +#define CLONE_ARGS_SIZE_VER0 64 /* sizeof first published struct */ + /* * Scheduling policies */ diff --git a/kernel/fork.c b/kernel/fork.c index 541fd805fb88..a86e3841ee4e 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -2530,39 +2530,19 @@ SYSCALL_DEFINE5(clone, unsigned long, clone_flags, unsigned long, newsp, #ifdef __ARCH_WANT_SYS_CLONE3 noinline static int copy_clone_args_from_user(struct kernel_clone_args *kargs, struct clone_args __user *uargs, - size_t size) + size_t usize) { + int err; struct clone_args args; - if (unlikely(size > PAGE_SIZE)) + if (unlikely(usize > PAGE_SIZE)) return -E2BIG; - - if (unlikely(size < sizeof(struct clone_args))) + if (unlikely(usize < CLONE_ARGS_SIZE_VER0)) return -EINVAL; - if (unlikely(!access_ok(uargs, size))) - return -EFAULT; - - if (size > sizeof(struct clone_args)) { - unsigned char __user *addr; - unsigned char __user *end; - unsigned char val; - - addr = (void __user *)uargs + sizeof(struct clone_args); - end = (void __user *)uargs + size; - - for (; addr < end; addr++) { - if (get_user(val, addr)) - return -EFAULT; - if (val) - return -E2BIG; - } - - size = sizeof(struct clone_args); - } - - if (copy_from_user(, uargs, size)) - return -EFAULT; + err = copy_struct_from_user(, sizeof(args), uargs, usize); + if (err) + return err; /* * Verify that higher 32bits of exit_signal are unset and that -- 2.23.0