[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <880ef2b4-14a5-49fe-81ad-4b6ea430e911@molgen.mpg.de>
Date: Thu, 22 Jan 2026 12:16:25 +0100
From: Paul Menzel <pmenzel@...gen.mpg.de>
To: Zilin Guan <zilin@....edu.cn>
Cc: anthony.l.nguyen@...el.com, przemyslaw.kitszel@...el.com,
andrew+netdev@...n.ch, davem@...emloft.net, edumazet@...gle.com,
kuba@...nel.org, pabeni@...hat.com, intel-wired-lan@...ts.osuosl.org,
linux-kernel@...r.kernel.org, netdev@...r.kernel.org, jianhao.xu@....edu.cn
Subject: Re: [Intel-wired-lan] [PATCH net] ice: Fix memory leak in
ice_set_ringparam()
Dear Zilin,
Thank you for your patch.
Am 22.01.26 um 04:26 schrieb Zilin Guan:
> In ice_set_ringparam, tx_rings and xdp_rings are allocated before
> rx_rings. If the allocation of rx_rings fails, the code jumps to
> the done label leaking both tx_rings and xdp_rings. Furthermore, if
> the setup of an individual Rx ring fails during the loop, the code jumps
RX ring
> to the free_tx label which releases tx_rings but leaks xdp_rings.
>
> Fix this by introducing a free_xdp label and updating the error paths to
> ensure both xdp_rings and tx_rings are properly freed if rx_rings
> allocation or setup fails.
>
> Compile tested only. Issue found using a prototype static analysis tool
> and code review.
>
> Fixes: fcea6f3da546 ("ice: Add stats and ethtool support")
> Fixes: efc2214b6047 ("ice: Add support for XDP")
> Signed-off-by: Zilin Guan <zilin@....edu.cn>
> ---
> drivers/net/ethernet/intel/ice/ice_ethtool.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c b/drivers/net/ethernet/intel/ice/ice_ethtool.c
> index 969d4f8f9c02..1643b118144a 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
> @@ -3318,7 +3318,7 @@ ice_set_ringparam(struct net_device *netdev, struct ethtool_ringparam *ring,
> rx_rings = kcalloc(vsi->num_rxq, sizeof(*rx_rings), GFP_KERNEL);
> if (!rx_rings) {
> err = -ENOMEM;
> - goto done;
> + goto free_xdp;
> }
>
> ice_for_each_rxq(vsi, i) {
> @@ -3345,7 +3345,7 @@ ice_set_ringparam(struct net_device *netdev, struct ethtool_ringparam *ring,
> }
> kfree(rx_rings);
> err = -ENOMEM;
> - goto free_tx;
> + goto free_xdp;
> }
> }
>
> @@ -3398,6 +3398,13 @@ ice_set_ringparam(struct net_device *netdev, struct ethtool_ringparam *ring,
> }
> goto done;
>
> +free_xdp:
> + if (xdp_rings) {
> + ice_for_each_xdp_txq(vsi, i)
> + ice_free_tx_ring(&xdp_rings[i]);
> + kfree(xdp_rings);
> + }
> +
> free_tx:
> /* error cleanup if the Rx allocations failed after getting Tx */
> if (tx_rings) {
Reviewed-by: Paul Menzel <pmenzel@...gen.mpg.de>
Kind regards,
Paul
Powered by blists - more mailing lists