Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

rapidio: avoid bogus __alloc_size warning

Patch series "Add __alloc_size()", v3.

GCC and Clang both use the "alloc_size" attribute to assist with bounds
checking around the use of allocation functions. Add the attribute,
adjust the Makefile to silence needless warnings, and add the hints to
the allocators where possible. These changes have been in use for a
while now in GrapheneOS.

This patch (of 8):

After adding __alloc_size attributes to the allocators, GCC 9.3 (but not
later) may incorrectly evaluate the arguments to check_copy_size(),
getting seemingly confused by the size being returned from array_size().
Instead, perform the calculation once, which both makes the code more
readable and avoids the bug in GCC.

In file included from arch/x86/include/asm/preempt.h:7,
from include/linux/preempt.h:78,
from include/linux/spinlock.h:55,
from include/linux/mm_types.h:9,
from include/linux/buildid.h:5,
from include/linux/module.h:14,
from drivers/rapidio/devices/rio_mport_cdev.c:13:
In function 'check_copy_size',
inlined from 'copy_from_user' at include/linux/uaccess.h:191:6,
inlined from 'rio_mport_transfer_ioctl' at drivers/rapidio/devices/rio_mport_cdev.c:983:6:
include/linux/thread_info.h:213:4: error: call to '__bad_copy_to' declared with attribute error: copy destination size is too small
213 | __bad_copy_to();
| ^~~~~~~~~~~~~~~

But the allocation size and the copy size are identical:

transfer = vmalloc(array_size(sizeof(*transfer), transaction.count));
if (!transfer)
return -ENOMEM;

if (unlikely(copy_from_user(transfer,
(void __user *)(uintptr_t)transaction.block,
array_size(sizeof(*transfer), transaction.count)))) {

Link: https://lkml.kernel.org/r/20210930222704.2631604-1-keescook@chromium.org
Link: https://lkml.kernel.org/r/20210930222704.2631604-2-keescook@chromium.org
Link: https://lore.kernel.org/linux-mm/202109091134.FHnRmRxu-lkp@intel.com/
Signed-off-by: Kees Cook <keescook@chromium.org>
Reviewed-by: John Hubbard <jhubbard@nvidia.com>
Reported-by: kernel test robot <lkp@intel.com>
Cc: Matt Porter <mporter@kernel.crashing.org>
Cc: Alexandre Bounine <alex.bou9@gmail.com>
Cc: Jing Xiangfeng <jingxiangfeng@huawei.com>
Cc: Ira Weiny <ira.weiny@intel.com>
Cc: Souptick Joarder <jrdr.linux@gmail.com>
Cc: Gustavo A. R. Silva <gustavoars@kernel.org>
Cc: Andy Whitcroft <apw@canonical.com>
Cc: Christoph Lameter <cl@linux.com>
Cc: Daniel Micay <danielmicay@gmail.com>
Cc: David Rientjes <rientjes@google.com>
Cc: Dennis Zhou <dennis@kernel.org>
Cc: Dwaipayan Ray <dwaipayanray1@gmail.com>
Cc: Joe Perches <joe@perches.com>
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Lukas Bulwahn <lukas.bulwahn@gmail.com>
Cc: Miguel Ojeda <ojeda@kernel.org>
Cc: Nathan Chancellor <nathan@kernel.org>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: Randy Dunlap <rdunlap@infradead.org>
Cc: Tejun Heo <tj@kernel.org>
Cc: Vlastimil Babka <vbabka@suse.cz>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

authored by

Kees Cook and committed by
Linus Torvalds
75da0eba d73dad4e

+5 -4
+5 -4
drivers/rapidio/devices/rio_mport_cdev.c
··· 965 965 struct rio_transfer_io *transfer; 966 966 enum dma_data_direction dir; 967 967 int i, ret = 0; 968 + size_t size; 968 969 969 970 if (unlikely(copy_from_user(&transaction, arg, sizeof(transaction)))) 970 971 return -EFAULT; ··· 977 976 priv->md->properties.transfer_mode) == 0) 978 977 return -ENODEV; 979 978 980 - transfer = vmalloc(array_size(sizeof(*transfer), transaction.count)); 979 + size = array_size(sizeof(*transfer), transaction.count); 980 + transfer = vmalloc(size); 981 981 if (!transfer) 982 982 return -ENOMEM; 983 983 984 984 if (unlikely(copy_from_user(transfer, 985 985 (void __user *)(uintptr_t)transaction.block, 986 - array_size(sizeof(*transfer), transaction.count)))) { 986 + size))) { 987 987 ret = -EFAULT; 988 988 goto out_free; 989 989 } ··· 996 994 transaction.sync, dir, &transfer[i]); 997 995 998 996 if (unlikely(copy_to_user((void __user *)(uintptr_t)transaction.block, 999 - transfer, 1000 - array_size(sizeof(*transfer), transaction.count)))) 997 + transfer, size))) 1001 998 ret = -EFAULT; 1002 999 1003 1000 out_free: