linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] rsi_usb: Fix use-after-free in rsi_rx_done_handler
@ 2021-10-29 19:49 Zekun Shen
  2021-11-01 14:06 ` Kalle Valo
  2021-11-29 10:43 ` rsi: Fix use-after-free in rsi_rx_done_handler() Kalle Valo
  0 siblings, 2 replies; 3+ messages in thread
From: Zekun Shen @ 2021-10-29 19:49 UTC (permalink / raw)
  To: bruceshenzk
  Cc: Amitkumar Karwar, Siva Rebbagondla, Kalle Valo, David S. Miller,
	Jakub Kicinski, linux-wireless, netdev, linux-kernel, brendandg

When freeing rx_cb->rx_skb, the pointer is not set to NULL,
a later rsi_rx_done_handler call will try to read the freed
address.
This bug will very likley lead to double free, although
detected early as use-after-free bug.

The bug is triggerable with a compromised/malfunctional usb
device. After applying the patch, the same input no longer
triggers the use-after-free.

Attached is the kasan report from fuzzing.

BUG: KASAN: use-after-free in rsi_rx_done_handler+0x354/0x430 [rsi_usb]
Read of size 4 at addr ffff8880188e5930 by task modprobe/231
Call Trace:
 <IRQ>
 dump_stack+0x76/0xa0
 print_address_description.constprop.0+0x16/0x200
 ? rsi_rx_done_handler+0x354/0x430 [rsi_usb]
 ? rsi_rx_done_handler+0x354/0x430 [rsi_usb]
 __kasan_report.cold+0x37/0x7c
 ? dma_direct_unmap_page+0x90/0x110
 ? rsi_rx_done_handler+0x354/0x430 [rsi_usb]
 kasan_report+0xe/0x20
 rsi_rx_done_handler+0x354/0x430 [rsi_usb]
 __usb_hcd_giveback_urb+0x1e4/0x380
 usb_giveback_urb_bh+0x241/0x4f0
 ? __usb_hcd_giveback_urb+0x380/0x380
 ? apic_timer_interrupt+0xa/0x20
 tasklet_action_common.isra.0+0x135/0x330
 __do_softirq+0x18c/0x634
 ? handle_irq_event+0xcd/0x157
 ? handle_edge_irq+0x1eb/0x7b0
 irq_exit+0x114/0x140
 do_IRQ+0x91/0x1e0
 common_interrupt+0xf/0xf
 </IRQ>

Reported-by: Zekun Shen <bruceshenzk@gmail.com>
Reported-by: Brendan Dolan-Gavitt <brendandg@nyu.edu>
Signed-off-by: Zekun Shen <bruceshenzk@gmail.com>
---
 drivers/net/wireless/rsi/rsi_91x_usb.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/rsi/rsi_91x_usb.c b/drivers/net/wireless/rsi/rsi_91x_usb.c
index 416976f09..d9e9bf26e 100644
--- a/drivers/net/wireless/rsi/rsi_91x_usb.c
+++ b/drivers/net/wireless/rsi/rsi_91x_usb.c
@@ -272,8 +272,12 @@ static void rsi_rx_done_handler(struct urb *urb)
 	struct rsi_91x_usbdev *dev = (struct rsi_91x_usbdev *)rx_cb->data;
 	int status = -EINVAL;
 
+	if (!rx_cb->rx_skb)
+		return;
+
 	if (urb->status) {
 		dev_kfree_skb(rx_cb->rx_skb);
+		rx_cb->rx_skb = NULL;
 		return;
 	}
 
@@ -297,8 +301,10 @@ static void rsi_rx_done_handler(struct urb *urb)
 	if (rsi_rx_urb_submit(dev->priv, rx_cb->ep_num, GFP_ATOMIC))
 		rsi_dbg(ERR_ZONE, "%s: Failed in urb submission", __func__);
 
-	if (status)
+	if (status) {
 		dev_kfree_skb(rx_cb->rx_skb);
+		rx_cb->rx_skb = NULL;
+	}
 }
 
 static void rsi_rx_urb_kill(struct rsi_hw *adapter, u8 ep_num)
-- 
2.25.1


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

* Re: [PATCH] rsi_usb: Fix use-after-free in rsi_rx_done_handler
  2021-10-29 19:49 [PATCH] rsi_usb: Fix use-after-free in rsi_rx_done_handler Zekun Shen
@ 2021-11-01 14:06 ` Kalle Valo
  2021-11-29 10:43 ` rsi: Fix use-after-free in rsi_rx_done_handler() Kalle Valo
  1 sibling, 0 replies; 3+ messages in thread
From: Kalle Valo @ 2021-11-01 14:06 UTC (permalink / raw)
  To: Zekun Shen
  Cc: Amitkumar Karwar, Siva Rebbagondla, David S. Miller,
	Jakub Kicinski, linux-wireless, netdev, linux-kernel, brendandg

Zekun Shen <bruceshenzk@gmail.com> writes:

> When freeing rx_cb->rx_skb, the pointer is not set to NULL,
> a later rsi_rx_done_handler call will try to read the freed
> address.
> This bug will very likley lead to double free, although
> detected early as use-after-free bug.
>
> The bug is triggerable with a compromised/malfunctional usb
> device. After applying the patch, the same input no longer
> triggers the use-after-free.
>
> Attached is the kasan report from fuzzing.
>
> BUG: KASAN: use-after-free in rsi_rx_done_handler+0x354/0x430 [rsi_usb]
> Read of size 4 at addr ffff8880188e5930 by task modprobe/231
> Call Trace:
>  <IRQ>
>  dump_stack+0x76/0xa0
>  print_address_description.constprop.0+0x16/0x200
>  ? rsi_rx_done_handler+0x354/0x430 [rsi_usb]
>  ? rsi_rx_done_handler+0x354/0x430 [rsi_usb]
>  __kasan_report.cold+0x37/0x7c
>  ? dma_direct_unmap_page+0x90/0x110
>  ? rsi_rx_done_handler+0x354/0x430 [rsi_usb]
>  kasan_report+0xe/0x20
>  rsi_rx_done_handler+0x354/0x430 [rsi_usb]
>  __usb_hcd_giveback_urb+0x1e4/0x380
>  usb_giveback_urb_bh+0x241/0x4f0
>  ? __usb_hcd_giveback_urb+0x380/0x380
>  ? apic_timer_interrupt+0xa/0x20
>  tasklet_action_common.isra.0+0x135/0x330
>  __do_softirq+0x18c/0x634
>  ? handle_irq_event+0xcd/0x157
>  ? handle_edge_irq+0x1eb/0x7b0
>  irq_exit+0x114/0x140
>  do_IRQ+0x91/0x1e0
>  common_interrupt+0xf/0xf
>  </IRQ>
>
> Reported-by: Zekun Shen <bruceshenzk@gmail.com>
> Reported-by: Brendan Dolan-Gavitt <brendandg@nyu.edu>
> Signed-off-by: Zekun Shen <bruceshenzk@gmail.com>

There's no need to have the author in Reported-by tag, so I'll remove
that.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

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

* Re: rsi: Fix use-after-free in rsi_rx_done_handler()
  2021-10-29 19:49 [PATCH] rsi_usb: Fix use-after-free in rsi_rx_done_handler Zekun Shen
  2021-11-01 14:06 ` Kalle Valo
@ 2021-11-29 10:43 ` Kalle Valo
  1 sibling, 0 replies; 3+ messages in thread
From: Kalle Valo @ 2021-11-29 10:43 UTC (permalink / raw)
  To: Zekun Shen
  Cc: bruceshenzk, Amitkumar Karwar, Siva Rebbagondla, David S. Miller,
	Jakub Kicinski, linux-wireless, netdev, linux-kernel, brendandg

Zekun Shen <bruceshenzk@gmail.com> wrote:

> When freeing rx_cb->rx_skb, the pointer is not set to NULL,
> a later rsi_rx_done_handler call will try to read the freed
> address.
> This bug will very likley lead to double free, although
> detected early as use-after-free bug.
> 
> The bug is triggerable with a compromised/malfunctional usb
> device. After applying the patch, the same input no longer
> triggers the use-after-free.
> 
> Attached is the kasan report from fuzzing.
> 
> BUG: KASAN: use-after-free in rsi_rx_done_handler+0x354/0x430 [rsi_usb]
> Read of size 4 at addr ffff8880188e5930 by task modprobe/231
> Call Trace:
>  <IRQ>
>  dump_stack+0x76/0xa0
>  print_address_description.constprop.0+0x16/0x200
>  ? rsi_rx_done_handler+0x354/0x430 [rsi_usb]
>  ? rsi_rx_done_handler+0x354/0x430 [rsi_usb]
>  __kasan_report.cold+0x37/0x7c
>  ? dma_direct_unmap_page+0x90/0x110
>  ? rsi_rx_done_handler+0x354/0x430 [rsi_usb]
>  kasan_report+0xe/0x20
>  rsi_rx_done_handler+0x354/0x430 [rsi_usb]
>  __usb_hcd_giveback_urb+0x1e4/0x380
>  usb_giveback_urb_bh+0x241/0x4f0
>  ? __usb_hcd_giveback_urb+0x380/0x380
>  ? apic_timer_interrupt+0xa/0x20
>  tasklet_action_common.isra.0+0x135/0x330
>  __do_softirq+0x18c/0x634
>  ? handle_irq_event+0xcd/0x157
>  ? handle_edge_irq+0x1eb/0x7b0
>  irq_exit+0x114/0x140
>  do_IRQ+0x91/0x1e0
>  common_interrupt+0xf/0xf
>  </IRQ>
> 
> Reported-by: Brendan Dolan-Gavitt <brendandg@nyu.edu>
> Signed-off-by: Zekun Shen <bruceshenzk@gmail.com>

Patch applied to wireless-drivers-next.git, thanks.

b07e3c6ebc0c rsi: Fix use-after-free in rsi_rx_done_handler()

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/YXxQL/vIiYcZUu/j@10-18-43-117.dynapool.wireless.nyu.edu/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


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

end of thread, other threads:[~2021-11-29 10:45 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-29 19:49 [PATCH] rsi_usb: Fix use-after-free in rsi_rx_done_handler Zekun Shen
2021-11-01 14:06 ` Kalle Valo
2021-11-29 10:43 ` rsi: Fix use-after-free in rsi_rx_done_handler() Kalle Valo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).