[PATCH] uaccess: Add minimum bounds check on kernel buffer size

[PATCH] uaccess: Add minimum bounds check on kernel buffer size

[Kees Cook]

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: Aleksa Sarai <cyphar@cyphar.com>
Cc: Christian Brauner <christian.brauner@ubuntu.com>
Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Dinh Nguyen <dinguyen@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Alexander Potapenko <glider@google.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 include/linux/uaccess.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
index afb18f198843..ab9728138ad6 100644
--- a/include/linux/uaccess.h
+++ b/include/linux/uaccess.h
@@ -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);
-- 
2.34.1

Re: [PATCH] uaccess: Add minimum bounds check on kernel buffer size

[Arnd Bergmann]

On Fri, Feb 3, 2023, at 20:35, Kees Cook wrote:
> 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;
>       |                           ^~~
>

Hi Kees,

I started building with gcc-13.0.1 myself but ran into a lot of
other -Warray-bounds warnings in randconfig builds, so I ended up
turning it off once more with CONFIG_CC_NO_ARRAY_BOUNDS in order
to keep building without warnings.

Is there anything else I need to do to get to the point of
just addressing actual issues instead of false positives?
Do you already have a patch series for fixing the others?

> diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
> index afb18f198843..ab9728138ad6 100644
> --- a/include/linux/uaccess.h
> +++ b/include/linux/uaccess.h
> @@ -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;
> +

WARN_ON_ONCE() may be a little expensive since that adds two
comparisons and a static variable to each copy, but it's probably
fine. 

      Arnd

Re: [PATCH] uaccess: Add minimum bounds check on kernel buffer size

[Arnd Bergmann]

On Fri, Feb 3, 2023, at 22:23, Arnd Bergmann wrote:
> On Fri, Feb 3, 2023, at 20:35, Kees Cook wrote:
>> 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;
>>       |                           ^~~
>>
>
> Hi Kees,
>
> I started building with gcc-13.0.1 myself but ran into a lot of
> other -Warray-bounds warnings in randconfig builds, so I ended up
> turning it off once more with CONFIG_CC_NO_ARRAY_BOUNDS in order
> to keep building without warnings.
>
> Is there anything else I need to do to get to the point of
> just addressing actual issues instead of false positives?
> Do you already have a patch series for fixing the others?

For reference, I just tried it out again and within half an hour I found
about 30 unique warnings, which is way less than I had remembered,
though some of these just show up a lot:

arch/arm/include/asm/io.h:113:9: error: array subscript 0 is outside array bounds of 'void[0]' [-Werror=array-bounds=]
arch/arm/include/asm/io.h:95:9: error: array subscript 0 is outside array bounds of 'void[0]' [-Werror=array-bounds=]
arch/arm/include/asm/memory.h:311:22: error: array subscript -1 is outside array bounds of 'char[2147483647]' [-Werror=array-bounds=]
arch/arm64/kernel/setup.c:228:56: error: array subscript -1 is outside array bounds of 'char[]' [-Werror=array-bounds=]
arch/x86/include/asm/io.h:56:3: error: array subscript 0 is outside array bounds of 'void[0]' [-Werror=array-bounds=]
drivers/cpufreq/brcmstb-avs-cpufreq.c:449:28: error: array subscript 5 is outside array bounds of 'void[60]' [-Werror=array-bounds=]
drivers/dma/dw-axi-dmac/dw-axi-dmac-platform.c:329:46: error: array subscript 0 is outside array bounds of 'struct virt_dma_desc[24403223]' [-Werror=array-bounds=]
drivers/gpu/drm/nouveau/nvif/outp.c:140:9: error: array subscript 'unsigned char[16][0]' is partly outside array bounds of 'u8[15]' {aka 'unsigned char[15]'} [-Werror=array-bounds=]
drivers/hwmon/lm85.c:1110:26: error: array subscript [0, 31] is outside array bounds of 'struct lm85_autofan[3]' [-Werror=array-bounds=]
drivers/infiniband/core/user_mad.c:564:50: error: array subscript 'struct ib_rmpp_mad[0]' is partly outside array bounds of 'unsigned char[140]' [-Werror=array-bounds=]
drivers/infiniband/core/user_mad.c:566:42: error: array subscript 'struct ib_rmpp_mad[0]' is partly outside array bounds of 'unsigned char[140]' [-Werror=array-bounds=]
drivers/infiniband/core/user_mad.c:618:25: error: array subscript 'struct ib_rmpp_mad[0]' is partly outside array bounds of 'unsigned char[140]' [-Werror=array-bounds=]
drivers/infiniband/core/user_mad.c:622:44: error: array subscript 'struct ib_rmpp_mad[0]' is partly outside array bounds of 'unsigned char[140]' [-Werror=array-bounds=]
drivers/media/tuners/mxl5005s.c:3429:72: error: array subscript 32 is above array bounds of 'u16[25]' {aka 'short unsigned int[25]'} [-Werror=array-bounds=]
drivers/media/tuners/mxl5005s.c:3448:70: error: array subscript 32 is above array bounds of 'u16[25]' {aka 'short unsigned int[25]'} [-Werror=array-bounds=]
drivers/spi/spi-imx.c:1687:34: error: array subscript 0 is outside array bounds of 'const bool[0]' {aka 'const _Bool[]'} [-Werror=array-bounds=]
drivers/staging/rtl8712/rtl871x_xmit.c:949:40: error: array subscript 4 is outside array bounds of 'void[448]' [-Werror=array-bounds=]
drivers/staging/rtl8712/rtl871x_xmit.c:950:39: error: array subscript 4 is outside array bounds of 'void[448]' [-Werror=array-bounds=]
drivers/thermal/hisi_thermal.c:430:21: error: array subscript 1 is outside array bounds of 'void[20]' [-Werror=array-bounds=]
drivers/thermal/hisi_thermal.c:431:21: error: array subscript 1 is outside array bounds of 'void[20]' [-Werror=array-bounds=]
drivers/thermal/hisi_thermal.c:432:21: error: array subscript 1 is outside array bounds of 'void[20]' [-Werror=array-bounds=]
drivers/usb/host/xhci-mvebu.c:37:28: error: array subscript [0, 89478485] is outside array bounds of 'const struct mbus_dram_window[0]' [-Werror=array-bounds=]
drivers/usb/storage/ene_ub6250.c:1050:44: error: array subscript 'struct ms_bootblock_idi[0]' is partly outside array bounds of 'unsigned char[512]' [-Werror=array-bounds=]
fs/btrfs/sysfs.c:636:13: error: array subscript -18 is outside array bounds of 'struct kobject[59652323]' [-Werror=array-bounds=]
fs/crypto/fname.c:506:33: error: array subscript 'struct fscrypt_nokey_name[0]' is partly outside array bounds of 'unsigned char[189]' [-Werror=array-bounds=]
include/linux/fortify-string.h:25:34: error: array subscript 0 is outside array bounds of 'const char[0]' [-Werror=array-bounds=]
include/linux/fortify-string.h:57:33: error: array subscript 'unsigned char[16][0]' is partly outside array bounds of 'u8[15]' {aka 'unsigned char[15]'} [-Werror=array-bounds=]
net/socket.c:651:21: error: array subscript -1 is outside array bounds of 'struct inode[3016128]' [-Werror=array-bounds=]
net/socket.c:652:26: error: array subscript -1 is outside array bounds of 'struct inode[3016128]' [-Werror=array-bounds=]

