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 <[email protected]>
---
  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 <[email protected]>


Kind regards,

Paul

Reply via email to