netdev - Re: [PATCH] net/mlx4: Fix build errors with gcc 10.3.1

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <ccea3237-b592-4248-b63a-796f4889204d@gmail.com>
Date: Mon, 28 Oct 2024 11:18:14 +0200
From: Tariq Toukan <ttoukan.linux@...il.com>
To: Li Huafei <lihuafei1@...wei.com>, tariqt@...dia.com,
 andrew+netdev@...n.ch, davem@...emloft.net, edumazet@...gle.com,
 kuba@...nel.org, pabeni@...hat.com, gustavoars@...nel.org
Cc: netdev@...r.kernel.org, linux-rdma@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH] net/mlx4: Fix build errors with gcc 10.3.1



On 26/10/2024 13:02, Li Huafei wrote:
> When compiling the kernel in my environment (with gcc version gcc
> 10.3.1), I encountered the following compilation check error:
> 
>   In function ‘check_copy_size’,
>       inlined from ‘copy_to_user’ at ./include/linux/uaccess.h:210:7,
>       inlined from ‘mlx4_init_user_cqes’ at drivers/net/ethernet/mellanox/mlx4/cq.c:317:9:
>   ./include/linux/thread_info.h:244:4: error: call to ‘__bad_copy_from’ declared with attribute error: copy source size is too small
>     244 |    __bad_copy_from();
> 
> mlx4_init_user_cqes() checks the size of the buf before copying data,
> ensuring that there will be no out-of-bounds copying, so this should be
> a false positive. I searched the git commit history and found that the
> commit 75da0eba0a47 ("rapidio: avoid bogus __alloc_size warning") fixed
> a similar issue, where the compiler encountered an error when expanding
> the arguments of check_copy_size().  Saving the result of array_size()
> to a temporary variable and using this variable as the argument of
> copy_to_user() can avoid this gcc warning.
> 
> Additionally, I tested older (9.4.0) and newer (10.3.5) versions and did
> not encounter the same problem, so this should be a bug in a specific
> intermediate version.
> 
> Signed-off-by: Li Huafei <lihuafei1@...wei.com>
> ---
>   drivers/net/ethernet/mellanox/mlx4/cq.c | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx4/cq.c b/drivers/net/ethernet/mellanox/mlx4/cq.c
> index e130e7259275..5169c7a4097b 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/cq.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/cq.c
> @@ -293,6 +293,7 @@ static int mlx4_init_user_cqes(void *buf, int entries, int cqe_size)
>   	void *init_ents;
>   	int err = 0;
>   	int i;
> +	size_t size = array_size(entries, cqe_size);
>   
>   	init_ents = kmalloc(PAGE_SIZE, GFP_KERNEL);
>   	if (!init_ents)
> @@ -314,9 +315,7 @@ static int mlx4_init_user_cqes(void *buf, int entries, int cqe_size)
>   			buf += PAGE_SIZE;
>   		}
>   	} else {
> -		err = copy_to_user((void __user *)buf, init_ents,
> -				   array_size(entries, cqe_size)) ?
> -			-EFAULT : 0;
> +		err = copy_to_user((void __user *)buf, init_ents, size) ? -EFAULT : 0;
>   	}
>   
>   out:


As you mention, the bug is in the compiler, in a very specific 
intermediate version.
Why would you modify the driver then?