rtl818x: Kill URBs before clearing tx status queue

[PATCH v2] rtl818x: Kill URBs before clearing tx status queue

Posted by Daniil Dulov 7 months, 3 weeks ago

In rtl8187_stop() move the call of usb_kill_anchored_urbs() before clearing
b_tx_status.queue. This change prevents callbacks from using already freed
skb due to anchor was not killed before freeing such skb.

 BUG: kernel NULL pointer dereference, address: 0000000000000080
 #PF: supervisor read access in kernel mode
 #PF: error_code(0x0000) - not-present page
 PGD 0 P4D 0
 Oops: Oops: 0000 [#1] SMP NOPTI
 CPU: 7 UID: 0 PID: 0 Comm: swapper/7 Not tainted 6.15.0 #8 PREEMPT(voluntary)
 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 0.0.0 02/06/2015
 RIP: 0010:ieee80211_tx_status_irqsafe+0x21/0xc0 [mac80211]
 Call Trace:
  <IRQ>
  rtl8187_tx_cb+0x116/0x150 [rtl8187]
  __usb_hcd_giveback_urb+0x9d/0x120
  usb_giveback_urb_bh+0xbb/0x140
  process_one_work+0x19b/0x3c0
  bh_worker+0x1a7/0x210
  tasklet_action+0x10/0x30
  handle_softirqs+0xf0/0x340
  __irq_exit_rcu+0xcd/0xf0
  common_interrupt+0x85/0xa0
  </IRQ>

Tested on RTL8187BvE device.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Fixes: c1db52b9d27e ("rtl8187: Use usb anchor facilities to manage urbs")
Signed-off-by: Daniil Dulov <d.dulov@aladdin.ru>
---
 drivers/net/wireless/realtek/rtl818x/rtl8187/dev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtl818x/rtl8187/dev.c b/drivers/net/wireless/realtek/rtl818x/rtl8187/dev.c
index 220ac5bdf279..8a57d6c72335 100644
--- a/drivers/net/wireless/realtek/rtl818x/rtl8187/dev.c
+++ b/drivers/net/wireless/realtek/rtl818x/rtl8187/dev.c
@@ -1041,10 +1041,11 @@ static void rtl8187_stop(struct ieee80211_hw *dev, bool suspend)
 	rtl818x_iowrite8(priv, &priv->map->CONFIG4, reg | RTL818X_CONFIG4_VCOOFF);
 	rtl818x_iowrite8(priv, &priv->map->EEPROM_CMD, RTL818X_EEPROM_CMD_NORMAL);
 
+	usb_kill_anchored_urbs(&priv->anchored);
+
 	while ((skb = skb_dequeue(&priv->b_tx_status.queue)))
 		dev_kfree_skb_any(skb);
 
-	usb_kill_anchored_urbs(&priv->anchored);
 	mutex_unlock(&priv->conf_mutex);
 
 	if (!priv->is_rtl8187b)
-- 
2.34.1

Re: [PATCH v2] rtl818x: Kill URBs before clearing tx status queue

Posted by Ping-Ke Shih 7 months, 2 weeks ago

Daniil Dulov <d.dulov@aladdin.ru> wrote:

> In rtl8187_stop() move the call of usb_kill_anchored_urbs() before clearing
> b_tx_status.queue. This change prevents callbacks from using already freed
> skb due to anchor was not killed before freeing such skb.
> 
>  BUG: kernel NULL pointer dereference, address: 0000000000000080
>  #PF: supervisor read access in kernel mode
>  #PF: error_code(0x0000) - not-present page
>  PGD 0 P4D 0
>  Oops: Oops: 0000 [#1] SMP NOPTI
>  CPU: 7 UID: 0 PID: 0 Comm: swapper/7 Not tainted 6.15.0 #8 PREEMPT(voluntary)
>  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 0.0.0 02/06/2015
>  RIP: 0010:ieee80211_tx_status_irqsafe+0x21/0xc0 [mac80211]
>  Call Trace:
>   <IRQ>
>   rtl8187_tx_cb+0x116/0x150 [rtl8187]
>   __usb_hcd_giveback_urb+0x9d/0x120
>   usb_giveback_urb_bh+0xbb/0x140
>   process_one_work+0x19b/0x3c0
>   bh_worker+0x1a7/0x210
>   tasklet_action+0x10/0x30
>   handle_softirqs+0xf0/0x340
>   __irq_exit_rcu+0xcd/0xf0
>   common_interrupt+0x85/0xa0
>   </IRQ>
> 
> Tested on RTL8187BvE device.
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Fixes: c1db52b9d27e ("rtl8187: Use usb anchor facilities to manage urbs")
> Signed-off-by: Daniil Dulov <d.dulov@aladdin.ru>
> Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>

1 patch(es) applied to rtw-next branch of rtw.git, thanks.

16d8fd74dbfc wifi: rtl818x: Kill URBs before clearing tx status queue

---
https://github.com/pkshih/rtw.git

RE: [PATCH v2] rtl818x: Kill URBs before clearing tx status queue

Posted by Ping-Ke Shih 7 months, 3 weeks ago

Daniil Dulov <d.dulov@aladdin.ru> wrote:
> In rtl8187_stop() move the call of usb_kill_anchored_urbs() before clearing
> b_tx_status.queue. This change prevents callbacks from using already freed
> skb due to anchor was not killed before freeing such skb.
> 
>  BUG: kernel NULL pointer dereference, address: 0000000000000080
>  #PF: supervisor read access in kernel mode
>  #PF: error_code(0x0000) - not-present page
>  PGD 0 P4D 0
>  Oops: Oops: 0000 [#1] SMP NOPTI
>  CPU: 7 UID: 0 PID: 0 Comm: swapper/7 Not tainted 6.15.0 #8 PREEMPT(voluntary)
>  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 0.0.0 02/06/2015
>  RIP: 0010:ieee80211_tx_status_irqsafe+0x21/0xc0 [mac80211]
>  Call Trace:
>   <IRQ>
>   rtl8187_tx_cb+0x116/0x150 [rtl8187]
>   __usb_hcd_giveback_urb+0x9d/0x120
>   usb_giveback_urb_bh+0xbb/0x140
>   process_one_work+0x19b/0x3c0
>   bh_worker+0x1a7/0x210
>   tasklet_action+0x10/0x30
>   handle_softirqs+0xf0/0x340
>   __irq_exit_rcu+0xcd/0xf0
>   common_interrupt+0x85/0xa0
>   </IRQ>
> 
> Tested on RTL8187BvE device.
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Fixes: c1db52b9d27e ("rtl8187: Use usb anchor facilities to manage urbs")
> Signed-off-by: Daniil Dulov <d.dulov@aladdin.ru>

Reviewed-by: Ping-Ke Shih <pkshih@realtek.com>

The subject prefix should be "wifi: rtl818x: ...". 
I can fix it. Don't need to resend because of this.