If you like, I can run this for a while longer to get to something close to
a complete list.

      Arnd

Re: [PATCH] uaccess: Add minimum bounds check on kernel buffer size

[Kees Cook]

On Fri, Feb 03, 2023 at 10:23:13PM +0100, Arnd Bergmann wrote:
> On Fri, Feb 3, 2023, at 20:35, Kees Cook wrote:
> > 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;
> >       |                           ^~~
> >
> 
> Hi Kees,
> 
> I started building with gcc-13.0.1 myself but ran into a lot of
> other -Warray-bounds warnings in randconfig builds, so I ended up
> turning it off once more with CONFIG_CC_NO_ARRAY_BOUNDS in order
> to keep building without warnings.

Understood. AFAIK, all the open bugs I (and you) filed with GCC 13 have
been fixed related to -Warray-bounds. The most recent was the misbehavior
between CONFIG_UBSAN_SHIFT and -Warray-bounds. (Though the shift checking
still exposes some warnings since it introduces an implicit bounds check
on the shift variable, but they're not _wrong_ any more.)

> Is there anything else I need to do to get to the point of
> just addressing actual issues instead of false positives?
> Do you already have a patch series for fixing the others?

I've been working through the list that I see when building with
-Warray-bounds and -fstrict-flex-arrays=3. Some are real bugs, as usual.

> > diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
> > index afb18f198843..ab9728138ad6 100644
> > --- a/include/linux/uaccess.h
> > +++ b/include/linux/uaccess.h
> > @@ -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;
> > +
> 
> WARN_ON_ONCE() may be a little expensive since that adds two
> comparisons and a static variable to each copy, but it's probably
> fine. 

Yeah. IMO, copy_struct_from_user() is not fast path and having better
bounds checking when coming from userspace is well worth it.

-- 
Kees Cook

Re: [PATCH] uaccess: Add minimum bounds check on kernel buffer size

[Geert Uytterhoeven]

Hi Arnd,

On Fri, Feb 3, 2023 at 10:23 PM Arnd Bergmann <arnd@arndb.de> wrote:
> On Fri, Feb 3, 2023, at 20:35, Kees Cook wrote:
> > --- a/include/linux/uaccess.h
> > +++ b/include/linux/uaccess.h
> > @@ -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;
> > +
>
> WARN_ON_ONCE() may be a little expensive since that adds two
> comparisons and a static variable to each copy, but it's probably
> fine.

When seeing this, I was a bit worried about the size increase.
Hence I gave it a try on atari_defconfig and ran bloat-o-meter.
Surprisingly, there was no size increase at all, as all checks
were optimized away.

Hence perhaps this can become a compile-time check?

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] uaccess: Add minimum bounds check on kernel buffer size

