Skip to content

Commit

Permalink
uaccess: Add minimum bounds check on kernel buffer size
Browse files Browse the repository at this point in the history
[ Upstream commit 04ffde1 ]

While there is logic about the difference between ksize and usize,
copy_struct_from_user() didn't check the size of the destination buffer
(when it was known) against ksize. Add this check so there is an upper
bounds check on the possible memset() call, otherwise lower bounds
checks made by callers will trigger bounds warnings under -Warray-bounds.
Seen under GCC 13:

In function 'copy_struct_from_user',
    inlined from 'iommufd_fops_ioctl' at
../drivers/iommu/iommufd/main.c:333:8:
../include/linux/fortify-string.h:59:33: warning: '__builtin_memset' offset [57, 4294967294] is out of the bounds [0, 56] of object 'buf' with type 'union ucmd_buffer' [-Warray-bounds=]
   59 | #define __underlying_memset     __builtin_memset
      |                                 ^
../include/linux/fortify-string.h:453:9: note: in expansion of macro '__underlying_memset'
  453 |         __underlying_memset(p, c, __fortify_size); \
      |         ^~~~~~~~~~~~~~~~~~~
../include/linux/fortify-string.h:461:25: note: in expansion of macro '__fortify_memset_chk'
  461 | #define memset(p, c, s) __fortify_memset_chk(p, c, s, \
      |                         ^~~~~~~~~~~~~~~~~~~~
../include/linux/uaccess.h:334:17: note: in expansion of macro 'memset'
  334 |                 memset(dst + size, 0, rest);
      |                 ^~~~~~
../drivers/iommu/iommufd/main.c: In function 'iommufd_fops_ioctl':
../drivers/iommu/iommufd/main.c:311:27: note: 'buf' declared here
  311 |         union ucmd_buffer buf;
      |                           ^~~

Cc: Christian Brauner <[email protected]>
Cc: Rasmus Villemoes <[email protected]>
Cc: Arnd Bergmann <[email protected]>
Cc: Dinh Nguyen <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Geert Uytterhoeven <[email protected]>
Cc: Alexander Potapenko <[email protected]>
Acked-by: Aleksa Sarai <[email protected]>
Signed-off-by: Kees Cook <[email protected]>
Link: https://lore.kernel.org/lkml/[email protected]/
Signed-off-by: Sasha Levin <[email protected]>
  • Loading branch information
kees authored and gregkh committed Mar 10, 2023
1 parent f101594 commit 9ff7fc0
Showing 1 changed file with 4 additions and 0 deletions.
4 changes: 4 additions & 0 deletions include/linux/uaccess.h
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,10 @@ copy_struct_from_user(void *dst, size_t ksize, const void __user *src,
size_t size = min(ksize, usize);
size_t rest = max(ksize, usize) - size;

/* Double check if ksize is larger than a known object size. */
if (WARN_ON_ONCE(ksize > __builtin_object_size(dst, 1)))
return -E2BIG;

/* Deal with trailing bytes. */
if (usize < ksize) {
memset(dst + size, 0, rest);
Expand Down

0 comments on commit 9ff7fc0

Please sign in to comment.