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

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

[Kohei Enju]

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>
---
 drivers/net/ethernet/intel/igc/igc_tsn.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/igc/igc_tsn.c b/drivers/net/ethernet/intel/igc/igc_tsn.c
index 8a110145bfee..eaa274248d54 100644
--- a/drivers/net/ethernet/intel/igc/igc_tsn.c
+++ b/drivers/net/ethernet/intel/igc/igc_tsn.c
@@ -109,10 +109,14 @@ 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)
+		igc_flush_tx_descriptors(ring);
 
 	__netif_tx_unlock(nq);
 
+	if (err)
+		dev_kfree_skb_any(skb);
+
 	return err;
 }
 
-- 
2.51.0

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

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> Of Kohei Enju
> Sent: Sunday, March 29, 2026 4:51 PM
> To: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org
> Cc: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel,
> Przemyslaw <przemyslaw.kitszel@intel.com>; Andrew Lunn
> <andrew+netdev@lunn.ch>; David S. Miller <davem@davemloft.net>; Eric
> Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo
> Abeni <pabeni@redhat.com>; Faizal Rahim
> <faizal.abdul.rahim@linux.intel.com>; Kohei Enju <kohei@enjuk.jp>
> Subject: [Intel-wired-lan] [PATCH iwl-net v1] igc: fix potential skb
> leak in igc_fpe_xmit_smd_frame()
> 
> 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>
> ---
>  drivers/net/ethernet/intel/igc/igc_tsn.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/igc/igc_tsn.c
> b/drivers/net/ethernet/intel/igc/igc_tsn.c
> index 8a110145bfee..eaa274248d54 100644
> --- a/drivers/net/ethernet/intel/igc/igc_tsn.c
> +++ b/drivers/net/ethernet/intel/igc/igc_tsn.c
> @@ -109,10 +109,14 @@ 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)
> +		igc_flush_tx_descriptors(ring);
> 
>  	__netif_tx_unlock(nq);
> 
> +	if (err)
> +		dev_kfree_skb_any(skb);
> +
>  	return err;
>  }
> 
> --
> 2.51.0

I'd recommend adding reproduction steps and Cc: stable@vger.kernel.org
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>

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

[Simon Horman]

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.

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;
 }
 

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

[Kohei Enju]

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;
>  }
>