[Kees Cook]

On Mon, Feb 06, 2023 at 09:03:19PM +0100, Geert Uytterhoeven wrote:
> Hi Arnd,
> 
> On Fri, Feb 3, 2023 at 10:23 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > On Fri, Feb 3, 2023, at 20:35, Kees Cook wrote:
> > > --- a/include/linux/uaccess.h
> > > +++ b/include/linux/uaccess.h
> > > @@ -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;
> > > +
> >
> > WARN_ON_ONCE() may be a little expensive since that adds two
> > comparisons and a static variable to each copy, but it's probably
> > fine.
> 
> When seeing this, I was a bit worried about the size increase.
> Hence I gave it a try on atari_defconfig and ran bloat-o-meter.
> Surprisingly, there was no size increase at all, as all checks
> were optimized away.
> 
> Hence perhaps this can become a compile-time check?

Normally it should optimize away, yes.

-- 
Kees Cook

Re: [PATCH] uaccess: Add minimum bounds check on kernel buffer size

[Yann Droneaud]

Hi,

Le 06/02/2023 à 21:03, Geert Uytterhoeven a écrit :

> Hi Arnd,
>
> On Fri, Feb 3, 2023 at 10:23 PM Arnd Bergmann <arnd@arndb.de> wrote:
>> On Fri, Feb 3, 2023, at 20:35, Kees Cook wrote:
>>> --- a/include/linux/uaccess.h
>>> +++ b/include/linux/uaccess.h
>>> @@ -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;
>>> +
>> WARN_ON_ONCE() may be a little expensive since that adds two
>> comparisons and a static variable to each copy, but it's probably
>> fine.
> When seeing this, I was a bit worried about the size increase.
> Hence I gave it a try on atari_defconfig and ran bloat-o-meter.
> Surprisingly, there was no size increase at all, as all checks
> were optimized away.
>
> Hence perhaps this can become a compile-time check?

It should be a compile-time check, because one would not want __builtin_object_size(dst, 1) to return -1 if dst' size is not known at compile-time.

Regards.

-- 
Yann Droneaud
OPTEYA

Re: [PATCH] uaccess: Add minimum bounds check on kernel buffer size

[Kees Cook]

On Tue, Feb 07, 2023 at 10:06:14AM +0100, Yann Droneaud wrote:
> Hi,
> 
> Le 06/02/2023 à 21:03, Geert Uytterhoeven a écrit :
> 
> > Hi Arnd,
> > 
> > On Fri, Feb 3, 2023 at 10:23 PM Arnd Bergmann <arnd@arndb.de> wrote:
> > > On Fri, Feb 3, 2023, at 20:35, Kees Cook wrote:
> > > > --- a/include/linux/uaccess.h
> > > > +++ b/include/linux/uaccess.h
> > > > @@ -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;
> > > > +
> > > WARN_ON_ONCE() may be a little expensive since that adds two
> > > comparisons and a static variable to each copy, but it's probably
> > > fine.
> > When seeing this, I was a bit worried about the size increase.
> > Hence I gave it a try on atari_defconfig and ran bloat-o-meter.
> > Surprisingly, there was no size increase at all, as all checks
> > were optimized away.
> > 
> > Hence perhaps this can become a compile-time check?
> 
> It should be a compile-time check, because one would not want __builtin_object_size(dst, 1) to return -1 if dst' size is not known at compile-time.

Note that it's size_t, so it's actually SIZE_MAX, which is why these
tests will vanish most of the time. i.e. it cannot ever be possible for
the SIZE_MAX case so the entire test is elided.

And when ksize is known at compile time and __bos is not SIZE_MAX, the
result is also known to be either always true or always false, etc.

What's nice here is that when ksize is only run-time known and the buffer
size is compile-time known, we'll keep the test.

-- 
Kees Cook

Re: [PATCH] uaccess: Add minimum bounds check on kernel buffer size

[Aleksa Sarai]

[-- Attachment #1: Type: text/plain, Size: 3033 bytes --]

On 2023-02-03, Kees Cook <keescook@chromium.org> wrote:
> 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;
>       |                           ^~~

Makes sense.

Acked-by: Aleksa Sarai <cyphar@cyphar.com>

> 
> Cc: Aleksa Sarai <cyphar@cyphar.com>
> Cc: Christian Brauner <christian.brauner@ubuntu.com>
> Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Dinh Nguyen <dinguyen@kernel.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Alexander Potapenko <glider@google.com>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
>  include/linux/uaccess.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
> index afb18f198843..ab9728138ad6 100644
> --- a/include/linux/uaccess.h
> +++ b/include/linux/uaccess.h
> @@ -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);
> -- 
> 2.34.1
> 

-- 
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]