Re: [PATCH iwl-net v2] ice: Skip PTP timestamp processing during release

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] net: ice: SKB memory leak in PTP Tx tracker when state is not ready
--

commit 20fdb60a0f81d0a78832f24386472d8613b50aea
Author: Sergey Temerkhanov <sergey.temerkhanov@intel.com>

ice: Skip PTP timestamp processing during release

This commit adds early returns to the PTP timestamp processing paths
when the PTP state is not ICE_PTP_READY. It is intended to prevent running
callbacks on unready hardware and hitting uninitialized structures.

> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c
> index b2de76065f46d..c0c12e28e08fa 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ptp.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
> @@ -2815,6 +2815,9 @@ s8 ice_ptp_request_ts(struct ice_ptp_tx *tx, struct sk_buff *skb)
>  
>  void ice_ptp_process_ts(struct ice_pf *pf)
>  {
> +	if (pf->ptp.state != ICE_PTP_READY)
> +		return;
> +

[Severity: High]
Does this early return bypass the 2-second SKB timeout cleanup logic in
ice_ptp_process_tx_tstamp()?

If PTP initialization or rebuild fails, setting pf->ptp.state to
ICE_PTP_ERROR (or any non-READY state), but the normal NIC rebuild
continues and succeeds (link_up becomes true, tx->init remains true),
userspace can still send packets with SO_TIMESTAMPING.

In this scenario, ice_ptp_request_ts() accepts the packets and queues the SKB
in tx->tstamps[idx].skb because ice_ptp_is_tx_tracker_up() ignores
pf->ptp.state.

Since ice_ptp_process_ts() now returns immediately, the timeout cleanup logic
inside ice_ptp_process_tx_tstamp() is bypassed. Will this permanently leak the
queued SKBs until the tracker fills up and drops new requests?

>  	switch (pf->ptp.tx_interrupt_mode) {
>  	case ICE_PTP_TX_INTERRUPT_NONE:
>  		/* This device has the clock owner handle timestamps for it */

[ ... ]

> @@ -2944,6 +2947,9 @@ irqreturn_t ice_ptp_ts_irq(struct ice_pf *pf)
>  		return IRQ_WAKE_THREAD;
>  	case ICE_MAC_E830:
>  		/* E830 can read timestamps in the top half using rd32() */
> +		if (pf->ptp.state != ICE_PTP_READY)
> +			return IRQ_HANDLED;
> +

[Severity: High]
Similarly for the E830 IRQ handler, does this early return bypass the cleanup
logic in ice_ptp_process_ts() and cause the same SKB memory leak when the PTP
state is not ready?

>  		ice_ptp_process_ts(pf);
>  
>  		if (ice_ptp_tx_tstamps_pending(pf, true)) {
>  			/* Process outstanding Tx timestamps. If there

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261006095435.419128-1-sergey.temerkhanov@intel.com?part=1