Message-Id: <6a9d6c05-b232-49f2-a269-74ebe249f961@app.fastmail.com>
Date:   Fri, 03 Feb 2023 23:01:09 +0100
From:   "Arnd Bergmann" <arnd@...db.de>
To:     "Kees Cook" <keescook@...omium.org>
Cc:     "Aleksa Sarai" <cyphar@...har.com>,
        "Christian Brauner" <christian.brauner@...ntu.com>,
        "Rasmus Villemoes" <linux@...musvillemoes.dk>,
        "Dinh Nguyen" <dinguyen@...nel.org>,
        "Catalin Marinas" <catalin.marinas@....com>,
        "Andrew Morton" <akpm@...ux-foundation.org>,
        "Geert Uytterhoeven" <geert@...ux-m68k.org>,
        "Alexander Potapenko" <glider@...gle.com>,
        "Christian Brauner" <brauner@...nel.org>,
        "Stafford Horne" <shorne@...il.com>,
        "Alexander Viro" <viro@...iv.linux.org.uk>,
        "Christophe Leroy" <christophe.leroy@...roup.eu>,
        linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org
Subject: Re: [PATCH] uaccess: Add minimum bounds check on kernel buffer size

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

Powered by blists - more mailing lists