Re: [PATCH v1 1/4] lib: introduce copy_struct_from_user() helper

2019-09-25 Thread kbuild test robot
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

Re: [PATCH v1 1/4] lib: introduce copy_struct_from_user() helper

2019-09-25 Thread Linus Torvalds
On Wed, Sep 25, 2019 at 12:43 PM Al Viro wrote: > > FWIW, I would probably add a kernel-space analogue of that thing at the > same time - check that an area is all-zeroes is not all that rare. Hmm. Maybe. > Another thing is that for s390 we almost certainly want something better > than word-by-w

Re: [PATCH v1 1/4] lib: introduce copy_struct_from_user() helper

2019-09-25 Thread Al Viro
On Wed, Sep 25, 2019 at 11:13:18AM -0700, Linus Torvalds wrote: > On Wed, Sep 25, 2019 at 11:04 AM Al Viro wrote: > > > > IMO it's better to lift reading the first word out of the loop, like this: > > ... > > Do you see any problems with that variant? > > That looks fine to me too. > > It's a

Re: [PATCH v1 1/4] lib: introduce copy_struct_from_user() helper

2019-09-25 Thread kbuild test robot
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

Re: [PATCH v1 1/4] lib: introduce copy_struct_from_user() helper

2019-09-25 Thread Linus Torvalds
On Wed, Sep 25, 2019 at 11:04 AM Al Viro wrote: > > IMO it's better to lift reading the first word out of the loop, like this: > ... > Do you see any problems with that variant? That looks fine to me too. It's a bit harder for humans to read because of how it reads the word from user space one

Re: [PATCH v1 1/4] lib: introduce copy_struct_from_user() helper

2019-09-25 Thread Al Viro
On Wed, Sep 25, 2019 at 10:48:31AM -0700, Linus Torvalds wrote: > On Wed, Sep 25, 2019 at 10:21 AM Aleksa Sarai wrote: > > > > Just to make sure I understand, the following diff would this solve the > > problem? If so, I'll apply it, and re-send in a few hours. > > Actually, looking at it more, i

Re: [PATCH v1 1/4] lib: introduce copy_struct_from_user() helper

2019-09-25 Thread Linus Torvalds
On Wed, Sep 25, 2019 at 10:21 AM Aleksa Sarai wrote: > > Just to make sure I understand, the following diff would this solve the > problem? If so, I'll apply it, and re-send in a few hours. Actually, looking at it more, it's still buggy. That final "size smaller than unsigned long" doesn't corre

Re: [PATCH v1 1/4] lib: introduce copy_struct_from_user() helper

2019-09-25 Thread Aleksa Sarai
On 2019-09-25, Linus Torvalds wrote: > On Wed, Sep 25, 2019 at 10:00 AM Aleksa Sarai wrote: > > > > +int is_zeroed_user(const void __user *from, size_t size) > > I like how you've done this, but it's buggy and only works on 64-bit. > > All the "u64" and "8" cases need to be "unsigned long" and

Re: [PATCH v1 1/4] lib: introduce copy_struct_from_user() helper

2019-09-25 Thread Christian Brauner
On Wed, Sep 25, 2019 at 07:18:11PM +0200, Christian Brauner wrote: > On Wed, Sep 25, 2019 at 06:59:12PM +0200, Aleksa Sarai wrote: > > A common pattern for syscall extensions is increasing the size of a > > struct passed from userspace, such that the zero-value of the new fields > > result in the o

Re: [PATCH v1 1/4] lib: introduce copy_struct_from_user() helper

2019-09-25 Thread Christian Brauner
On Wed, Sep 25, 2019 at 06:59:12PM +0200, Aleksa Sarai wrote: > A common pattern for syscall extensions is increasing the size of a > struct passed from userspace, such that the zero-value of the new fields > result in the old kernel behaviour (allowing for a mix of userspace and > kernel vintages

Re: [PATCH v1 1/4] lib: introduce copy_struct_from_user() helper

2019-09-25 Thread Linus Torvalds
On Wed, Sep 25, 2019 at 10:00 AM Aleksa Sarai wrote: > > +int is_zeroed_user(const void __user *from, size_t size) I like how you've done this, but it's buggy and only works on 64-bit. All the "u64" and "8" cases need to be "unsigned long" and "sizeof(unsigned long)". Part of that requirement i

[PATCH v1 1/4] lib: introduce copy_struct_from_user() helper

2019-09-25 Thread Aleksa Sarai
A common pattern for syscall extensions is increasing the size of a struct passed from userspace, such that the zero-value of the new fields result in the old kernel behaviour (allowing for a mix of userspace and kernel vintages to operate on one another in most cases). While this interface exists