netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] sfc: use kmalloc_array() instead of kmalloc()
@ 2025-08-25 15:12 Qianfeng Rong
  2025-08-26 19:15 ` Edward Cree
  0 siblings, 1 reply; 2+ messages in thread
From: Qianfeng Rong @ 2025-08-25 15:12 UTC (permalink / raw)
  To: Edward Cree, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, netdev, linux-net-drivers,
	linux-kernel
  Cc: Qianfeng Rong

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>
---
 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;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] sfc: use kmalloc_array() instead of kmalloc()
  2025-08-25 15:12 [PATCH] sfc: use kmalloc_array() instead of kmalloc() Qianfeng Rong
@ 2025-08-26 19:15 ` Edward Cree
  0 siblings, 0 replies; 2+ messages in thread
From: Edward Cree @ 2025-08-26 19:15 UTC (permalink / raw)
  To: Qianfeng Rong, linux-net-drivers, Andy Moreton
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, netdev, linux-kernel

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;


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2025-08-26 19:15 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-25 15:12 [PATCH] sfc: use kmalloc_array() instead of kmalloc() Qianfeng Rong
2025-08-26 19:15 ` Edward Cree

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).