public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH net] ixgbevf: fix use-after-free in VEPA multicast source pruning
@ 2026-04-13 18:24 Michael Bommarito
  2026-04-15 16:17 ` Simon Horman
  0 siblings, 1 reply; 4+ messages in thread
From: Michael Bommarito @ 2026-04-13 18:24 UTC (permalink / raw)
  To: intel-wired-lan
  Cc: Tony Nguyen, Przemek Kitszel, Andrew Lunn, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev, stable,
	linux-kernel, Michael Bommarito

ixgbevf_clean_rx_irq() prunes frames whose source MAC matches the VF's
own address (VEPA multicast workaround) by freeing the skb and
continuing to the next descriptor:

    dev_kfree_skb_irq(skb);
    continue;

The skb pointer is declared outside the while loop and persists across
iterations.  Because the continue skips the "skb = NULL" reset at the
bottom of the loop, the next iteration enters the "else if (skb)" path
and calls ixgbevf_add_rx_frag() on the freed skb, dereferencing
skb_shinfo(skb)->nr_frags — a use-after-free in NAPI softirq context.

The sibling driver iavf already handles this correctly by nulling the
pointer before continuing.  Apply the same pattern here.

I do not have ixgbevf hardware; the bug was found by static analysis
(scan_drop_continue_loops.py + semgrep drop_continue_in_loop, multi-tool
corroboration with the highest score in the scan).  The UAF was confirmed
under KASAN by loading a test module that reproduces the exact code
pattern (alloc skb, kfree_skb, then read skb_shinfo(skb)->nr_frags):

  BUG: KASAN: slab-use-after-free in ixgbevf_uaf_test_init+0x100/0x1000
  Read of size 8 at addr 000000006163ae78 by task insmod/30
  freed 208-byte region [000000006163adc0, 000000006163ae90)

QEMU emulates igb (82576) but not ixgbe (82599), and the igbvf VF
driver does not include the VEPA source pruning path, so a full
end-to-end reproduction with emulated hardware was not possible.

Fixes: bad17234ba70 ("ixgbevf: Change receive model to use double buffered page based receives")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-6
Assisted-by: Codex:gpt-5-4
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
 drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index 42f89a179a3f..4ba3be961ab6 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -1221,6 +1221,7 @@ static int ixgbevf_clean_rx_irq(struct ixgbevf_q_vector *q_vector,
 		    ether_addr_equal(rx_ring->netdev->dev_addr,
 				     eth_hdr(skb)->h_source)) {
 			dev_kfree_skb_irq(skb);
+			skb = NULL;
 			continue;
 		}
 
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH net] ixgbevf: fix use-after-free in VEPA multicast source pruning
  2026-04-13 18:24 [PATCH net] ixgbevf: fix use-after-free in VEPA multicast source pruning Michael Bommarito
@ 2026-04-15 16:17 ` Simon Horman
  2026-04-15 16:30   ` Michael Bommarito
  0 siblings, 1 reply; 4+ messages in thread
From: Simon Horman @ 2026-04-15 16:17 UTC (permalink / raw)
  To: Michael Bommarito
  Cc: intel-wired-lan, Tony Nguyen, Przemek Kitszel, Andrew Lunn,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	netdev, stable, linux-kernel

On Mon, Apr 13, 2026 at 02:24:27PM -0400, Michael Bommarito wrote:
> ixgbevf_clean_rx_irq() prunes frames whose source MAC matches the VF's
> own address (VEPA multicast workaround) by freeing the skb and
> continuing to the next descriptor:
> 
>     dev_kfree_skb_irq(skb);
>     continue;
> 
> The skb pointer is declared outside the while loop and persists across
> iterations.  Because the continue skips the "skb = NULL" reset at the
> bottom of the loop, the next iteration enters the "else if (skb)" path
> and calls ixgbevf_add_rx_frag() on the freed skb, dereferencing
> skb_shinfo(skb)->nr_frags — a use-after-free in NAPI softirq context.
> 
> The sibling driver iavf already handles this correctly by nulling the
> pointer before continuing.  Apply the same pattern here.
> 
> I do not have ixgbevf hardware; the bug was found by static analysis
> (scan_drop_continue_loops.py + semgrep drop_continue_in_loop, multi-tool
> corroboration with the highest score in the scan).  The UAF was confirmed
> under KASAN by loading a test module that reproduces the exact code
> pattern (alloc skb, kfree_skb, then read skb_shinfo(skb)->nr_frags):
> 
>   BUG: KASAN: slab-use-after-free in ixgbevf_uaf_test_init+0x100/0x1000
>   Read of size 8 at addr 000000006163ae78 by task insmod/30
>   freed 208-byte region [000000006163adc0, 000000006163ae90)
> 
> QEMU emulates igb (82576) but not ixgbe (82599), and the igbvf VF
> driver does not include the VEPA source pruning path, so a full
> end-to-end reproduction with emulated hardware was not possible.
> 
> Fixes: bad17234ba70 ("ixgbevf: Change receive model to use double buffered page based receives")
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-4-6
> Assisted-by: Codex:gpt-5-4
> Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>

Reviewed-by: Simon Horman <horms@kernel.org>

Sashiko flags a number of issues in the same function that
do not seem related to your patch.

I'd suggest looking over them if you are interested in
follow-up work in this area.

...

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net] ixgbevf: fix use-after-free in VEPA multicast source pruning
  2026-04-15 16:17 ` Simon Horman
@ 2026-04-15 16:30   ` Michael Bommarito
  2026-04-16 17:13     ` Simon Horman
  0 siblings, 1 reply; 4+ messages in thread
