Re: [PATCH] sfc: use kmalloc_array() instead of kmalloc()

[Edward Cree <ecree.xilinx@gmail.com>]

On 25/08/2025 16:12, Qianfeng Rong wrote:
> As noted in the kernel documentation [1], open-coded multiplication in
> allocator arguments is discouraged because it can lead to integer overflow.
> 
> Use kmalloc_array() to gain built-in overflow protection, making memory
> allocation safer when calculating allocation size compared to explicit
> multiplication.
> 
> Link: https://www.kernel.org/doc/html/next/process/deprecated.html#open-coded-arithmetic-in-allocator-arguments #1
> Signed-off-by: Qianfeng Rong <rongqianfeng@vivo.com>

This is fine on its own terms, but I would prefer the use of array_size()
 (as in the existing ef100 code) rather than kmalloc_array(), because the
 new X4 devices report a buffer size rather than a number of stats to the
 host, meaning that the common code will need to work in terms of size
 rather than num when support for stats on X4 is added.
Looping in AndyM who's been working on said support and can hopefully
 correct me if I've misstated anything.

-ed

> ---
>  drivers/net/ethernet/sfc/ef10.c      | 4 ++--
>  drivers/net/ethernet/sfc/ef100_nic.c | 3 ++-
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/sfc/ef10.c b/drivers/net/ethernet/sfc/ef10.c
> index fcec81f862ec..311df5467c4a 100644
> --- a/drivers/net/ethernet/sfc/ef10.c
> +++ b/drivers/net/ethernet/sfc/ef10.c
> @@ -1326,8 +1326,8 @@ static int efx_ef10_init_nic(struct efx_nic *efx)
>  		efx->must_realloc_vis = false;
>  	}
>  
> -	nic_data->mc_stats = kmalloc(efx->num_mac_stats * sizeof(__le64),
> -				     GFP_KERNEL);
> +	nic_data->mc_stats = kmalloc_array(efx->num_mac_stats, sizeof(__le64),
> +					   GFP_KERNEL);
>  	if (!nic_data->mc_stats)
>  		return -ENOMEM;
>  
> diff --git a/drivers/net/ethernet/sfc/ef100_nic.c b/drivers/net/ethernet/sfc/ef100_nic.c
> index 3ad95a4c8af2..f4b74381831f 100644
> --- a/drivers/net/ethernet/sfc/ef100_nic.c
> +++ b/drivers/net/ethernet/sfc/ef100_nic.c
> @@ -640,7 +640,8 @@ static size_t ef100_update_stats(struct efx_nic *efx,
>  				 u64 *full_stats,
>  				 struct rtnl_link_stats64 *core_stats)
>  {
> -	__le64 *mc_stats = kmalloc(array_size(efx->num_mac_stats, sizeof(__le64)), GFP_ATOMIC);
> +	__le64 *mc_stats = kmalloc_array(efx->num_mac_stats, sizeof(__le64),
> +					 GFP_ATOMIC);
>  	struct ef100_nic_data *nic_data = efx->nic_data;
>  	DECLARE_BITMAP(mask, EF100_STAT_COUNT) = {};
>  	u64 *stats = nic_data->stats;