[Intel-wired-lan] [PATCH v1 iwl-net] ice: fix potential NULL pointer deref in error path of ice_set_ringparam()

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Kohei Enju <kohei at enjuk.jp>
> Sent: Friday, February 20, 2026 7:40 PM
> To: intel-wired-lan at lists.osuosl.org; netdev at vger.kernel.org
> Cc: Nguyen, Anthony L <anthony.l.nguyen at intel.com>; Kitszel,
> Przemyslaw <przemyslaw.kitszel at intel.com>; Andrew Lunn
> <andrew+netdev at lunn.ch>; David S. Miller <davem at davemloft.net>; Eric
> Dumazet <edumazet at google.com>; Jakub Kicinski <kuba at kernel.org>; Paolo
> Abeni <pabeni at redhat.com>; Loktionov, Aleksandr
> <aleksandr.loktionov at intel.com>; Alice Michael
> <alice.michael at intel.com>; Greenwalt, Paul <paul.greenwalt at intel.com>;
> Fijalkowski, Maciej <maciej.fijalkowski at intel.com>;
> kohei.enju at gmail.com; Kohei Enju <kohei at enjuk.jp>
> Subject: [PATCH v1 iwl-net] ice: fix potential NULL pointer deref in
> error path of ice_set_ringparam()
> 
> ice_set_ringparam nullifies tstamp_ring of temporary tx_rings, without
> clearing ICE_TX_RING_FLAGS_TXTIME bit.
> When ICE_TX_RING_FLAGS_TXTIME is set and the subsequent
> ice_setup_tx_ring() call fails, a NULL pointer dereference could
> happen in the unwinding sequence:
> 
> ice_clean_tx_ring()
> -> ice_is_txtime_cfg() == true (ICE_TX_RING_FLAGS_TXTIME is set)
> -> ice_free_tx_tstamp_ring()
>   -> ice_free_tstamp_ring()
>     -> tstamp_ring->desc (NULL deref)
> 
> Clear ICE_TX_RING_FLAGS_TXTIME bit to avoid the potential issue.
> 
> Note that this potential issue is found by manual code review.
> Compile test only since unfortunately I don't have E830 devices.
> 
> Fixes: ccde82e90946 ("ice: add E830 Earliest TxTime First Offload
> support")
If it's a fix, shouldn't it go to net?

> Signed-off-by: Kohei Enju <kohei at enjuk.jp>
> ---
>  drivers/net/ethernet/intel/ice/ice_ethtool.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c
> b/drivers/net/ethernet/intel/ice/ice_ethtool.c
> index 7f769a90dde1..5ed86648d0d6 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
> @@ -3290,6 +3290,7 @@ ice_set_ringparam(struct net_device *netdev,
> struct ethtool_ringparam *ring,
>  		tx_rings[i].desc = NULL;
>  		tx_rings[i].tx_buf = NULL;
>  		tx_rings[i].tstamp_ring = NULL;
> +		clear_bit(ICE_TX_RING_FLAGS_TXTIME, tx_rings[i].flags);
>  		tx_rings[i].tx_tstamps = &pf->ptp.port.tx;
>  		err = ice_setup_tx_ring(&tx_rings[i]);
If ice_setup_tx_ring() internally reads ICE_TX_RING_FLAGS_TXTIME to decide whether to allocate the tstamp_ring, then clearing the bit first means ice_setup_tx_ring() skips TxTime setup even on success - leaving TxTime silently broken after ice_set_ringparam() completes normally. The crash is fixed on the error path, but I'm afraid a functional regression is introduced on the success path.
Please correct me if I'm wrong.

Thank you
Alex

>  		if (err) {
> --
> 2.51.0