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

2019-09-27 Thread Christian Brauner
On Fri, Sep 27, 2019 at 11:07:36AM +1000, Aleksa Sarai wrote:
> On 2019-09-26, Christian Brauner  wrote:
> > On Thu, Sep 26, 2019 at 01:03:29AM +0200, Aleksa Sarai wrote:
> > > +int is_zeroed_user(const void __user *from, size_t size)
> > > +{
> > > + unsigned long val;
> > > + uintptr_t align = (uintptr_t) from % sizeof(unsigned long);
> > > +
> > > + if (unlikely(!size))
> > > + return true;
> > 
> > You're returning "true" and another implicit boolean with (val == 0)
> > down below but -EFAULT in other places. But that function is int
> > is_zeroed_user() Would probably be good if you either switch to bool
> > is_zeroed_user() as the name suggests or rename the function and have
> > it return an int everywhere.
> 
> I just checked, and in C11 (and presumably in older specs) it is
> guaranteed that "true" and "false" from  have the values 1
> and 0 (respectively) [§7.18]. So this is perfectly well-defined.
> 
If you declare a function as returning an int, return ints and don't mix
returning ints and "proper" C boolean types. This:

static int foo()
{
if (bla)
return true;
return -1;
}

is just messy.

> 
> Personally, I think it's more readable to have:
> 
>   if (unlikely(size == 0))
> return true;
>   /* ... */
>   return (val == 0);
> 
> compared to:
> 
>   if (unlikely(size == 0))
> return 1;
>   /* ... */
>   return val ? 0 : 1;

Just do:

if (unlikely(size == 0))
return 1;
/* ... */
return (val == 0);

You don't need to change the last return.

Also, as I said in a previous mail: Please wait for rc1 (that's just two
days) to be out so you can base your patchset on that as there are
changes in mainline that cause a merge conflict with your changes.

Thanks!
Christian


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

2019-09-26 Thread Aleksa Sarai
On 2019-09-26, Christian Brauner  wrote:
> On Thu, Sep 26, 2019 at 01:03:29AM +0200, Aleksa Sarai wrote:
> > +int is_zeroed_user(const void __user *from, size_t size)
> > +{
> > +   unsigned long val;
> > +   uintptr_t align = (uintptr_t) from % sizeof(unsigned long);
> > +
> > +   if (unlikely(!size))
> > +   return true;
> 
> You're returning "true" and another implicit boolean with (val == 0)
> down below but -EFAULT in other places. But that function is int
> is_zeroed_user() Would probably be good if you either switch to bool
> is_zeroed_user() as the name suggests or rename the function and have
> it return an int everywhere.

I just checked, and in C11 (and presumably in older specs) it is
guaranteed that "true" and "false" from  have the values 1
and 0 (respectively) [§7.18]. So this is perfectly well-defined.

Personally, I think it's more readable to have:

  if (unlikely(size == 0))
return true;
  /* ... */
  return (val == 0);

compared to:

  if (unlikely(size == 0))
return 1;
  /* ... */
  return val ? 0 : 1;

But I will change the function name (to check_zeroed_user) to make it
clearer that it isn't returning a boolean and that you need to check for
negative returns.

-- 
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH



signature.asc
Description: PGP signature


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

2019-09-26 Thread Christian Brauner
On Thu, Sep 26, 2019 at 01:03:29AM +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 to operate on one another in most cases).
> 
> While this interface exists for communication in both directions, only
> one interface is straightforward to have reasonable semantics for
> (userspace passing a struct to the kernel). For kernel returns to
> userspace, what the correct semantics are (whether there should be an
> error if userspace is unaware of a new extension) is very
> syscall-dependent and thus probably cannot be unified between syscalls
> (a good example of this problem is [1]).
> 
> Previously there was no common lib/ function that implemented
> the necessary extension-checking semantics (and different syscalls
> implemented them slightly differently or incompletely[2]). Future
> patches replace common uses of this pattern to make use of
> copy_struct_from_user().
> 
> Some in-kernel selftests that insure that the handling of alignment and
> various byte patterns are all handled identically to memchr_inv() usage.
> 
> [1]: commit 1251201c0d34 ("sched/core: Fix uclamp ABI bug, clean up and
>  robustify sched_read_attr() ABI logic and code")
> 
> [2]: For instance {sched_setattr,perf_event_open,clone3}(2) all do do
>  similar checks to copy_struct_from_user() while rt_sigprocmask(2)
>  always rejects differently-sized struct arguments.
> 
> Suggested-by: Rasmus Villemoes 
> Signed-off-by: Aleksa Sarai 
> ---
>  include/linux/bitops.h  |   7 +++
>  include/linux/uaccess.h |   4 ++
>  lib/strnlen_user.c  |   8 +--
>  lib/test_user_copy.c|  59 ++---
>  lib/usercopy.c  | 115 
>  5 files changed, 180 insertions(+), 13 deletions(-)
> 
> diff --git a/include/linux/bitops.h b/include/linux/bitops.h
> index cf074bce3eb3..a23f4c054768 100644
> --- a/include/linux/bitops.h
> +++ b/include/linux/bitops.h
> @@ -4,6 +4,13 @@
>  #include 
>  #include 
>  
> +/* Set bits in the first 'n' bytes when loaded from memory */
> +#ifdef __LITTLE_ENDIAN
> +#  define aligned_byte_mask(n) ((1ul << 8*(n))-1)
> +#else
> +#  define aligned_byte_mask(n) (~0xfful << (BITS_PER_LONG - 8 - 8*(n)))
> +#endif

Nti: The style in bitops.h suggestes this should be:

+/* Set bits in the first 'n' bytes when loaded from memory */
+#ifdef __LITTLE_ENDIAN
+#  define aligned_byte_mask(n) ((1UL << 8*(n))-1)
+#else
+#  define aligned_byte_mask(n) (~0xffUL << (BITS_PER_LONG - 8 - 8*(n)))
+#endif

Using UL also makes 0xffUL clearer.

> +
>  #define BITS_PER_TYPE(type) (sizeof(type) * BITS_PER_BYTE)
>  #define BITS_TO_LONGS(nr)DIV_ROUND_UP(nr, BITS_PER_TYPE(long))
>  
> diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
> index 34a038563d97..824569e309e4 100644
> --- a/include/linux/uaccess.h
> +++ b/include/linux/uaccess.h
> @@ -230,6 +230,10 @@ static inline unsigned long 
> __copy_from_user_inatomic_nocache(void *to,
>  
>  #endif   /* ARCH_HAS_NOCACHE_UACCESS */
>  
> +extern int is_zeroed_user(const void __user *from, size_t count);
> +extern int copy_struct_from_user(void *dst, size_t ksize,
> +  const void __user *src, size_t usize);
> +
>  /*
>   * probe_kernel_read(): safely attempt to read from a location
>   * @dst: pointer to the buffer that shall take the data
> diff --git a/lib/strnlen_user.c b/lib/strnlen_user.c
> index 7f2db3fe311f..39d588aaa8cd 100644
> --- a/lib/strnlen_user.c
> +++ b/lib/strnlen_user.c
> @@ -2,16 +2,10 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  
> -/* Set bits in the first 'n' bytes when loaded from memory */
> -#ifdef __LITTLE_ENDIAN
> -#  define aligned_byte_mask(n) ((1ul << 8*(n))-1)
> -#else
> -#  define aligned_byte_mask(n) (~0xfful << (BITS_PER_LONG - 8 - 8*(n)))
> -#endif
> -
>  /*
>   * Do a strnlen, return length of string *with* final '\0'.
>   * 'count' is the user-supplied count, while 'max' is the
> diff --git a/lib/test_user_copy.c b/lib/test_user_copy.c
> index 67bcd5dfd847..f7cde3845ccc 100644
> --- a/lib/test_user_copy.c
> +++ b/lib/test_user_copy.c
> @@ -31,14 +31,58 @@
>  # define TEST_U64
>  #endif
>  
> -#define test(condition, msg) \
> -({   \
> - int cond = (condition); \
> - if (cond)   \
> - pr_warn("%s\n", msg);   \
> - cond;   \
> +#define test(condition, msg, ...)\
> +({   \
> + int cond = (condition); \
> + if (cond)   \
> + pr_warn("[%d] " msg "\n", __LINE__, ##__VA_ARGS__); \
> + 

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

2019-09-25 Thread kbuild test robot
Hi Aleksa,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING 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-071752
config: sh-allmodconfig (attached as .config)
compiler: sh4-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=sh 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot 

All warnings (new ones prefixed by >>):

   In file included from include/linux/printk.h:7:0,
from include/linux/kernel.h:15,
from include/asm-generic/bug.h:18,
from arch/sh/include/asm/bug.h:112,
from include/linux/bug.h:5,
from include/linux/mmdebug.h:5,
from include/linux/mm.h:9,
from include/linux/mman.h:5,
from lib/test_user_copy.c:13:
   lib/test_user_copy.c: In function 'test_is_zeroed_user':
   include/linux/kern_levels.h:5:18: warning: format '%lu' expects argument of 
type 'long unsigned int', but argument 5 has type 'size_t {aka unsigned int}' 
[-Wformat=]
#define KERN_SOH "\001"  /* ASCII Start Of Header */
 ^
   include/linux/kern_levels.h:12:22: note: in expansion of macro 'KERN_SOH'
#define KERN_WARNING KERN_SOH "4" /* warning conditions */
 ^~~~
   include/linux/printk.h:306:9: note: in expansion of macro 'KERN_WARNING'
 printk(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
^~~~
   include/linux/printk.h:307:17: note: in expansion of macro 'pr_warning'
#define pr_warn pr_warning
^~
>> lib/test_user_copy.c:38:3: note: in expansion of macro 'pr_warn'
  pr_warn("[%d] " msg "\n", __LINE__, ##__VA_ARGS__); \
  ^~~
>> lib/test_user_copy.c:77:11: note: in expansion of macro 'test'
   ret |= test(retval != expected,
  ^~~~
   include/linux/kern_levels.h:5:18: warning: format '%lu' expects argument of 
type 'long unsigned int', but argument 6 has type 'size_t {aka unsigned int}' 
[-Wformat=]
#define KERN_SOH "\001"  /* ASCII Start Of Header */
 ^
   include/linux/kern_levels.h:12:22: note: in expansion of macro 'KERN_SOH'
#define KERN_WARNING KERN_SOH "4" /* warning conditions */
 ^~~~
   include/linux/printk.h:306:9: note: in expansion of macro 'KERN_WARNING'
 printk(KERN_WARNING pr_fmt(fmt), ##__VA_ARGS__)
^~~~
   include/linux/printk.h:307:17: note: in expansion of macro 'pr_warning'
#define pr_warn pr_warning
^~
>> lib/test_user_copy.c:38:3: note: in expansion of macro 'pr_warn'
  pr_warn("[%d] " msg "\n", __LINE__, ##__VA_ARGS__); \
  ^~~
>> lib/test_user_copy.c:77:11: note: in expansion of macro 'test'
   ret |= test(retval != expected,
  ^~~~

vim +/pr_warn +38 lib/test_user_copy.c

33  
34  #define test(condition, msg, ...)   
\
35  ({  
\
36  int cond = (condition); 
\
37  if (cond)   
\
  > 38  pr_warn("[%d] " msg "\n", __LINE__, ##__VA_ARGS__); 
\
39  cond;   
\
40  })
41  
42  static int test_is_zeroed_user(char *kmem, char __user *umem, size_t 
size)
43  {
44  int ret = 0;
45  size_t start, end, i;
46  size_t zero_start = size / 4;
47  size_t zero_end = size - zero_start;
48  
49  /*
50   * We conduct a series of is_zeroed_user() tests on a block of 
memory
51   * with the following byte-pattern (trying every possible 
[start,end]
52   * pair):
53   *
54   *   [ 00 ff 00 ff ... 00 00 00 00 ... ff 00 ff 00 ]
55   *
56   * And we verify that is_zeroed_user() acts identically to 
memchr_inv().
57   */
58  
59  for (i = 0; i < zero_start; i += 2)
60  kmem[i] = 0x00;
61  for (i = 1; i < zero_start; i += 2)
62  kmem[i] = 0xff;
63  
64  for (i = zero_end; i < size; i += 2)
65

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

2019-09-25 Thread Christian Brauner
On Thu, Sep 26, 2019 at 01:03:29AM +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 to operate on one another in most cases).
> 
> While this interface exists for communication in both directions, only
> one interface is straightforward to have reasonable semantics for
> (userspace passing a struct to the kernel). For kernel returns to
> userspace, what the correct semantics are (whether there should be an
> error if userspace is unaware of a new extension) is very
> syscall-dependent and thus probably cannot be unified between syscalls
> (a good example of this problem is [1]).
> 
> Previously there was no common lib/ function that implemented
> the necessary extension-checking semantics (and different syscalls
> implemented them slightly differently or incompletely[2]). Future
> patches replace common uses of this pattern to make use of
> copy_struct_from_user().
> 
> Some in-kernel selftests that insure that the handling of alignment and
> various byte patterns are all handled identically to memchr_inv() usage.
> 
> [1]: commit 1251201c0d34 ("sched/core: Fix uclamp ABI bug, clean up and
>  robustify sched_read_attr() ABI logic and code")
> 
> [2]: For instance {sched_setattr,perf_event_open,clone3}(2) all do do
>  similar checks to copy_struct_from_user() while rt_sigprocmask(2)
>  always rejects differently-sized struct arguments.
> 
> Suggested-by: Rasmus Villemoes 
> Signed-off-by: Aleksa Sarai 
> ---
>  include/linux/bitops.h  |   7 +++
>  include/linux/uaccess.h |   4 ++
>  lib/strnlen_user.c  |   8 +--
>  lib/test_user_copy.c|  59 ++---
>  lib/usercopy.c  | 115 
>  5 files changed, 180 insertions(+), 13 deletions(-)
> 
> diff --git a/include/linux/bitops.h b/include/linux/bitops.h
> index cf074bce3eb3..a23f4c054768 100644
> --- a/include/linux/bitops.h
> +++ b/include/linux/bitops.h
> @@ -4,6 +4,13 @@
>  #include 
>  #include 
>  
> +/* Set bits in the first 'n' bytes when loaded from memory */
> +#ifdef __LITTLE_ENDIAN
> +#  define aligned_byte_mask(n) ((1ul << 8*(n))-1)
> +#else
> +#  define aligned_byte_mask(n) (~0xfful << (BITS_PER_LONG - 8 - 8*(n)))
> +#endif
> +
>  #define BITS_PER_TYPE(type) (sizeof(type) * BITS_PER_BYTE)
>  #define BITS_TO_LONGS(nr)DIV_ROUND_UP(nr, BITS_PER_TYPE(long))
>  
> diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
> index 34a038563d97..824569e309e4 100644
> --- a/include/linux/uaccess.h
> +++ b/include/linux/uaccess.h
> @@ -230,6 +230,10 @@ static inline unsigned long 
> __copy_from_user_inatomic_nocache(void *to,
>  
>  #endif   /* ARCH_HAS_NOCACHE_UACCESS */
>  
> +extern int is_zeroed_user(const void __user *from, size_t count);
> +extern int copy_struct_from_user(void *dst, size_t ksize,
> +  const void __user *src, size_t usize);
> +
>  /*
>   * probe_kernel_read(): safely attempt to read from a location
>   * @dst: pointer to the buffer that shall take the data
> diff --git a/lib/strnlen_user.c b/lib/strnlen_user.c
> index 7f2db3fe311f..39d588aaa8cd 100644
> --- a/lib/strnlen_user.c
> +++ b/lib/strnlen_user.c
> @@ -2,16 +2,10 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  
> -/* Set bits in the first 'n' bytes when loaded from memory */
> -#ifdef __LITTLE_ENDIAN
> -#  define aligned_byte_mask(n) ((1ul << 8*(n))-1)
> -#else
> -#  define aligned_byte_mask(n) (~0xfful << (BITS_PER_LONG - 8 - 8*(n)))
> -#endif
> -
>  /*
>   * Do a strnlen, return length of string *with* final '\0'.
>   * 'count' is the user-supplied count, while 'max' is the
> diff --git a/lib/test_user_copy.c b/lib/test_user_copy.c
> index 67bcd5dfd847..f7cde3845ccc 100644
> --- a/lib/test_user_copy.c
> +++ b/lib/test_user_copy.c
> @@ -31,14 +31,58 @@
>  # define TEST_U64
>  #endif
>  
> -#define test(condition, msg) \
> -({   \
> - int cond = (condition); \
> - if (cond)   \
> - pr_warn("%s\n", msg);   \
> - cond;   \
> +#define test(condition, msg, ...)\
> +({   \
> + int cond = (condition); \
> + if (cond)   \
> + pr_warn("[%d] " msg "\n", __LINE__, ##__VA_ARGS__); \
> + cond;   \
>  })
>  
> +static int test_is_zeroed_user(char *kmem, char __user *umem, size_t size)
> +{
> + int ret = 0;
> + size_t start, end, i;
> + size_t zero_start = size / 4;
> + size_t zero_end = size - zero_start;
> +
> + /*
> +  * 

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

2019-09-25 Thread Aleksa Sarai
(Damn, I forgot to add Kees to Cc.)

On 2019-09-26, 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 to operate on one another in most cases).
> 
> While this interface exists for communication in both directions, only
> one interface is straightforward to have reasonable semantics for
> (userspace passing a struct to the kernel). For kernel returns to
> userspace, what the correct semantics are (whether there should be an
> error if userspace is unaware of a new extension) is very
> syscall-dependent and thus probably cannot be unified between syscalls
> (a good example of this problem is [1]).
> 
> Previously there was no common lib/ function that implemented
> the necessary extension-checking semantics (and different syscalls
> implemented them slightly differently or incompletely[2]). Future
> patches replace common uses of this pattern to make use of
> copy_struct_from_user().
> 
> Some in-kernel selftests that insure that the handling of alignment and
> various byte patterns are all handled identically to memchr_inv() usage.
> 
> [1]: commit 1251201c0d34 ("sched/core: Fix uclamp ABI bug, clean up and
>  robustify sched_read_attr() ABI logic and code")
> 
> [2]: For instance {sched_setattr,perf_event_open,clone3}(2) all do do
>  similar checks to copy_struct_from_user() while rt_sigprocmask(2)
>  always rejects differently-sized struct arguments.
> 
> Suggested-by: Rasmus Villemoes 
> Signed-off-by: Aleksa Sarai 
> ---
>  include/linux/bitops.h  |   7 +++
>  include/linux/uaccess.h |   4 ++
>  lib/strnlen_user.c  |   8 +--
>  lib/test_user_copy.c|  59 ++---
>  lib/usercopy.c  | 115 
>  5 files changed, 180 insertions(+), 13 deletions(-)
> 
> diff --git a/include/linux/bitops.h b/include/linux/bitops.h
> index cf074bce3eb3..a23f4c054768 100644
> --- a/include/linux/bitops.h
> +++ b/include/linux/bitops.h
> @@ -4,6 +4,13 @@
>  #include 
>  #include 
>  
> +/* Set bits in the first 'n' bytes when loaded from memory */
> +#ifdef __LITTLE_ENDIAN
> +#  define aligned_byte_mask(n) ((1ul << 8*(n))-1)
> +#else
> +#  define aligned_byte_mask(n) (~0xfful << (BITS_PER_LONG - 8 - 8*(n)))
> +#endif
> +
>  #define BITS_PER_TYPE(type) (sizeof(type) * BITS_PER_BYTE)
>  #define BITS_TO_LONGS(nr)DIV_ROUND_UP(nr, BITS_PER_TYPE(long))
>  
> diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
> index 34a038563d97..824569e309e4 100644
> --- a/include/linux/uaccess.h
> +++ b/include/linux/uaccess.h
> @@ -230,6 +230,10 @@ static inline unsigned long 
> __copy_from_user_inatomic_nocache(void *to,
>  
>  #endif   /* ARCH_HAS_NOCACHE_UACCESS */
>  
> +extern int is_zeroed_user(const void __user *from, size_t count);
> +extern int copy_struct_from_user(void *dst, size_t ksize,
> +  const void __user *src, size_t usize);
> +
>  /*
>   * probe_kernel_read(): safely attempt to read from a location
>   * @dst: pointer to the buffer that shall take the data
> diff --git a/lib/strnlen_user.c b/lib/strnlen_user.c
> index 7f2db3fe311f..39d588aaa8cd 100644
> --- a/lib/strnlen_user.c
> +++ b/lib/strnlen_user.c
> @@ -2,16 +2,10 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  
> -/* Set bits in the first 'n' bytes when loaded from memory */
> -#ifdef __LITTLE_ENDIAN
> -#  define aligned_byte_mask(n) ((1ul << 8*(n))-1)
> -#else
> -#  define aligned_byte_mask(n) (~0xfful << (BITS_PER_LONG - 8 - 8*(n)))
> -#endif
> -
>  /*
>   * Do a strnlen, return length of string *with* final '\0'.
>   * 'count' is the user-supplied count, while 'max' is the
> diff --git a/lib/test_user_copy.c b/lib/test_user_copy.c
> index 67bcd5dfd847..f7cde3845ccc 100644
> --- a/lib/test_user_copy.c
> +++ b/lib/test_user_copy.c
> @@ -31,14 +31,58 @@
>  # define TEST_U64
>  #endif
>  
> -#define test(condition, msg) \
> -({   \
> - int cond = (condition); \
> - if (cond)   \
> - pr_warn("%s\n", msg);   \
> - cond;   \
> +#define test(condition, msg, ...)\
> +({   \
> + int cond = (condition); \
> + if (cond)   \
> + pr_warn("[%d] " msg "\n", __LINE__, ##__VA_ARGS__); \
> + cond;   \
>  })
>  
> +static int test_is_zeroed_user(char *kmem, char __user *umem, size_t size)
> +{
> + int ret = 0;
> + size_t start, end, i;
> + size_t zero_start = size / 4;
> + size_t zero_end = size - zero_start;
> +
> + /*

[PATCH v2 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 for communication in both directions, only
one interface is straightforward to have reasonable semantics for
(userspace passing a struct to the kernel). For kernel returns to
userspace, what the correct semantics are (whether there should be an
error if userspace is unaware of a new extension) is very
syscall-dependent and thus probably cannot be unified between syscalls
(a good example of this problem is [1]).

Previously there was no common lib/ function that implemented
the necessary extension-checking semantics (and different syscalls
implemented them slightly differently or incompletely[2]). Future
patches replace common uses of this pattern to make use of
copy_struct_from_user().

Some in-kernel selftests that insure that the handling of alignment and
various byte patterns are all handled identically to memchr_inv() usage.

[1]: commit 1251201c0d34 ("sched/core: Fix uclamp ABI bug, clean up and
 robustify sched_read_attr() ABI logic and code")

[2]: For instance {sched_setattr,perf_event_open,clone3}(2) all do do
 similar checks to copy_struct_from_user() while rt_sigprocmask(2)
 always rejects differently-sized struct arguments.

Suggested-by: Rasmus Villemoes 
Signed-off-by: Aleksa Sarai 
---
 include/linux/bitops.h  |   7 +++
 include/linux/uaccess.h |   4 ++
 lib/strnlen_user.c  |   8 +--
 lib/test_user_copy.c|  59 ++---
 lib/usercopy.c  | 115 
 5 files changed, 180 insertions(+), 13 deletions(-)

diff --git a/include/linux/bitops.h b/include/linux/bitops.h
index cf074bce3eb3..a23f4c054768 100644
--- a/include/linux/bitops.h
+++ b/include/linux/bitops.h
@@ -4,6 +4,13 @@
 #include 
 #include 
 
+/* Set bits in the first 'n' bytes when loaded from memory */
+#ifdef __LITTLE_ENDIAN
+#  define aligned_byte_mask(n) ((1ul << 8*(n))-1)
+#else
+#  define aligned_byte_mask(n) (~0xfful << (BITS_PER_LONG - 8 - 8*(n)))
+#endif
+
 #define BITS_PER_TYPE(type) (sizeof(type) * BITS_PER_BYTE)
 #define BITS_TO_LONGS(nr)  DIV_ROUND_UP(nr, BITS_PER_TYPE(long))
 
diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
index 34a038563d97..824569e309e4 100644
--- a/include/linux/uaccess.h
+++ b/include/linux/uaccess.h
@@ -230,6 +230,10 @@ static inline unsigned long 
__copy_from_user_inatomic_nocache(void *to,
 
 #endif /* ARCH_HAS_NOCACHE_UACCESS */
 
+extern int is_zeroed_user(const void __user *from, size_t count);
+extern int copy_struct_from_user(void *dst, size_t ksize,
+const void __user *src, size_t usize);
+
 /*
  * probe_kernel_read(): safely attempt to read from a location
  * @dst: pointer to the buffer that shall take the data
diff --git a/lib/strnlen_user.c b/lib/strnlen_user.c
index 7f2db3fe311f..39d588aaa8cd 100644
--- a/lib/strnlen_user.c
+++ b/lib/strnlen_user.c
@@ -2,16 +2,10 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 
-/* Set bits in the first 'n' bytes when loaded from memory */
-#ifdef __LITTLE_ENDIAN
-#  define aligned_byte_mask(n) ((1ul << 8*(n))-1)
-#else
-#  define aligned_byte_mask(n) (~0xfful << (BITS_PER_LONG - 8 - 8*(n)))
-#endif
-
 /*
  * Do a strnlen, return length of string *with* final '\0'.
  * 'count' is the user-supplied count, while 'max' is the
diff --git a/lib/test_user_copy.c b/lib/test_user_copy.c
index 67bcd5dfd847..f7cde3845ccc 100644
--- a/lib/test_user_copy.c
+++ b/lib/test_user_copy.c
@@ -31,14 +31,58 @@
 # define TEST_U64
 #endif
 
-#define test(condition, msg)   \
-({ \
-   int cond = (condition); \
-   if (cond)   \
-   pr_warn("%s\n", msg);   \
-   cond;   \
+#define test(condition, msg, ...)  \
+({ \
+   int cond = (condition); \
+   if (cond)   \
+   pr_warn("[%d] " msg "\n", __LINE__, ##__VA_ARGS__); \
+   cond;   \
 })
 
+static int test_is_zeroed_user(char *kmem, char __user *umem, size_t size)
+{
+   int ret = 0;
+   size_t start, end, i;
+   size_t zero_start = size / 4;
+   size_t zero_end = size - zero_start;
+
+   /*
+* We conduct a series of is_zeroed_user() tests on a block of memory
+* with the following byte-pattern (trying every possible [start,end]
+* pair):
+*
+*   [ 00 ff 00 ff ... 00 00 00 00 ... ff 00 ff 00 ]
+*
+* And we verify that is_z