From: Kees Cook <keescook@xxxxxxxxxxxx>
Subject: 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@xxxxxxxxxxxx
Link: https://lkml.kernel.org/r/20210930222704.2631604-2-keescook@xxxxxxxxxxxx
Link: https://lore.kernel.org/linux-mm/202109091134.FHnRmRxu-lkp@xxxxxxxxx/
Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx>
Reviewed-by: John Hubbard <jhubbard@xxxxxxxxxx>
Reported-by: kernel test robot <lkp@xxxxxxxxx>
Cc: Matt Porter <mporter@xxxxxxxxxxxxxxxxxxx>
Cc: Alexandre Bounine <alex.bou9@xxxxxxxxx>
Cc: Jing Xiangfeng <jingxiangfeng@xxxxxxxxxx>
Cc: Ira Weiny <ira.weiny@xxxxxxxxx>
Cc: Souptick Joarder <jrdr.linux@xxxxxxxxx>
Cc: Gustavo A. R. Silva <gustavoars@xxxxxxxxxx>
Cc: Andy Whitcroft <apw@xxxxxxxxxxxxx>
Cc: Christoph Lameter <cl@xxxxxxxxx>
Cc: Daniel Micay <danielmicay@xxxxxxxxx>
Cc: David Rientjes <rientjes@xxxxxxxxxx>
Cc: Dennis Zhou <dennis@xxxxxxxxxx>
Cc: Dwaipayan Ray <dwaipayanray1@xxxxxxxxx>
Cc: Joe Perches <joe@xxxxxxxxxxx>
Cc: Joonsoo Kim <iamjoonsoo.kim@xxxxxxx>
Cc: Lukas Bulwahn <lukas.bulwahn@xxxxxxxxx>
Cc: Miguel Ojeda <ojeda@xxxxxxxxxx>
Cc: Nathan Chancellor <nathan@xxxxxxxxxx>
Cc: Nick Desaulniers <ndesaulniers@xxxxxxxxxx>
Cc: Pekka Enberg <penberg@xxxxxxxxxx>
Cc: Randy Dunlap <rdunlap@xxxxxxxxxxxxx>
Cc: Tejun Heo <tj@xxxxxxxxxx>
Cc: Vlastimil Babka <vbabka@xxxxxxx>
Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
---
drivers/rapidio/devices/rio_mport_cdev.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
--- a/drivers/rapidio/devices/rio_mport_cdev.c~rapidio-avoid-bogus-__alloc_size-warning
+++ a/drivers/rapidio/devices/rio_mport_cdev.c
@@ -965,6 +965,7 @@ static int rio_mport_transfer_ioctl(stru
struct rio_transfer_io *transfer;
enum dma_data_direction dir;
int i, ret = 0;
+ size_t size;
if (unlikely(copy_from_user(&transaction, arg, sizeof(transaction))))
return -EFAULT;
@@ -976,13 +977,14 @@ static int rio_mport_transfer_ioctl(stru
priv->md->properties.transfer_mode) == 0)
return -ENODEV;
- transfer = vmalloc(array_size(sizeof(*transfer), transaction.count));
+ size = array_size(sizeof(*transfer), transaction.count);
+ transfer = vmalloc(size);
if (!transfer)
return -ENOMEM;
if (unlikely(copy_from_user(transfer,
(void __user *)(uintptr_t)transaction.block,
- array_size(sizeof(*transfer), transaction.count)))) {
+ size))) {
ret = -EFAULT;
goto out_free;
}
@@ -994,8 +996,7 @@ static int rio_mport_transfer_ioctl(stru
transaction.sync, dir, &transfer[i]);
if (unlikely(copy_to_user((void __user *)(uintptr_t)transaction.block,
- transfer,
- array_size(sizeof(*transfer), transaction.count))))
+ transfer, size)))
ret = -EFAULT;
out_free:
_
