| Message ID | YZbAsgT17yxu4Otk@a-10-27-17-117.dynapool.vpn.nyu.edu (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 6a405f6c372d14707b87d3097b361b69899a26c8 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | atlantic: fix double-free in aq_ring_tx_clean | expand |
> We found this bug while fuzzing the device driver. Using and freeing > the dangling pointer buff->skb would cause use-after-free and > double-free. > > This bug is triggerable with compromised/malfunctioning devices. We > found the bug with QEMU emulation and tested the patch by emulation. > We did NOT test on a real device. > > Attached is the bug report. > Reviewed-by: Igor Russkikh <irusskikh@marvell.com> Thank you for submitting this! Igor
Hello: This patch was applied to netdev/net.git (master) by David S. Miller <davem@davemloft.net>: On Thu, 18 Nov 2021 16:08:02 -0500 you wrote: > We found this bug while fuzzing the device driver. Using and freeing > the dangling pointer buff->skb would cause use-after-free and > double-free. > > This bug is triggerable with compromised/malfunctioning devices. We > found the bug with QEMU emulation and tested the patch by emulation. > We did NOT test on a real device. > > [...] Here is the summary with links: - atlantic: fix double-free in aq_ring_tx_clean https://git.kernel.org/netdev/net/c/6a405f6c372d You are awesome, thank you!
diff --git a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c index 24122ccda614..81b3756417ec 100644 --- a/drivers/net/ethernet/aquantia/atlantic/aq_ring.c +++ b/drivers/net/ethernet/aquantia/atlantic/aq_ring.c @@ -298,13 +298,14 @@ bool aq_ring_tx_clean(struct aq_ring_s *self) } } - if (unlikely(buff->is_eop)) { + if (unlikely(buff->is_eop && buff->skb)) { u64_stats_update_begin(&self->stats.tx.syncp); ++self->stats.tx.packets; self->stats.tx.bytes += buff->skb->len; u64_stats_update_end(&self->stats.tx.syncp); dev_kfree_skb_any(buff->skb); + buff->skb = NULL; } buff->pa = 0U; buff->eop_index = 0xffffU;
We found this bug while fuzzing the device driver. Using and freeing the dangling pointer buff->skb would cause use-after-free and double-free. This bug is triggerable with compromised/malfunctioning devices. We found the bug with QEMU emulation and tested the patch by emulation. We did NOT test on a real device. Attached is the bug report. BUG: KASAN: double-free or invalid-free in consume_skb+0x6c/0x1c0 Call Trace: dump_stack+0x76/0xa0 print_address_description.constprop.0+0x16/0x200 ? consume_skb+0x6c/0x1c0 kasan_report_invalid_free+0x61/0xa0 ? consume_skb+0x6c/0x1c0 __kasan_slab_free+0x15e/0x170 ? consume_skb+0x6c/0x1c0 kfree+0x8c/0x230 consume_skb+0x6c/0x1c0 aq_ring_tx_clean+0x5c2/0xa80 [atlantic] aq_vec_poll+0x309/0x5d0 [atlantic] ? _sub_I_65535_1+0x20/0x20 [atlantic] ? __next_timer_interrupt+0xba/0xf0 net_rx_action+0x363/0xbd0 ? call_timer_fn+0x240/0x240 ? __switch_to_asm+0x34/0x70 ? napi_busy_loop+0x520/0x520 ? net_tx_action+0x379/0x720 __do_softirq+0x18c/0x634 ? takeover_tasklets+0x5f0/0x5f0 run_ksoftirqd+0x15/0x20 smpboot_thread_fn+0x2f1/0x6b0 ? smpboot_unregister_percpu_thread+0x160/0x160 ? __kthread_parkme+0x80/0x100 ? smpboot_unregister_percpu_thread+0x160/0x160 kthread+0x2b5/0x3b0 ? kthread_create_on_node+0xd0/0xd0 ret_from_fork+0x22/0x40 Reported-by: Brendan Dolan-Gavitt <brendandg@nyu.edu> Signed-off-by: Zekun Shen <bruceshenzk@gmail.com> --- drivers/net/ethernet/aquantia/atlantic/aq_ring.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)