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 <[email protected]> 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/[email protected]?part=1
