Re: [Intel-wired-lan] [PATCH iwl-net v1] igc: fix potential skb leak in igc_fpe_xmit_smd_frame()

Kohei Enju
2026-04-04T11:40:42Z

On 04/03 15:05, Simon Horman wrote:
> On Sun, Mar 29, 2026 at 02:50:49PM +0000, Kohei Enju wrote:
> > When igc_fpe_init_tx_descriptor() fails, no one takes care of an
> > allocated skb, leaking it. [1]
> >
> > Use dev_kfree_skb_any() on failure.
> > Also call igc_flush_tx_descriptors() only on success.
> >
> > [1]
> > unreferenced object 0xffff88813aee1b40 (size 224):
> > comm "softirq", pid 0, jiffies 4294709256
> > [...]
> > backtrace (crc dee31384):
> > kmem_cache_alloc_node_noprof+0x54f/0x640
> > __alloc_skb+0xd9/0x5b0
> > igc_fpe_xmit_smd_frame.isra.0+0xad/0x510
> > igc_fpe_send_mpacket+0x32/0x80
> > [...]
> >
> > Fixes: 5422570c0010 ("igc: add support for frame preemption verification")
> > Signed-off-by: Kohei Enju <kohei@enjuk.jp>
>
> I am wondering if we can achieve this while sticking to the idiom
> that the main thread of execution is for the non-error flow,
> while conditions handle errors.

Hi Simon-san, thank you for the suggestion.
I agree. It seems cleaner for me. I'll work on v2.

> Something like this (completely untested!):
>
> diff --git a/drivers/net/ethernet/intel/igc/igc_tsn.c b/drivers/net/ethernet/intel/igc/igc_tsn.c
> index 8a110145bfee..02dd9f0290a3 100644
> --- a/drivers/net/ethernet/intel/igc/igc_tsn.c
> +++ b/drivers/net/ethernet/intel/igc/igc_tsn.c
> @@ -109,10 +109,16 @@ static int igc_fpe_xmit_smd_frame(struct igc_adapter *adapter,
> __netif_tx_lock(nq, cpu);
>
> err = igc_fpe_init_tx_descriptor(ring, skb, type);
> - igc_flush_tx_descriptors(ring);
> + if (err)
> + goto err_free_skb_any;
>
> + igc_flush_tx_descriptors(ring);
> __netif_tx_unlock(nq);
> + return 0;
>
> +err_free_skb_any:
> + __netif_tx_unlock(nq);
> + dev_kfree_skb_any(skb);
> return err;
> }
>