From: Michael Bommarito @ 2026-04-15 16:30 UTC (permalink / raw)
  To: Simon Horman
  Cc: intel-wired-lan, Tony Nguyen, Przemek Kitszel, Andrew Lunn,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	netdev, stable, linux-kernel

On Wed, Apr 15, 2026 at 12:17 PM Simon Horman <horms@kernel.org> wrote:
> Sashiko flags a number of issues in the same function that
> do not seem related to your patch.
>
> I'd suggest looking over them if you are interested in
> follow-up work in this area.

Sure, I'd be happy to keep going here if you're open to more hardening
patches.

Two Qs for you:

1. Do you want smaller patches for each or bigger method-level patches?

2. Anything on my list below that you would *not* want me touching?
I'll combine with anything I can find from your Sashiko items

    1. line 104
       rule:   semgrep bug-on-in-net-code (CWE-617)
       match:  BUG_ON(!test_bit(__IXGBEVF_SERVICE_SCHED,
&adapter->state))
       where:  ixgbevf_service_event_schedule()
       status: untriaged

    2. lines 1219-1225
       rule:   net-drop-continue-in-loop + scan_drop_continue_loops.py
       match:  VEPA multicast pruning kfree_skb + continue (UAF)
       where:  ixgbevf_clean_rx_irq()
       status: SHIPPED as commit ca62ac02b30d (this patch)

    3. line 2769
       rule:   semgrep signed-int-as-size-param-kmalloc
       match:  q_vector = kzalloc(size, GFP_KERNEL)  (signed size)
       status: untriaged

    4. line 3452
       rule:   semgrep signed-int-as-size-param-kmalloc
       match:  tx_ring->tx_buffer_info = vmalloc(size)  (signed size)
       status: untriaged

    5. line 3530
       rule:   semgrep signed-int-as-size-param-kmalloc
       match:  rx_ring->rx_buffer_info = vmalloc(size)  (signed size)
       status: untriaged

    6. line 4114
       rule:   semgrep narrow-accumulator-overflow
       match:  i += tx_ring->count;
       status: untriaged

    7. line 4189
       rule:   semgrep narrow-accumulator-overflow
       match:  count += TXD_USE_COUNT(skb_frag_size(frag));
       status: untriaged

    8. line 4192
       rule:   semgrep narrow-accumulator-overflow
       match:  count += skb_shinfo(skb)->nr_frags;
       status: untriaged

    9. line 4695
       rule:   coccinelle cancel_work.cocci
       match:  INIT_WORK(&adapter->service_task, ixgbevf_service_task)
               with no matching cancel_work_sync on teardown path
       status: untriaged

   10. line 4752
       rule:   coccinelle null_after_free.cocci
       where:  ixgbevf_probe() err_dma path
       status: untriaged

   11. line 4795
       rule:   coccinelle null_after_free.cocci
       where:  ixgbevf_remove()
       status: untriaged

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net] ixgbevf: fix use-after-free in VEPA multicast source pruning
  2026-04-15 16:30   ` Michael Bommarito
@ 2026-04-16 17:13     ` Simon Horman
  0 siblings, 0 replies; 4+ messages in thread
From: Simon Horman @ 2026-04-16 17:13 UTC (permalink / raw)
  To: Michael Bommarito
  Cc: intel-wired-lan, Tony Nguyen, Przemek Kitszel, Andrew Lunn,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	netdev, stable, linux-kernel

On Wed, Apr 15, 2026 at 12:30:45PM -0400, Michael Bommarito wrote:
> On Wed, Apr 15, 2026 at 12:17 PM Simon Horman <horms@kernel.org> wrote:
> > Sashiko flags a number of issues in the same function that
> > do not seem related to your patch.
> >
> > I'd suggest looking over them if you are interested in
> > follow-up work in this area.
> 
> Sure, I'd be happy to keep going here if you're open to more hardening
> patches.

Speaking for myself: I'm happy to review patches that correct bugs.

I'm also happy to review patches that otherwise improve the code.
But I think the Intel people might be able to provide better guidance here.

Please be aware of the Netdev guidance on cleanups:

> 
> Two Qs for you:
> 
> 1. Do you want smaller patches for each or bigger method-level patches?

The general rule of thumb is one patch per problem.
Personally, I prefer small patches.

> 
> 2. Anything on my list below that you would *not* want me touching?
> I'll combine with anything I can find from your Sashiko items

...

>     3. line 2769
>        rule:   semgrep signed-int-as-size-param-kmalloc
>        match:  q_vector = kzalloc(size, GFP_KERNEL)  (signed size)
>        status: untriaged
> 
>     4. line 3452
>        rule:   semgrep signed-int-as-size-param-kmalloc
>        match:  tx_ring->tx_buffer_info = vmalloc(size)  (signed size)
>        status: untriaged
> 
>     5. line 3530
>        rule:   semgrep signed-int-as-size-param-kmalloc
>        match:  rx_ring->rx_buffer_info = vmalloc(size)  (signed size)
>        status: untriaged

I didn't look closely, but: I am a little skeptical that these signed size
problems are worth fixing; while the other items on your list look worth
fixing to me.

...

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-04-16 17:13 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-13 18:24 [PATCH net] ixgbevf: fix use-after-free in VEPA multicast source pruning Michael Bommarito
2026-04-15 16:17 ` Simon Horman
2026-04-15 16:30   ` Michael Bommarito
2026-04-16 17:13     ` Simon Horman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox