netdev - Re: [PATCH 1/2] net/mlx4: fix build error from usercopy size check

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: <Z6sBEoUB02Q8paEu@U-V2QX163P-2032.local>
Date: Tue, 11 Feb 2025 15:49:38 +0800
From: YinFengwei <fengwei_yin@...ux.alibaba.com>
To: Arnd Bergmann <arnd@...nel.org>
Cc: Tariq Toukan <tariqt@...dia.com>,
	"Gustavo A. R. Silva" <gustavoars@...nel.org>,
	Arnd Bergmann <arnd@...db.de>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	netdev@...r.kernel.org, linux-rdma@...r.kernel.org,
	linux-kernel@...r.kernel.org, fengwei_yin@...ux.alibaba.com
Subject: Re: [PATCH 1/2] net/mlx4: fix build error from usercopy size check

Hi Arnd,

On Tue, Apr 18, 2023 at 01:47:11PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@...db.de>
> 
> The array_size() helper is used here to prevent accidental overflow in
> mlx4_init_user_cqes(), but as this returns SIZE_MAX in case an overflow
> would happen, the logic in copy_to_user() now detects that as overflowing
> the source:
> 
> In file included from arch/x86/include/asm/preempt.h:9,
>                  from include/linux/preempt.h:78,
>                  from include/linux/percpu.h:6,
>                  from include/linux/context_tracking_state.h:5,
>                  from include/linux/hardirq.h:5,
>                  from drivers/net/ethernet/mellanox/mlx4/cq.c:37:
> In function 'check_copy_size',
>     inlined from 'copy_to_user' at include/linux/uaccess.h:190:6,
>     inlined from 'mlx4_init_user_cqes' at drivers/net/ethernet/mellanox/mlx4/cq.c:317:9,
>     inlined from 'mlx4_cq_alloc' at drivers/net/ethernet/mellanox/mlx4/cq.c:394:10:
> 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();
>       |    ^~~~~~~~~~~~~~~~~
> 
> Move the size logic out, and instead use the same size value for the
> comparison and the copy.
I could hit this build error with latest upstream kernel tree with
gcc 10.2.1 20200825 for arm64 platform. No such build error for x86
platform.

This patch can fix the build error. I am wondering whether you will
push this fix to upstream kernel. You can add:
  Tested-by: Yin Fengwei <fengwei_fyin@...ux.alibaba.com>


Regards
Yin, Fengwei

> 
> Fixes: f69bf5dee7ef ("net/mlx4: Use array_size() helper in copy_to_user()")
> Signed-off-by: Arnd Bergmann <arnd@...db.de>
> ---
>  drivers/net/ethernet/mellanox/mlx4/cq.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx4/cq.c b/drivers/net/ethernet/mellanox/mlx4/cq.c
> index 4d4f9cf9facb..020cb8e2883f 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/cq.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/cq.c
> @@ -290,6 +290,7 @@ static void mlx4_cq_free_icm(struct mlx4_dev *dev, int cqn)
>  static int mlx4_init_user_cqes(void *buf, int entries, int cqe_size)
>  {
>  	int entries_per_copy = PAGE_SIZE / cqe_size;
> +	size_t copy_size = array_size(entries, cqe_size);
>  	void *init_ents;
>  	int err = 0;
>  	int i;
> @@ -304,7 +305,7 @@ static int mlx4_init_user_cqes(void *buf, int entries, int cqe_size)
>  	 */
>  	memset(init_ents, 0xcc, PAGE_SIZE);
>  
> -	if (entries_per_copy < entries) {
> +	if (copy_size > PAGE_SIZE) {
>  		for (i = 0; i < entries / entries_per_copy; i++) {
>  			err = copy_to_user((void __user *)buf, init_ents, PAGE_SIZE) ?
>  				-EFAULT : 0;
> @@ -315,7 +316,7 @@ static int mlx4_init_user_cqes(void *buf, int entries, int cqe_size)
>  		}
>  	} else {
>  		err = copy_to_user((void __user *)buf, init_ents,
> -				   array_size(entries, cqe_size)) ?
> +				   copy_size) ?
>  			-EFAULT : 0;
>  	}
>  
> -- 
> 2.39.2
>