[PATCH 0/5] ath9k: bug fixes

[PATCH 0/5] ath9k: bug fixes

[Qiujun Huang]

This series fixes some bugs in ath9k reported and tested by syzbot.
https://lore.kernel.org/linux-usb/000000000000666c9c05a1c05d12@google.com
https://lore.kernel.org/linux-usb/0000000000003d7c1505a2168418@google.com
https://lore.kernel.org/linux-usb/0000000000006ac55b05a1c05d72@google.com
https://lore.kernel.org/linux-usb/0000000000000002fc05a1d61a68@google.com
https://lore.kernel.org/linux-usb/000000000000590f6b05a1c05d15@google.com

Qiujun Huang (5):
  ath9k: Fix use-after-free Read in htc_connect_service
  ath9k: Fix use-after-free Read in ath9k_wmi_ctrl_rx
  ath9k: Fix use-after-free Write in ath9k_htc_rx_msg
  ath9x: Fix stack-out-of-bounds Write in ath9k_hif_usb_rx_cb
  ath9k: Fix general protection fault in ath9k_hif_usb_rx_cb

 drivers/net/wireless/ath/ath9k/hif_usb.c      | 58 ++++++++++++++-----
 drivers/net/wireless/ath/ath9k/hif_usb.h      |  6 ++
 drivers/net/wireless/ath/ath9k/htc_drv_init.c | 10 +++-
 drivers/net/wireless/ath/ath9k/htc_hst.c      |  6 +-
 drivers/net/wireless/ath/ath9k/wmi.c          |  6 +-
 drivers/net/wireless/ath/ath9k/wmi.h          |  3 +-
 6 files changed, 67 insertions(+), 22 deletions(-)

-- 
2.17.1

[PATCH 1/5] ath9k: Fix use-after-free Read in htc_connect_service

[Qiujun Huang]

The skb is consumed by htc_send_epid, so it needn't release again.

The case reported by syzbot:

https://lore.kernel.org/linux-usb/000000000000590f6b05a1c05d15@google.com
usb 1-1: ath9k_htc: Firmware ath9k_htc/htc_9271-1.4.0.fw requested
usb 1-1: ath9k_htc: Transferred FW: ath9k_htc/htc_9271-1.4.0.fw, size:
51008
usb 1-1: Service connection timeout for: 256
==================================================================
BUG: KASAN: use-after-free in atomic_read
include/asm-generic/atomic-instrumented.h:26 [inline]
BUG: KASAN: use-after-free in refcount_read include/linux/refcount.h:134
[inline]
BUG: KASAN: use-after-free in skb_unref include/linux/skbuff.h:1042
[inline]
BUG: KASAN: use-after-free in kfree_skb+0x32/0x3d0 net/core/skbuff.c:692
Read of size 4 at addr ffff8881d0957994 by task kworker/1:2/83

Call Trace:
kfree_skb+0x32/0x3d0 net/core/skbuff.c:692
htc_connect_service.cold+0xa9/0x109
drivers/net/wireless/ath/ath9k/htc_hst.c:282
ath9k_wmi_connect+0xd2/0x1a0 drivers/net/wireless/ath/ath9k/wmi.c:265
ath9k_init_htc_services.constprop.0+0xb4/0x650
drivers/net/wireless/ath/ath9k/htc_drv_init.c:146
ath9k_htc_probe_device+0x25a/0x1d80
drivers/net/wireless/ath/ath9k/htc_drv_init.c:959
ath9k_htc_hw_init+0x31/0x60
drivers/net/wireless/ath/ath9k/htc_hst.c:501
ath9k_hif_usb_firmware_cb+0x26b/0x500
drivers/net/wireless/ath/ath9k/hif_usb.c:1187
request_firmware_work_func+0x126/0x242
drivers/base/firmware_loader/main.c:976
process_one_work+0x94b/0x1620 kernel/workqueue.c:2264
worker_thread+0x96/0xe20 kernel/workqueue.c:2410
kthread+0x318/0x420 kernel/kthread.c:255
ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352

Allocated by task 83:
kmem_cache_alloc_node+0xdc/0x330 mm/slub.c:2814
__alloc_skb+0xba/0x5a0 net/core/skbuff.c:198
alloc_skb include/linux/skbuff.h:1081 [inline]
htc_connect_service+0x2cc/0x840
drivers/net/wireless/ath/ath9k/htc_hst.c:257
ath9k_wmi_connect+0xd2/0x1a0 drivers/net/wireless/ath/ath9k/wmi.c:265
ath9k_init_htc_services.constprop.0+0xb4/0x650
drivers/net/wireless/ath/ath9k/htc_drv_init.c:146
ath9k_htc_probe_device+0x25a/0x1d80
drivers/net/wireless/ath/ath9k/htc_drv_init.c:959
ath9k_htc_hw_init+0x31/0x60
drivers/net/wireless/ath/ath9k/htc_hst.c:501
ath9k_hif_usb_firmware_cb+0x26b/0x500
drivers/net/wireless/ath/ath9k/hif_usb.c:1187
request_firmware_work_func+0x126/0x242
drivers/base/firmware_loader/main.c:976
process_one_work+0x94b/0x1620 kernel/workqueue.c:2264
worker_thread+0x96/0xe20 kernel/workqueue.c:2410
kthread+0x318/0x420 kernel/kthread.c:255
ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352

Freed by task 0:
kfree_skb+0x102/0x3d0 net/core/skbuff.c:690
ath9k_htc_txcompletion_cb+0x1f8/0x2b0
drivers/net/wireless/ath/ath9k/htc_hst.c:356
hif_usb_regout_cb+0x10b/0x1b0
drivers/net/wireless/ath/ath9k/hif_usb.c:90
__usb_hcd_giveback_urb+0x29a/0x550 drivers/usb/core/hcd.c:1650
usb_hcd_giveback_urb+0x368/0x420 drivers/usb/core/hcd.c:1716
dummy_timer+0x1258/0x32ae drivers/usb/gadget/udc/dummy_hcd.c:1966
call_timer_fn+0x195/0x6f0 kernel/time/timer.c:1404
expire_timers kernel/time/timer.c:1449 [inline]
__run_timers kernel/time/timer.c:1773 [inline]
__run_timers kernel/time/timer.c:1740 [inline]
run_timer_softirq+0x5f9/0x1500 kernel/time/timer.c:1786
__do_softirq+0x21e/0x950 kernel/softirq.c:292

Reported-and-tested-by: syzbot+9505af1ae303dabdc646@syzkaller.appspotmail.com
Signed-off-by: Qiujun Huang <hqjagain@gmail.com>
---
 drivers/net/wireless/ath/ath9k/htc_hst.c | 3 ---
 drivers/net/wireless/ath/ath9k/wmi.c     | 1 -
 2 files changed, 4 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/htc_hst.c b/drivers/net/wireless/ath/ath9k/htc_hst.c
index d091c8ebdcf0..1bf63a4efb4c 100644
--- a/drivers/net/wireless/ath/ath9k/htc_hst.c
+++ b/drivers/net/wireless/ath/ath9k/htc_hst.c
@@ -170,7 +170,6 @@ static int htc_config_pipe_credits(struct htc_target *target)
 	time_left = wait_for_completion_timeout(&target->cmd_wait, HZ);
 	if (!time_left) {
 		dev_err(target->dev, "HTC credit config timeout\n");
-		kfree_skb(skb);
 		return -ETIMEDOUT;
 	}
 
@@ -206,7 +205,6 @@ static int htc_setup_complete(struct htc_target *target)
 	time_left = wait_for_completion_timeout(&target->cmd_wait, HZ);
 	if (!time_left) {
 		dev_err(target->dev, "HTC start timeout\n");
-		kfree_skb(skb);
 		return -ETIMEDOUT;
 	}
 
@@ -279,7 +277,6 @@ int htc_connect_service(struct htc_target *target,
 	if (!time_left) {
 		dev_err(target->dev, "Service connection timeout for: %d\n",
 			service_connreq->service_id);
-		kfree_skb(skb);
 		return -ETIMEDOUT;
 	}
 
diff --git a/drivers/net/wireless/ath/ath9k/wmi.c b/drivers/net/wireless/ath/ath9k/wmi.c
index cdc146091194..d1f6710ca63b 100644
--- a/drivers/net/wireless/ath/ath9k/wmi.c
+++ b/drivers/net/wireless/ath/ath9k/wmi.c
@@ -336,7 +336,6 @@ int ath9k_wmi_cmd(struct wmi *wmi, enum wmi_cmd_id cmd_id,
 		ath_dbg(common, WMI, "Timeout waiting for WMI command: %s\n",
 			wmi_cmd_to_name(cmd_id));
 		mutex_unlock(&wmi->op_mutex);
-		kfree_skb(skb);
 		return -ETIMEDOUT;
 	}
 
-- 
2.17.1

Re: [PATCH 1/5] ath9k: Fix use-after-free Read in htc_connect_service

[Kalle Valo]

Qiujun Huang <hqjagain@gmail.com> wrote:

> The skb is consumed by htc_send_epid, so it needn't release again.
> 
> The case reported by syzbot:
> 
> https://lore.kernel.org/linux-usb/000000000000590f6b05a1c05d15@google.com
> usb 1-1: ath9k_htc: Firmware ath9k_htc/htc_9271-1.4.0.fw requested
> usb 1-1: ath9k_htc: Transferred FW: ath9k_htc/htc_9271-1.4.0.fw, size:
> 51008
> usb 1-1: Service connection timeout for: 256
> ==================================================================
> BUG: KASAN: use-after-free in atomic_read
> include/asm-generic/atomic-instrumented.h:26 [inline]
> BUG: KASAN: use-after-free in refcount_read include/linux/refcount.h:134
> [inline]
> BUG: KASAN: use-after-free in skb_unref include/linux/skbuff.h:1042
> [inline]
> BUG: KASAN: use-after-free in kfree_skb+0x32/0x3d0 net/core/skbuff.c:692
> Read of size 4 at addr ffff8881d0957994 by task kworker/1:2/83
> 
> Call Trace:
> kfree_skb+0x32/0x3d0 net/core/skbuff.c:692
> htc_connect_service.cold+0xa9/0x109
> drivers/net/wireless/ath/ath9k/htc_hst.c:282
> ath9k_wmi_connect+0xd2/0x1a0 drivers/net/wireless/ath/ath9k/wmi.c:265
> ath9k_init_htc_services.constprop.0+0xb4/0x650
> drivers/net/wireless/ath/ath9k/htc_drv_init.c:146
> ath9k_htc_probe_device+0x25a/0x1d80
> drivers/net/wireless/ath/ath9k/htc_drv_init.c:959
> ath9k_htc_hw_init+0x31/0x60
> drivers/net/wireless/ath/ath9k/htc_hst.c:501
> ath9k_hif_usb_firmware_cb+0x26b/0x500
> drivers/net/wireless/ath/ath9k/hif_usb.c:1187
> request_firmware_work_func+0x126/0x242
> drivers/base/firmware_loader/main.c:976
> process_one_work+0x94b/0x1620 kernel/workqueue.c:2264
> worker_thread+0x96/0xe20 kernel/workqueue.c:2410
> kthread+0x318/0x420 kernel/kthread.c:255
> ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
> 
> Allocated by task 83:
> kmem_cache_alloc_node+0xdc/0x330 mm/slub.c:2814
> __alloc_skb+0xba/0x5a0 net/core/skbuff.c:198
> alloc_skb include/linux/skbuff.h:1081 [inline]
> htc_connect_service+0x2cc/0x840
> drivers/net/wireless/ath/ath9k/htc_hst.c:257
> ath9k_wmi_connect+0xd2/0x1a0 drivers/net/wireless/ath/ath9k/wmi.c:265
> ath9k_init_htc_services.constprop.0+0xb4/0x650
> drivers/net/wireless/ath/ath9k/htc_drv_init.c:146
> ath9k_htc_probe_device+0x25a/0x1d80
> drivers/net/wireless/ath/ath9k/htc_drv_init.c:959
> ath9k_htc_hw_init+0x31/0x60
> drivers/net/wireless/ath/ath9k/htc_hst.c:501
> ath9k_hif_usb_firmware_cb+0x26b/0x500
> drivers/net/wireless/ath/ath9k/hif_usb.c:1187
> request_firmware_work_func+0x126/0x242
> drivers/base/firmware_loader/main.c:976
> process_one_work+0x94b/0x1620 kernel/workqueue.c:2264
> worker_thread+0x96/0xe20 kernel/workqueue.c:2410
> kthread+0x318/0x420 kernel/kthread.c:255
> ret_from_fork+0x24/0x30 arch/x86/entry/entry_64.S:352
> 
> Freed by task 0:
> kfree_skb+0x102/0x3d0 net/core/skbuff.c:690
> ath9k_htc_txcompletion_cb+0x1f8/0x2b0
> drivers/net/wireless/ath/ath9k/htc_hst.c:356
> hif_usb_regout_cb+0x10b/0x1b0
> drivers/net/wireless/ath/ath9k/hif_usb.c:90
> __usb_hcd_giveback_urb+0x29a/0x550 drivers/usb/core/hcd.c:1650
> usb_hcd_giveback_urb+0x368/0x420 drivers/usb/core/hcd.c:1716
> dummy_timer+0x1258/0x32ae drivers/usb/gadget/udc/dummy_hcd.c:1966
> call_timer_fn+0x195/0x6f0 kernel/time/timer.c:1404
> expire_timers kernel/time/timer.c:1449 [inline]
> __run_timers kernel/time/timer.c:1773 [inline]
> __run_timers kernel/time/timer.c:1740 [inline]
> run_timer_softirq+0x5f9/0x1500 kernel/time/timer.c:1786
> __do_softirq+0x21e/0x950 kernel/softirq.c:292
> 
> Reported-and-tested-by: syzbot+9505af1ae303dabdc646@syzkaller.appspotmail.com
> Signed-off-by: Qiujun Huang <hqjagain@gmail.com>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>

5 patches applied to ath-next branch of ath.git, thanks.

ced21a4c726b ath9k: Fix use-after-free Read in htc_connect_service
abeaa85054ff ath9k: Fix use-after-free Read in ath9k_wmi_ctrl_rx
e4ff08a4d727 ath9k: Fix use-after-free Write in ath9k_htc_rx_msg
19d6c375d671 ath9x: Fix stack-out-of-bounds Write in ath9k_hif_usb_rx_cb
2bbcaaee1fcb ath9k: Fix general protection fault in ath9k_hif_usb_rx_cb

-- 
https://patchwork.kernel.org/patch/11474039/

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

Re: [PATCH 1/5] ath9k: Fix use-after-free Read in htc_connect_service

[Dan Carpenter]

This patch is correct but there are a lot of error paths in
hif_usb_send() which don't free the skb.  Some error paths *do* free
the skb but most don't.  It's really a lot of work to sort through and
fix.

regards,
dan carpenter

[PATCH 2/5] ath9k: Fix use-after-free Read in ath9k_wmi_ctrl_rx

[Qiujun Huang]

Free wmi later after cmd urb has been killed, as urb cb will access wmi.

the case reported by syzbot:
https://lore.kernel.org/linux-usb/0000000000000002fc05a1d61a68@google.com
BUG: KASAN: use-after-free in ath9k_wmi_ctrl_rx+0x416/0x500
drivers/net/wireless/ath/ath9k/wmi.c:215
Read of size 1 at addr ffff8881cef1417c by task swapper/1/0

Call Trace:
<IRQ>
ath9k_wmi_ctrl_rx+0x416/0x500 drivers/net/wireless/ath/ath9k/wmi.c:215
ath9k_htc_rx_msg+0x2da/0xaf0
drivers/net/wireless/ath/ath9k/htc_hst.c:459
ath9k_hif_usb_reg_in_cb+0x1ba/0x630
drivers/net/wireless/ath/ath9k/hif_usb.c:718
__usb_hcd_giveback_urb+0x29a/0x550 drivers/usb/core/hcd.c:1650
usb_hcd_giveback_urb+0x368/0x420 drivers/usb/core/hcd.c:1716
dummy_timer+0x1258/0x32ae drivers/usb/gadget/udc/dummy_hcd.c:1966
call_timer_fn+0x195/0x6f0 kernel/time/timer.c:1404
expire_timers kernel/time/timer.c:1449 [inline]
__run_timers kernel/time/timer.c:1773 [inline]
__run_timers kernel/time/timer.c:1740 [inline]
run_timer_softirq+0x5f9/0x1500 kernel/time/timer.c:1786

Reported-and-tested-by: syzbot+5d338854440137ea0fef@syzkaller.appspotmail.com
Signed-off-by: Qiujun Huang <hqjagain@gmail.com>
---
 drivers/net/wireless/ath/ath9k/hif_usb.c      |  5 +++--
 drivers/net/wireless/ath/ath9k/hif_usb.h      |  1 +
 drivers/net/wireless/ath/ath9k/htc_drv_init.c | 10 +++++++---
 drivers/net/wireless/ath/ath9k/wmi.c          |  5 ++++-
 drivers/net/wireless/ath/ath9k/wmi.h          |  3 ++-
 5 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.c b/drivers/net/wireless/ath/ath9k/hif_usb.c
index dd0c32379375..f227e19087ff 100644
--- a/drivers/net/wireless/ath/ath9k/hif_usb.c
+++ b/drivers/net/wireless/ath/ath9k/hif_usb.c
@@ -973,7 +973,7 @@ static int ath9k_hif_usb_alloc_urbs(struct hif_device_usb *hif_dev)
 	return -ENOMEM;
 }
 
-static void ath9k_hif_usb_dealloc_urbs(struct hif_device_usb *hif_dev)
+void ath9k_hif_usb_dealloc_urbs(struct hif_device_usb *hif_dev)
 {
 	usb_kill_anchored_urbs(&hif_dev->regout_submitted);
 	ath9k_hif_usb_dealloc_reg_in_urbs(hif_dev);
@@ -1341,8 +1341,9 @@ static void ath9k_hif_usb_disconnect(struct usb_interface *interface)
 
 	if (hif_dev->flags & HIF_USB_READY) {
 		ath9k_htc_hw_deinit(hif_dev->htc_handle, unplugged);
-		ath9k_htc_hw_free(hif_dev->htc_handle);
 		ath9k_hif_usb_dev_deinit(hif_dev);
+		ath9k_destoy_wmi(hif_dev->htc_handle->drv_priv);
+		ath9k_htc_hw_free(hif_dev->htc_handle);
 	}
 
 	usb_set_intfdata(interface, NULL);
diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.h b/drivers/net/wireless/ath/ath9k/hif_usb.h
index 7846916aa01d..a94e7e1c86e9 100644
--- a/drivers/net/wireless/ath/ath9k/hif_usb.h
+++ b/drivers/net/wireless/ath/ath9k/hif_usb.h
@@ -133,5 +133,6 @@ struct hif_device_usb {
 
 int ath9k_hif_usb_init(void);
 void ath9k_hif_usb_exit(void);
+void ath9k_hif_usb_dealloc_urbs(struct hif_device_usb *hif_dev);
 
 #endif /* HTC_USB_H */
diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_init.c b/drivers/net/wireless/ath/ath9k/htc_drv_init.c
index d961095ab01f..40a065028ebe 100644
--- a/drivers/net/wireless/ath/ath9k/htc_drv_init.c
+++ b/drivers/net/wireless/ath/ath9k/htc_drv_init.c
@@ -931,8 +931,9 @@ static int ath9k_init_device(struct ath9k_htc_priv *priv,
 int ath9k_htc_probe_device(struct htc_target *htc_handle, struct device *dev,
 			   u16 devid, char *product, u32 drv_info)
 {
-	struct ieee80211_hw *hw;
+	struct hif_device_usb *hif_dev;
 	struct ath9k_htc_priv *priv;
+	struct ieee80211_hw *hw;
 	int ret;
 
 	hw = ieee80211_alloc_hw(sizeof(struct ath9k_htc_priv), &ath9k_htc_ops);
@@ -967,7 +968,10 @@ int ath9k_htc_probe_device(struct htc_target *htc_handle, struct device *dev,
 	return 0;
 
 err_init:
-	ath9k_deinit_wmi(priv);
+	ath9k_stop_wmi(priv);
+	hif_dev = (struct hif_device_usb *)htc_handle->hif_dev;
+	ath9k_hif_usb_dealloc_urbs(hif_dev);
+	ath9k_destoy_wmi(priv);
 err_free:
 	ieee80211_free_hw(hw);
 	return ret;
@@ -982,7 +986,7 @@ void ath9k_htc_disconnect_device(struct htc_target *htc_handle, bool hotunplug)
 			htc_handle->drv_priv->ah->ah_flags |= AH_UNPLUGGED;
 
 		ath9k_deinit_device(htc_handle->drv_priv);
-		ath9k_deinit_wmi(htc_handle->drv_priv);
+		ath9k_stop_wmi(htc_handle->drv_priv);
 		ieee80211_free_hw(htc_handle->drv_priv->hw);
 	}
 }
diff --git a/drivers/net/wireless/ath/ath9k/wmi.c b/drivers/net/wireless/ath/ath9k/wmi.c
index d1f6710ca63b..e7a3127395be 100644
--- a/drivers/net/wireless/ath/ath9k/wmi.c
+++ b/drivers/net/wireless/ath/ath9k/wmi.c
@@ -112,14 +112,17 @@ struct wmi *ath9k_init_wmi(struct ath9k_htc_priv *priv)
 	return wmi;
 }
 
-void ath9k_deinit_wmi(struct ath9k_htc_priv *priv)
+void ath9k_stop_wmi(struct ath9k_htc_priv *priv)
 {
 	struct wmi *wmi = priv->wmi;
 
 	mutex_lock(&wmi->op_mutex);
 	wmi->stopped = true;
 	mutex_unlock(&wmi->op_mutex);
+}
 
+void ath9k_destoy_wmi(struct ath9k_htc_priv *priv)
+{
 	kfree(priv->wmi);
 }
 
diff --git a/drivers/net/wireless/ath/ath9k/wmi.h b/drivers/net/wireless/ath/ath9k/wmi.h
index 380175d5ecd7..d8b912206232 100644
--- a/drivers/net/wireless/ath/ath9k/wmi.h
+++ b/drivers/net/wireless/ath/ath9k/wmi.h
@@ -179,7 +179,6 @@ struct wmi {
 };
 
 struct wmi *ath9k_init_wmi(struct ath9k_htc_priv *priv);
-void ath9k_deinit_wmi(struct ath9k_htc_priv *priv);
 int ath9k_wmi_connect(struct htc_target *htc, struct wmi *wmi,
 		      enum htc_endpoint_id *wmi_ctrl_epid);
 int ath9k_wmi_cmd(struct wmi *wmi, enum wmi_cmd_id cmd_id,
@@ -189,6 +188,8 @@ int ath9k_wmi_cmd(struct wmi *wmi, enum wmi_cmd_id cmd_id,
 void ath9k_wmi_event_tasklet(unsigned long data);
 void ath9k_fatal_work(struct work_struct *work);
 void ath9k_wmi_event_drain(struct ath9k_htc_priv *priv);
+void ath9k_stop_wmi(struct ath9k_htc_priv *priv);
+void ath9k_destoy_wmi(struct ath9k_htc_priv *priv);
 
 #define WMI_CMD(_wmi_cmd)						\
 	do {								\
-- 
2.17.1

[PATCH 3/5] ath9k: Fix use-after-free Write in ath9k_htc_rx_msg

[Qiujun Huang]

Write out of slab bounds. We should check epid.

The case reported by syzbot:
https://lore.kernel.org/linux-usb/0000000000006ac55b05a1c05d72@google.com
BUG: KASAN: use-after-free in htc_process_conn_rsp
drivers/net/wireless/ath/ath9k/htc_hst.c:131 [inline]
BUG: KASAN: use-after-free in ath9k_htc_rx_msg+0xa25/0xaf0
drivers/net/wireless/ath/ath9k/htc_hst.c:443
Write of size 2 at addr ffff8881cea291f0 by task swapper/1/0

Call Trace:
 htc_process_conn_rsp drivers/net/wireless/ath/ath9k/htc_hst.c:131
[inline]
ath9k_htc_rx_msg+0xa25/0xaf0
drivers/net/wireless/ath/ath9k/htc_hst.c:443
ath9k_hif_usb_reg_in_cb+0x1ba/0x630
drivers/net/wireless/ath/ath9k/hif_usb.c:718
__usb_hcd_giveback_urb+0x29a/0x550 drivers/usb/core/hcd.c:1650
usb_hcd_giveback_urb+0x368/0x420 drivers/usb/core/hcd.c:1716
dummy_timer+0x1258/0x32ae drivers/usb/gadget/udc/dummy_hcd.c:1966
call_timer_fn+0x195/0x6f0 kernel/time/timer.c:1404
expire_timers kernel/time/timer.c:1449 [inline]
__run_timers kernel/time/timer.c:1773 [inline]
__run_timers kernel/time/timer.c:1740 [inline]
run_timer_softirq+0x5f9/0x1500 kernel/time/timer.c:1786

Reported-and-tested-by: syzbot+b1c61e5f11be5782f192@syzkaller.appspotmail.com
Signed-off-by: Qiujun Huang <hqjagain@gmail.com>
---
 drivers/net/wireless/ath/ath9k/htc_hst.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/wireless/ath/ath9k/htc_hst.c b/drivers/net/wireless/ath/ath9k/htc_hst.c
index 1bf63a4efb4c..d2e062eaf561 100644
--- a/drivers/net/wireless/ath/ath9k/htc_hst.c
+++ b/drivers/net/wireless/ath/ath9k/htc_hst.c
@@ -113,6 +113,9 @@ static void htc_process_conn_rsp(struct htc_target *target,
 
 	if (svc_rspmsg->status == HTC_SERVICE_SUCCESS) {
 		epid = svc_rspmsg->endpoint_id;
+		if (epid < 0 || epid >= ENDPOINT_MAX)
+			return;
+
 		service_id = be16_to_cpu(svc_rspmsg->service_id);
 		max_msglen = be16_to_cpu(svc_rspmsg->max_msg_len);
 		endpoint = &target->endpoint[epid];
-- 
2.17.1

[PATCH 4/5 resend] ath9x: Fix stack-out-of-bounds Write in ath9k_hif_usb_rx_cb

[Qiujun Huang]

Add barrier to accessing the stack array skb_pool.

The case reported by syzbot:
https://lore.kernel.org/linux-usb/0000000000003d7c1505a2168418@google.com
BUG: KASAN: stack-out-of-bounds in ath9k_hif_usb_rx_stream
drivers/net/wireless/ath/ath9k/hif_usb.c:626 [inline]
BUG: KASAN: stack-out-of-bounds in ath9k_hif_usb_rx_cb+0xdf6/0xf70
drivers/net/wireless/ath/ath9k/hif_usb.c:666
Write of size 8 at addr ffff8881db309a28 by task swapper/1/0

Call Trace:
ath9k_hif_usb_rx_stream drivers/net/wireless/ath/ath9k/hif_usb.c:626
[inline]
ath9k_hif_usb_rx_cb+0xdf6/0xf70
drivers/net/wireless/ath/ath9k/hif_usb.c:666
__usb_hcd_giveback_urb+0x1f2/0x470 drivers/usb/core/hcd.c:1648
usb_hcd_giveback_urb+0x368/0x420 drivers/usb/core/hcd.c:1713
dummy_timer+0x1258/0x32ae drivers/usb/gadget/udc/dummy_hcd.c:1966
call_timer_fn+0x195/0x6f0 kernel/time/timer.c:1404
expire_timers kernel/time/timer.c:1449 [inline]
__run_timers kernel/time/timer.c:1773 [inline]
__run_timers kernel/time/timer.c:1740 [inline]
run_timer_softirq+0x5f9/0x1500 kernel/time/timer.c:1786

Reported-and-tested-by: syzbot+d403396d4df67ad0bd5f@syzkaller.appspotmail.com
Signed-off-by: Qiujun Huang <hqjagain@gmail.com>
---
 drivers/net/wireless/ath/ath9k/hif_usb.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.c b/drivers/net/wireless/ath/ath9k/hif_usb.c
index f227e19087ff..6049d3766c64 100644
--- a/drivers/net/wireless/ath/ath9k/hif_usb.c
+++ b/drivers/net/wireless/ath/ath9k/hif_usb.c
@@ -612,6 +612,11 @@ static void ath9k_hif_usb_rx_stream(struct hif_device_usb *hif_dev,
 			hif_dev->remain_skb = nskb;
 			spin_unlock(&hif_dev->rx_lock);
 		} else {
+			if (pool_index == MAX_PKT_NUM_IN_TRANSFER) {
+				dev_err(&hif_dev->udev->dev,
+					"ath9k_htc: over RX MAX_PKT_NUM\n");
+				goto err;
+			}
 			nskb = __dev_alloc_skb(pkt_len + 32, GFP_ATOMIC);
 			if (!nskb) {
 				dev_err(&hif_dev->udev->dev,
-- 
2.17.1

[PATCH 5/5] ath9k: Fix general protection fault in ath9k_hif_usb_rx_cb

[Qiujun Huang]

In ath9k_hif_usb_rx_cb interface number is assumed to be 0.
usb_ifnum_to_if(urb->dev, 0)
But it isn't always true.

The case reported by syzbot:
https://lore.kernel.org/linux-usb/000000000000666c9c05a1c05d12@google.com
usb 2-1: new high-speed USB device number 2 using dummy_hcd
usb 2-1: config 1 has an invalid interface number: 2 but max is 0
usb 2-1: config 1 has no interface number 0
usb 2-1: New USB device found, idVendor=0cf3, idProduct=9271, bcdDevice=
1.08
usb 2-1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
general protection fault, probably for non-canonical address
0xdffffc0000000015: 0000 [#1] SMP KASAN
KASAN: null-ptr-deref in range [0x00000000000000a8-0x00000000000000af]
CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.6.0-rc5-syzkaller #0

Call Trace
__usb_hcd_giveback_urb+0x29a/0x550 drivers/usb/core/hcd.c:1650
usb_hcd_giveback_urb+0x368/0x420 drivers/usb/core/hcd.c:1716
dummy_timer+0x1258/0x32ae drivers/usb/gadget/udc/dummy_hcd.c:1966
call_timer_fn+0x195/0x6f0 kernel/time/timer.c:1404
expire_timers kernel/time/timer.c:1449 [inline]
__run_timers kernel/time/timer.c:1773 [inline]
__run_timers kernel/time/timer.c:1740 [inline]
run_timer_softirq+0x5f9/0x1500 kernel/time/timer.c:1786
__do_softirq+0x21e/0x950 kernel/softirq.c:292
invoke_softirq kernel/softirq.c:373 [inline]
irq_exit+0x178/0x1a0 kernel/softirq.c:413
exiting_irq arch/x86/include/asm/apic.h:546 [inline]
smp_apic_timer_interrupt+0x141/0x540 arch/x86/kernel/apic/apic.c:1146
apic_timer_interrupt+0xf/0x20 arch/x86/entry/entry_64.S:829

Reported-and-tested-by: syzbot+40d5d2e8a4680952f042@syzkaller.appspotmail.com
Signed-off-by: Qiujun Huang <hqjagain@gmail.com>
---
 drivers/net/wireless/ath/ath9k/hif_usb.c | 48 ++++++++++++++++++------
 drivers/net/wireless/ath/ath9k/hif_usb.h |  5 +++
 2 files changed, 42 insertions(+), 11 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.c b/drivers/net/wireless/ath/ath9k/hif_usb.c
index 6049d3766c64..4ed21dad6a8e 100644
--- a/drivers/net/wireless/ath/ath9k/hif_usb.c
+++ b/drivers/net/wireless/ath/ath9k/hif_usb.c
@@ -643,9 +643,9 @@ static void ath9k_hif_usb_rx_stream(struct hif_device_usb *hif_dev,
 
 static void ath9k_hif_usb_rx_cb(struct urb *urb)
 {
-	struct sk_buff *skb = (struct sk_buff *) urb->context;
-	struct hif_device_usb *hif_dev =
-		usb_get_intfdata(usb_ifnum_to_if(urb->dev, 0));
+	struct rx_buf *rx_buf = (struct rx_buf *)urb->context;
+	struct hif_device_usb *hif_dev = rx_buf->hif_dev;
+	struct sk_buff *skb = rx_buf->skb;
 	int ret;
 
 	if (!skb)
@@ -685,14 +685,15 @@ static void ath9k_hif_usb_rx_cb(struct urb *urb)
 	return;
 free:
 	kfree_skb(skb);
+	kfree(rx_buf);
 }
 
 static void ath9k_hif_usb_reg_in_cb(struct urb *urb)
 {
-	struct sk_buff *skb = (struct sk_buff *) urb->context;
+	struct rx_buf *rx_buf = (struct rx_buf *)urb->context;
+	struct hif_device_usb *hif_dev = rx_buf->hif_dev;
+	struct sk_buff *skb = rx_buf->skb;
 	struct sk_buff *nskb;
-	struct hif_device_usb *hif_dev =
-		usb_get_intfdata(usb_ifnum_to_if(urb->dev, 0));
 	int ret;
 
 	if (!skb)
@@ -750,6 +751,7 @@ static void ath9k_hif_usb_reg_in_cb(struct urb *urb)
 	return;
 free:
 	kfree_skb(skb);
+	kfree(rx_buf);
 	urb->context = NULL;
 }
 
@@ -795,7 +797,7 @@ static int ath9k_hif_usb_alloc_tx_urbs(struct hif_device_usb *hif_dev)
 	init_usb_anchor(&hif_dev->mgmt_submitted);
 
 	for (i = 0; i < MAX_TX_URB_NUM; i++) {
-		tx_buf = kzalloc(sizeof(struct tx_buf), GFP_KERNEL);
+		tx_buf = kzalloc(sizeof(*tx_buf), GFP_KERNEL);
 		if (!tx_buf)
 			goto err;
 
@@ -832,8 +834,9 @@ static void ath9k_hif_usb_dealloc_rx_urbs(struct hif_device_usb *hif_dev)
 
 static int ath9k_hif_usb_alloc_rx_urbs(struct hif_device_usb *hif_dev)
 {
-	struct urb *urb = NULL;
+	struct rx_buf *rx_buf = NULL;
 	struct sk_buff *skb = NULL;
+	struct urb *urb = NULL;
 	int i, ret;
 
 	init_usb_anchor(&hif_dev->rx_submitted);
@@ -841,6 +844,12 @@ static int ath9k_hif_usb_alloc_rx_urbs(struct hif_device_usb *hif_dev)
 
 	for (i = 0; i < MAX_RX_URB_NUM; i++) {
 
+		rx_buf = kzalloc(sizeof(*rx_buf), GFP_KERNEL);
+		if (!rx_buf) {
+			ret = -ENOMEM;
+			goto err_rxb;
+		}
+
 		/* Allocate URB */
 		urb = usb_alloc_urb(0, GFP_KERNEL);
 		if (urb == NULL) {
@@ -855,11 +864,14 @@ static int ath9k_hif_usb_alloc_rx_urbs(struct hif_device_usb *hif_dev)
 			goto err_skb;
 		}
 
+		rx_buf->hif_dev = hif_dev;
+		rx_buf->skb = skb;
+
 		usb_fill_bulk_urb(urb, hif_dev->udev,
 				  usb_rcvbulkpipe(hif_dev->udev,
 						  USB_WLAN_RX_PIPE),
 				  skb->data, MAX_RX_BUF_SIZE,
-				  ath9k_hif_usb_rx_cb, skb);
+				  ath9k_hif_usb_rx_cb, rx_buf);
 
 		/* Anchor URB */
 		usb_anchor_urb(urb, &hif_dev->rx_submitted);
@@ -885,6 +897,8 @@ static int ath9k_hif_usb_alloc_rx_urbs(struct hif_device_usb *hif_dev)
 err_skb:
 	usb_free_urb(urb);
 err_urb:
+	kfree(rx_buf);
+err_rxb:
 	ath9k_hif_usb_dealloc_rx_urbs(hif_dev);
 	return ret;
 }
@@ -896,14 +910,21 @@ static void ath9k_hif_usb_dealloc_reg_in_urbs(struct hif_device_usb *hif_dev)
 
 static int ath9k_hif_usb_alloc_reg_in_urbs(struct hif_device_usb *hif_dev)
 {
-	struct urb *urb = NULL;
+	struct rx_buf *rx_buf = NULL;
 	struct sk_buff *skb = NULL;
+	struct urb *urb = NULL;
 	int i, ret;
 
 	init_usb_anchor(&hif_dev->reg_in_submitted);
 
 	for (i = 0; i < MAX_REG_IN_URB_NUM; i++) {
 
+		rx_buf = kzalloc(sizeof(*rx_buf), GFP_KERNEL);
+		if (!rx_buf) {
+			ret = -ENOMEM;
+			goto err_rxb;
+		}
+
 		/* Allocate URB */
 		urb = usb_alloc_urb(0, GFP_KERNEL);
 		if (urb == NULL) {
@@ -918,11 +939,14 @@ static int ath9k_hif_usb_alloc_reg_in_urbs(struct hif_device_usb *hif_dev)
 			goto err_skb;
 		}
 
+		rx_buf->hif_dev = hif_dev;
+		rx_buf->skb = skb;
+
 		usb_fill_int_urb(urb, hif_dev->udev,
 				  usb_rcvintpipe(hif_dev->udev,
 						  USB_REG_IN_PIPE),
 				  skb->data, MAX_REG_IN_BUF_SIZE,
-				  ath9k_hif_usb_reg_in_cb, skb, 1);
+				  ath9k_hif_usb_reg_in_cb, rx_buf, 1);
 
 		/* Anchor URB */
 		usb_anchor_urb(urb, &hif_dev->reg_in_submitted);
@@ -948,6 +972,8 @@ static int ath9k_hif_usb_alloc_reg_in_urbs(struct hif_device_usb *hif_dev)
 err_skb:
 	usb_free_urb(urb);
 err_urb:
+	kfree(rx_buf);
+err_rxb:
 	ath9k_hif_usb_dealloc_reg_in_urbs(hif_dev);
 	return ret;
 }
diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.h b/drivers/net/wireless/ath/ath9k/hif_usb.h
index a94e7e1c86e9..5985aa15ca93 100644
--- a/drivers/net/wireless/ath/ath9k/hif_usb.h
+++ b/drivers/net/wireless/ath/ath9k/hif_usb.h
@@ -86,6 +86,11 @@ struct tx_buf {
 	struct list_head list;
 };
 
+struct rx_buf {
+	struct sk_buff *skb;
+	struct hif_device_usb *hif_dev;
+};
+
 #define HIF_USB_TX_STOP  BIT(0)
 #define HIF_USB_TX_FLUSH BIT(1)
 
-- 
2.17.1

Re: [PATCH 5/5] ath9k: Fix general protection fault in ath9k_hif_usb_rx_cb

[Dan Carpenter]

On Sat, Apr 04, 2020 at 12:18:38PM +0800, Qiujun Huang wrote:
> In ath9k_hif_usb_rx_cb interface number is assumed to be 0.
> usb_ifnum_to_if(urb->dev, 0)
> But it isn't always true.
> 
> The case reported by syzbot:
> https://lore.kernel.org/linux-usb/000000000000666c9c05a1c05d12@google.com
> usb 2-1: new high-speed USB device number 2 using dummy_hcd
> usb 2-1: config 1 has an invalid interface number: 2 but max is 0
> usb 2-1: config 1 has no interface number 0
> usb 2-1: New USB device found, idVendor=0cf3, idProduct=9271, bcdDevice=
> 1.08
> usb 2-1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
> general protection fault, probably for non-canonical address
> 0xdffffc0000000015: 0000 [#1] SMP KASAN
> KASAN: null-ptr-deref in range [0x00000000000000a8-0x00000000000000af]
> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.6.0-rc5-syzkaller #0
> 
> Call Trace
> __usb_hcd_giveback_urb+0x29a/0x550 drivers/usb/core/hcd.c:1650
> usb_hcd_giveback_urb+0x368/0x420 drivers/usb/core/hcd.c:1716
> dummy_timer+0x1258/0x32ae drivers/usb/gadget/udc/dummy_hcd.c:1966
> call_timer_fn+0x195/0x6f0 kernel/time/timer.c:1404
> expire_timers kernel/time/timer.c:1449 [inline]
> __run_timers kernel/time/timer.c:1773 [inline]
> __run_timers kernel/time/timer.c:1740 [inline]
> run_timer_softirq+0x5f9/0x1500 kernel/time/timer.c:1786
> __do_softirq+0x21e/0x950 kernel/softirq.c:292
> invoke_softirq kernel/softirq.c:373 [inline]
> irq_exit+0x178/0x1a0 kernel/softirq.c:413
> exiting_irq arch/x86/include/asm/apic.h:546 [inline]
> smp_apic_timer_interrupt+0x141/0x540 arch/x86/kernel/apic/apic.c:1146
> apic_timer_interrupt+0xf/0x20 arch/x86/entry/entry_64.S:829
> 
> Reported-and-tested-by: syzbot+40d5d2e8a4680952f042@syzkaller.appspotmail.com
> Signed-off-by: Qiujun Huang <hqjagain@gmail.com>
> ---
>  drivers/net/wireless/ath/ath9k/hif_usb.c | 48 ++++++++++++++++++------
>  drivers/net/wireless/ath/ath9k/hif_usb.h |  5 +++
>  2 files changed, 42 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.c b/drivers/net/wireless/ath/ath9k/hif_usb.c
> index 6049d3766c64..4ed21dad6a8e 100644
> --- a/drivers/net/wireless/ath/ath9k/hif_usb.c
> +++ b/drivers/net/wireless/ath/ath9k/hif_usb.c
> @@ -643,9 +643,9 @@ static void ath9k_hif_usb_rx_stream(struct hif_device_usb *hif_dev,
>  
>  static void ath9k_hif_usb_rx_cb(struct urb *urb)
>  {
> -	struct sk_buff *skb = (struct sk_buff *) urb->context;
> -	struct hif_device_usb *hif_dev =
> -		usb_get_intfdata(usb_ifnum_to_if(urb->dev, 0));
> +	struct rx_buf *rx_buf = (struct rx_buf *)urb->context;
> +	struct hif_device_usb *hif_dev = rx_buf->hif_dev;
> +	struct sk_buff *skb = rx_buf->skb;
>  	int ret;
>  
>  	if (!skb)

This "if (!skb)" error path returns directly and leaks "rx_buf".
Of course, it's an impossible condition.  We should just delete the
check.

> @@ -685,14 +685,15 @@ static void ath9k_hif_usb_rx_cb(struct urb *urb)
>  	return;
>  free:
>  	kfree_skb(skb);
> +	kfree(rx_buf);
>  }
>  
>  static void ath9k_hif_usb_reg_in_cb(struct urb *urb)
>  {
> -	struct sk_buff *skb = (struct sk_buff *) urb->context;
> +	struct rx_buf *rx_buf = (struct rx_buf *)urb->context;
> +	struct hif_device_usb *hif_dev = rx_buf->hif_dev;
> +	struct sk_buff *skb = rx_buf->skb;
>  	struct sk_buff *nskb;
> -	struct hif_device_usb *hif_dev =
> -		usb_get_intfdata(usb_ifnum_to_if(urb->dev, 0));
>  	int ret;
>  
>  	if (!skb)

Same.

> @@ -750,6 +751,7 @@ static void ath9k_hif_usb_reg_in_cb(struct urb *urb)
>  	return;
>  free:
>  	kfree_skb(skb);
> +	kfree(rx_buf);
>  	urb->context = NULL;
>  }
>  

regards,
dan carpenter

[BISECTED REGRESSION] ath9k: Fix general protection fault in ath9k_hif_usb_rx_cb

[Roman Mamedov]

On Sat,  4 Apr 2020 12:18:38 +0800
Qiujun Huang <hqjagain@gmail.com> wrote:

> In ath9k_hif_usb_rx_cb interface number is assumed to be 0.
> usb_ifnum_to_if(urb->dev, 0)
> But it isn't always true.
> 
> The case reported by syzbot:
> https://lore.kernel.org/linux-usb/000000000000666c9c05a1c05d12@google.com
> usb 2-1: new high-speed USB device number 2 using dummy_hcd
> usb 2-1: config 1 has an invalid interface number: 2 but max is 0
> usb 2-1: config 1 has no interface number 0
> usb 2-1: New USB device found, idVendor=0cf3, idProduct=9271, bcdDevice=
> 1.08
> usb 2-1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
> general protection fault, probably for non-canonical address
> 0xdffffc0000000015: 0000 [#1] SMP KASAN
> KASAN: null-ptr-deref in range [0x00000000000000a8-0x00000000000000af]
> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.6.0-rc5-syzkaller #0
> 
> Call Trace
> __usb_hcd_giveback_urb+0x29a/0x550 drivers/usb/core/hcd.c:1650
> usb_hcd_giveback_urb+0x368/0x420 drivers/usb/core/hcd.c:1716
> dummy_timer+0x1258/0x32ae drivers/usb/gadget/udc/dummy_hcd.c:1966
> call_timer_fn+0x195/0x6f0 kernel/time/timer.c:1404
> expire_timers kernel/time/timer.c:1449 [inline]
> __run_timers kernel/time/timer.c:1773 [inline]
> __run_timers kernel/time/timer.c:1740 [inline]
> run_timer_softirq+0x5f9/0x1500 kernel/time/timer.c:1786
> __do_softirq+0x21e/0x950 kernel/softirq.c:292
> invoke_softirq kernel/softirq.c:373 [inline]
> irq_exit+0x178/0x1a0 kernel/softirq.c:413
> exiting_irq arch/x86/include/asm/apic.h:546 [inline]
> smp_apic_timer_interrupt+0x141/0x540 arch/x86/kernel/apic/apic.c:1146
> apic_timer_interrupt+0xf/0x20 arch/x86/entry/entry_64.S:829
> 
> Reported-and-tested-by: syzbot+40d5d2e8a4680952f042@syzkaller.appspotmail.com
> Signed-off-by: Qiujun Huang <hqjagain@gmail.com>

This causes complete breakage of ath9k operation across all the stable kernel
series it got backported to, and I guess the mainline as well. Please see:
https://bugzilla.kernel.org/show_bug.cgi?id=208251
https://bugzilla.redhat.com/show_bug.cgi?id=1848631

Thanks


> ---
>  drivers/net/wireless/ath/ath9k/hif_usb.c | 48 ++++++++++++++++++------
>  drivers/net/wireless/ath/ath9k/hif_usb.h |  5 +++
>  2 files changed, 42 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.c b/drivers/net/wireless/ath/ath9k/hif_usb.c
> index 6049d3766c64..4ed21dad6a8e 100644
> --- a/drivers/net/wireless/ath/ath9k/hif_usb.c
> +++ b/drivers/net/wireless/ath/ath9k/hif_usb.c
> @@ -643,9 +643,9 @@ static void ath9k_hif_usb_rx_stream(struct hif_device_usb *hif_dev,
>  
>  static void ath9k_hif_usb_rx_cb(struct urb *urb)
>  {
> -	struct sk_buff *skb = (struct sk_buff *) urb->context;
> -	struct hif_device_usb *hif_dev =
> -		usb_get_intfdata(usb_ifnum_to_if(urb->dev, 0));
> +	struct rx_buf *rx_buf = (struct rx_buf *)urb->context;
> +	struct hif_device_usb *hif_dev = rx_buf->hif_dev;
> +	struct sk_buff *skb = rx_buf->skb;
>  	int ret;
>  
>  	if (!skb)
> @@ -685,14 +685,15 @@ static void ath9k_hif_usb_rx_cb(struct urb *urb)
>  	return;
>  free:
>  	kfree_skb(skb);
> +	kfree(rx_buf);
>  }
>  
>  static void ath9k_hif_usb_reg_in_cb(struct urb *urb)
>  {
> -	struct sk_buff *skb = (struct sk_buff *) urb->context;
> +	struct rx_buf *rx_buf = (struct rx_buf *)urb->context;
> +	struct hif_device_usb *hif_dev = rx_buf->hif_dev;
> +	struct sk_buff *skb = rx_buf->skb;
>  	struct sk_buff *nskb;
> -	struct hif_device_usb *hif_dev =
> -		usb_get_intfdata(usb_ifnum_to_if(urb->dev, 0));
>  	int ret;
>  
>  	if (!skb)
> @@ -750,6 +751,7 @@ static void ath9k_hif_usb_reg_in_cb(struct urb *urb)
>  	return;
>  free:
>  	kfree_skb(skb);
> +	kfree(rx_buf);
>  	urb->context = NULL;
>  }
>  
> @@ -795,7 +797,7 @@ static int ath9k_hif_usb_alloc_tx_urbs(struct hif_device_usb *hif_dev)
>  	init_usb_anchor(&hif_dev->mgmt_submitted);
>  
>  	for (i = 0; i < MAX_TX_URB_NUM; i++) {
> -		tx_buf = kzalloc(sizeof(struct tx_buf), GFP_KERNEL);
> +		tx_buf = kzalloc(sizeof(*tx_buf), GFP_KERNEL);
>  		if (!tx_buf)
>  			goto err;
>  
> @@ -832,8 +834,9 @@ static void ath9k_hif_usb_dealloc_rx_urbs(struct hif_device_usb *hif_dev)
>  
>  static int ath9k_hif_usb_alloc_rx_urbs(struct hif_device_usb *hif_dev)
>  {
> -	struct urb *urb = NULL;
> +	struct rx_buf *rx_buf = NULL;
>  	struct sk_buff *skb = NULL;
> +	struct urb *urb = NULL;
>  	int i, ret;
>  
>  	init_usb_anchor(&hif_dev->rx_submitted);
> @@ -841,6 +844,12 @@ static int ath9k_hif_usb_alloc_rx_urbs(struct hif_device_usb *hif_dev)
>  
>  	for (i = 0; i < MAX_RX_URB_NUM; i++) {
>  
> +		rx_buf = kzalloc(sizeof(*rx_buf), GFP_KERNEL);
> +		if (!rx_buf) {
> +			ret = -ENOMEM;
> +			goto err_rxb;
> +		}
> +
>  		/* Allocate URB */
>  		urb = usb_alloc_urb(0, GFP_KERNEL);
>  		if (urb == NULL) {
> @@ -855,11 +864,14 @@ static int ath9k_hif_usb_alloc_rx_urbs(struct hif_device_usb *hif_dev)
>  			goto err_skb;
>  		}
>  
> +		rx_buf->hif_dev = hif_dev;
> +		rx_buf->skb = skb;
> +
>  		usb_fill_bulk_urb(urb, hif_dev->udev,
>  				  usb_rcvbulkpipe(hif_dev->udev,
>  						  USB_WLAN_RX_PIPE),
>  				  skb->data, MAX_RX_BUF_SIZE,
> -				  ath9k_hif_usb_rx_cb, skb);
> +				  ath9k_hif_usb_rx_cb, rx_buf);
>  
>  		/* Anchor URB */
>  		usb_anchor_urb(urb, &hif_dev->rx_submitted);
> @@ -885,6 +897,8 @@ static int ath9k_hif_usb_alloc_rx_urbs(struct hif_device_usb *hif_dev)
>  err_skb:
>  	usb_free_urb(urb);
>  err_urb:
> +	kfree(rx_buf);
> +err_rxb:
>  	ath9k_hif_usb_dealloc_rx_urbs(hif_dev);
>  	return ret;
>  }
> @@ -896,14 +910,21 @@ static void ath9k_hif_usb_dealloc_reg_in_urbs(struct hif_device_usb *hif_dev)
>  
>  static int ath9k_hif_usb_alloc_reg_in_urbs(struct hif_device_usb *hif_dev)
>  {
> -	struct urb *urb = NULL;
> +	struct rx_buf *rx_buf = NULL;
>  	struct sk_buff *skb = NULL;
> +	struct urb *urb = NULL;
>  	int i, ret;
>  
>  	init_usb_anchor(&hif_dev->reg_in_submitted);
>  
>  	for (i = 0; i < MAX_REG_IN_URB_NUM; i++) {
>  
> +		rx_buf = kzalloc(sizeof(*rx_buf), GFP_KERNEL);
> +		if (!rx_buf) {
> +			ret = -ENOMEM;
> +			goto err_rxb;
> +		}
> +
>  		/* Allocate URB */
>  		urb = usb_alloc_urb(0, GFP_KERNEL);
>  		if (urb == NULL) {
> @@ -918,11 +939,14 @@ static int ath9k_hif_usb_alloc_reg_in_urbs(struct hif_device_usb *hif_dev)
>  			goto err_skb;
>  		}
>  
> +		rx_buf->hif_dev = hif_dev;
> +		rx_buf->skb = skb;
> +
>  		usb_fill_int_urb(urb, hif_dev->udev,
>  				  usb_rcvintpipe(hif_dev->udev,
>  						  USB_REG_IN_PIPE),
>  				  skb->data, MAX_REG_IN_BUF_SIZE,
> -				  ath9k_hif_usb_reg_in_cb, skb, 1);
> +				  ath9k_hif_usb_reg_in_cb, rx_buf, 1);
>  
>  		/* Anchor URB */
>  		usb_anchor_urb(urb, &hif_dev->reg_in_submitted);
> @@ -948,6 +972,8 @@ static int ath9k_hif_usb_alloc_reg_in_urbs(struct hif_device_usb *hif_dev)
>  err_skb:
>  	usb_free_urb(urb);
>  err_urb:
> +	kfree(rx_buf);
> +err_rxb:
>  	ath9k_hif_usb_dealloc_reg_in_urbs(hif_dev);
>  	return ret;
>  }
> diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.h b/drivers/net/wireless/ath/ath9k/hif_usb.h
> index a94e7e1c86e9..5985aa15ca93 100644
> --- a/drivers/net/wireless/ath/ath9k/hif_usb.h
> +++ b/drivers/net/wireless/ath/ath9k/hif_usb.h
> @@ -86,6 +86,11 @@ struct tx_buf {
>  	struct list_head list;
>  };
>  
> +struct rx_buf {
> +	struct sk_buff *skb;
> +	struct hif_device_usb *hif_dev;
> +};
> +
>  #define HIF_USB_TX_STOP  BIT(0)
>  #define HIF_USB_TX_FLUSH BIT(1)
>  


-- 
With respect,
Roman

Re: [BISECTED REGRESSION] ath9k: Fix general protection fault in ath9k_hif_usb_rx_cb

[Kalle Valo]

Roman Mamedov <rm@romanrm.net> writes:

> On Sat,  4 Apr 2020 12:18:38 +0800
> Qiujun Huang <hqjagain@gmail.com> wrote:
>
>> In ath9k_hif_usb_rx_cb interface number is assumed to be 0.
>> usb_ifnum_to_if(urb->dev, 0)
>> But it isn't always true.
>> 
>> The case reported by syzbot:
>> https://lore.kernel.org/linux-usb/000000000000666c9c05a1c05d12@google.com
>> usb 2-1: new high-speed USB device number 2 using dummy_hcd
>> usb 2-1: config 1 has an invalid interface number: 2 but max is 0
>> usb 2-1: config 1 has no interface number 0
>> usb 2-1: New USB device found, idVendor=0cf3, idProduct=9271, bcdDevice=
>> 1.08
>> usb 2-1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
>> general protection fault, probably for non-canonical address
>> 0xdffffc0000000015: 0000 [#1] SMP KASAN
>> KASAN: null-ptr-deref in range [0x00000000000000a8-0x00000000000000af]
>> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.6.0-rc5-syzkaller #0
>> 
>> Call Trace
>> __usb_hcd_giveback_urb+0x29a/0x550 drivers/usb/core/hcd.c:1650
>> usb_hcd_giveback_urb+0x368/0x420 drivers/usb/core/hcd.c:1716
>> dummy_timer+0x1258/0x32ae drivers/usb/gadget/udc/dummy_hcd.c:1966
>> call_timer_fn+0x195/0x6f0 kernel/time/timer.c:1404
>> expire_timers kernel/time/timer.c:1449 [inline]
>> __run_timers kernel/time/timer.c:1773 [inline]
>> __run_timers kernel/time/timer.c:1740 [inline]
>> run_timer_softirq+0x5f9/0x1500 kernel/time/timer.c:1786
>> __do_softirq+0x21e/0x950 kernel/softirq.c:292
>> invoke_softirq kernel/softirq.c:373 [inline]
>> irq_exit+0x178/0x1a0 kernel/softirq.c:413
>> exiting_irq arch/x86/include/asm/apic.h:546 [inline]
>> smp_apic_timer_interrupt+0x141/0x540 arch/x86/kernel/apic/apic.c:1146
>> apic_timer_interrupt+0xf/0x20 arch/x86/entry/entry_64.S:829
>> 
>> Reported-and-tested-by: syzbot+40d5d2e8a4680952f042@syzkaller.appspotmail.com
>> Signed-off-by: Qiujun Huang <hqjagain@gmail.com>
>
> This causes complete breakage of ath9k operation across all the stable kernel
> series it got backported to, and I guess the mainline as well. Please see:
> https://bugzilla.kernel.org/show_bug.cgi?id=208251
> https://bugzilla.redhat.com/show_bug.cgi?id=1848631

So there's no fix for this? I was under impression that someone fixed
this, but maybe I'm mixing with something else.

If this is not fixed can someone please submit a patch to revert the
offending commit (or commits) so that we get ath9k working again?

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

[PATCH] Revert "ath9k: Fix general protection fault in ath9k_hif_usb_rx_cb"

[Viktor Jägersküpper]

Kalle Valo writes:
> Roman Mamedov <rm@romanrm.net> writes:
> 
>> On Sat,  4 Apr 2020 12:18:38 +0800
>> Qiujun Huang <hqjagain@gmail.com> wrote:
>>
>>> In ath9k_hif_usb_rx_cb interface number is assumed to be 0.
>>> usb_ifnum_to_if(urb->dev, 0)
>>> But it isn't always true.
>>>
>>> The case reported by syzbot:
>>> https://lore.kernel.org/linux-usb/000000000000666c9c05a1c05d12@google.com
>>> usb 2-1: new high-speed USB device number 2 using dummy_hcd
>>> usb 2-1: config 1 has an invalid interface number: 2 but max is 0
>>> usb 2-1: config 1 has no interface number 0
>>> usb 2-1: New USB device found, idVendor=0cf3, idProduct=9271, bcdDevice=
>>> 1.08
>>> usb 2-1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
>>> general protection fault, probably for non-canonical address
>>> 0xdffffc0000000015: 0000 [#1] SMP KASAN
>>> KASAN: null-ptr-deref in range [0x00000000000000a8-0x00000000000000af]
>>> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.6.0-rc5-syzkaller #0
>>>
>>> Call Trace
>>> __usb_hcd_giveback_urb+0x29a/0x550 drivers/usb/core/hcd.c:1650
>>> usb_hcd_giveback_urb+0x368/0x420 drivers/usb/core/hcd.c:1716
>>> dummy_timer+0x1258/0x32ae drivers/usb/gadget/udc/dummy_hcd.c:1966
>>> call_timer_fn+0x195/0x6f0 kernel/time/timer.c:1404
>>> expire_timers kernel/time/timer.c:1449 [inline]
>>> __run_timers kernel/time/timer.c:1773 [inline]
>>> __run_timers kernel/time/timer.c:1740 [inline]
>>> run_timer_softirq+0x5f9/0x1500 kernel/time/timer.c:1786
>>> __do_softirq+0x21e/0x950 kernel/softirq.c:292
>>> invoke_softirq kernel/softirq.c:373 [inline]
>>> irq_exit+0x178/0x1a0 kernel/softirq.c:413
>>> exiting_irq arch/x86/include/asm/apic.h:546 [inline]
>>> smp_apic_timer_interrupt+0x141/0x540 arch/x86/kernel/apic/apic.c:1146
>>> apic_timer_interrupt+0xf/0x20 arch/x86/entry/entry_64.S:829
>>>
>>> Reported-and-tested-by: syzbot+40d5d2e8a4680952f042@syzkaller.appspotmail.com
>>> Signed-off-by: Qiujun Huang <hqjagain@gmail.com>
>>
>> This causes complete breakage of ath9k operation across all the stable kernel
>> series it got backported to, and I guess the mainline as well. Please see:
>> https://bugzilla.kernel.org/show_bug.cgi?id=208251
>> https://bugzilla.redhat.com/show_bug.cgi?id=1848631
> 
> So there's no fix for this? I was under impression that someone fixed
> this, but maybe I'm mixing with something else.
> 
> If this is not fixed can someone please submit a patch to revert the
> offending commit (or commits) so that we get ath9k working again?
> 

This reverts commit 2bbcaaee1fcbd83272e29f31e2bb7e70d8c49e05 ("ath9k: Fix general protection fault
in ath9k_hif_usb_rx_cb") because the driver gets stuck like this:

  [    5.778803] usb 1-5: Manufacturer: ATHEROS
  [   21.697488] usb 1-5: ath9k_htc: Firmware ath9k_htc/htc_9271-1.4.0.fw requested
  [   21.701377] usbcore: registered new interface driver ath9k_htc
  [   22.053705] usb 1-5: ath9k_htc: Transferred FW: ath9k_htc/htc_9271-1.4.0.fw, size: 51008
  [   22.306182] ath9k_htc 1-5:1.0: ath9k_htc: HTC initialized with 33 credits
  [  115.708513] ath9k_htc: Failed to initialize the device
  [  115.708683] usb 1-5: ath9k_htc: USB layer deinitialized

Reported-by: Roman Mamedov <rm@romanrm.net>
Ref: https://bugzilla.kernel.org/show_bug.cgi?id=208251
Fixes: 2bbcaaee1fcb ("ath9k: Fix general protection fault in ath9k_hif_usb_rx_cb")
Tested-by: Viktor Jägersküpper <viktor_jaegerskuepper@freenet.de>
Signed-off-by: Viktor Jägersküpper <viktor_jaegerskuepper@freenet.de>
---

I couldn't find any fix for this, so here is the patch which reverts the
offending commit. I have tested it with 5.8.0-rc3 and with 5.7.4.

Feel free to change the commit message if it is necessary or appropriate, I am
just a user affected by this bug.

diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.c b/drivers/net/wireless/ath/ath9k/hif_usb.c
index 4ed21dad6a8e..6049d3766c64 100644
--- a/drivers/net/wireless/ath/ath9k/hif_usb.c
+++ b/drivers/net/wireless/ath/ath9k/hif_usb.c
@@ -643,9 +643,9 @@ static void ath9k_hif_usb_rx_stream(struct hif_device_usb *hif_dev,
 
 static void ath9k_hif_usb_rx_cb(struct urb *urb)
 {
-       struct rx_buf *rx_buf = (struct rx_buf *)urb->context;
-       struct hif_device_usb *hif_dev = rx_buf->hif_dev;
-       struct sk_buff *skb = rx_buf->skb;
+       struct sk_buff *skb = (struct sk_buff *) urb->context;
+       struct hif_device_usb *hif_dev =
+               usb_get_intfdata(usb_ifnum_to_if(urb->dev, 0));
        int ret;
 
        if (!skb)
@@ -685,15 +685,14 @@ static void ath9k_hif_usb_rx_cb(struct urb *urb)
        return;
 free:
        kfree_skb(skb);
-       kfree(rx_buf);
 }
 
 static void ath9k_hif_usb_reg_in_cb(struct urb *urb)
 {
-       struct rx_buf *rx_buf = (struct rx_buf *)urb->context;
-       struct hif_device_usb *hif_dev = rx_buf->hif_dev;
-       struct sk_buff *skb = rx_buf->skb;
+       struct sk_buff *skb = (struct sk_buff *) urb->context;
        struct sk_buff *nskb;
+       struct hif_device_usb *hif_dev =
+               usb_get_intfdata(usb_ifnum_to_if(urb->dev, 0));
        int ret;
 
        if (!skb)
@@ -751,7 +750,6 @@ static void ath9k_hif_usb_reg_in_cb(struct urb *urb)
        return;
 free:
        kfree_skb(skb);
-       kfree(rx_buf);
        urb->context = NULL;
 }
 
@@ -797,7 +795,7 @@ static int ath9k_hif_usb_alloc_tx_urbs(struct hif_device_usb *hif_dev)
        init_usb_anchor(&hif_dev->mgmt_submitted);
 
        for (i = 0; i < MAX_TX_URB_NUM; i++) {
-               tx_buf = kzalloc(sizeof(*tx_buf), GFP_KERNEL);
+               tx_buf = kzalloc(sizeof(struct tx_buf), GFP_KERNEL);
                if (!tx_buf)
                        goto err;
 
@@ -834,9 +832,8 @@ static void ath9k_hif_usb_dealloc_rx_urbs(struct hif_device_usb *hif_dev)
 
 static int ath9k_hif_usb_alloc_rx_urbs(struct hif_device_usb *hif_dev)
 {
-       struct rx_buf *rx_buf = NULL;
-       struct sk_buff *skb = NULL;
        struct urb *urb = NULL;
+       struct sk_buff *skb = NULL;
        int i, ret;
 
        init_usb_anchor(&hif_dev->rx_submitted);
@@ -844,12 +841,6 @@ static int ath9k_hif_usb_alloc_rx_urbs(struct hif_device_usb *hif_dev)
 
        for (i = 0; i < MAX_RX_URB_NUM; i++) {
 
-               rx_buf = kzalloc(sizeof(*rx_buf), GFP_KERNEL);
-               if (!rx_buf) {
-                       ret = -ENOMEM;
-                       goto err_rxb;
-               }
-
                /* Allocate URB */
                urb = usb_alloc_urb(0, GFP_KERNEL);
                if (urb == NULL) {
@@ -864,14 +855,11 @@ static int ath9k_hif_usb_alloc_rx_urbs(struct hif_device_usb *hif_dev)
                        goto err_skb;
                }
 
-               rx_buf->hif_dev = hif_dev;
-               rx_buf->skb = skb;
-
                usb_fill_bulk_urb(urb, hif_dev->udev,
                                  usb_rcvbulkpipe(hif_dev->udev,
                                                  USB_WLAN_RX_PIPE),
                                  skb->data, MAX_RX_BUF_SIZE,
-                                 ath9k_hif_usb_rx_cb, rx_buf);
+                                 ath9k_hif_usb_rx_cb, skb);
 
                /* Anchor URB */
                usb_anchor_urb(urb, &hif_dev->rx_submitted);
@@ -897,8 +885,6 @@ static int ath9k_hif_usb_alloc_rx_urbs(struct hif_device_usb *hif_dev)
 err_skb:
        usb_free_urb(urb);
 err_urb:
-       kfree(rx_buf);
-err_rxb:
        ath9k_hif_usb_dealloc_rx_urbs(hif_dev);
        return ret;
 }
@@ -910,21 +896,14 @@ static void ath9k_hif_usb_dealloc_reg_in_urbs(struct hif_device_usb *hif_dev)
 
 static int ath9k_hif_usb_alloc_reg_in_urbs(struct hif_device_usb *hif_dev)
 {
-       struct rx_buf *rx_buf = NULL;
-       struct sk_buff *skb = NULL;
        struct urb *urb = NULL;
+       struct sk_buff *skb = NULL;
        int i, ret;
 
        init_usb_anchor(&hif_dev->reg_in_submitted);
 
        for (i = 0; i < MAX_REG_IN_URB_NUM; i++) {
 
-               rx_buf = kzalloc(sizeof(*rx_buf), GFP_KERNEL);
-               if (!rx_buf) {
-                       ret = -ENOMEM;
-                       goto err_rxb;
-               }
-
                /* Allocate URB */
                urb = usb_alloc_urb(0, GFP_KERNEL);
                if (urb == NULL) {
@@ -939,14 +918,11 @@ static int ath9k_hif_usb_alloc_reg_in_urbs(struct hif_device_usb *hif_dev)
                        goto err_skb;
                }
 
-               rx_buf->hif_dev = hif_dev;
-               rx_buf->skb = skb;
-
                usb_fill_int_urb(urb, hif_dev->udev,
                                  usb_rcvintpipe(hif_dev->udev,
                                                  USB_REG_IN_PIPE),
                                  skb->data, MAX_REG_IN_BUF_SIZE,
-                                 ath9k_hif_usb_reg_in_cb, rx_buf, 1);
+                                 ath9k_hif_usb_reg_in_cb, skb, 1);
 
                /* Anchor URB */
                usb_anchor_urb(urb, &hif_dev->reg_in_submitted);
@@ -972,8 +948,6 @@ static int ath9k_hif_usb_alloc_reg_in_urbs(struct hif_device_usb *hif_dev)
 err_skb:
        usb_free_urb(urb);
 err_urb:
-       kfree(rx_buf);
-err_rxb:
        ath9k_hif_usb_dealloc_reg_in_urbs(hif_dev);
        return ret;
 }
diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.h b/drivers/net/wireless/ath/ath9k/hif_usb.h
index 5985aa15ca93..a94e7e1c86e9 100644
--- a/drivers/net/wireless/ath/ath9k/hif_usb.h
+++ b/drivers/net/wireless/ath/ath9k/hif_usb.h
@@ -86,11 +86,6 @@ struct tx_buf {
        struct list_head list;
 };
 
-struct rx_buf {
-       struct sk_buff *skb;
-       struct hif_device_usb *hif_dev;
-};
-
 #define HIF_USB_TX_STOP  BIT(0)
 #define HIF_USB_TX_FLUSH BIT(1)
 

Re: [PATCH] Revert "ath9k: Fix general protection fault in ath9k_hif_usb_rx_cb"

[Roman Mamedov]

On Wed, 1 Jul 2020 17:53:27 +0200
Viktor Jägersküpper <viktor_jaegerskuepper@freenet.de> wrote:

> Kalle Valo writes:
> > Roman Mamedov <rm@romanrm.net> writes:
> > 
> >> On Sat,  4 Apr 2020 12:18:38 +0800
> >> Qiujun Huang <hqjagain@gmail.com> wrote:
> >>
> >>> In ath9k_hif_usb_rx_cb interface number is assumed to be 0.
> >>> usb_ifnum_to_if(urb->dev, 0)
> >>> But it isn't always true.
> >>>
> >>> The case reported by syzbot:
> >>> https://lore.kernel.org/linux-usb/000000000000666c9c05a1c05d12@google.com
> >>> usb 2-1: new high-speed USB device number 2 using dummy_hcd
> >>> usb 2-1: config 1 has an invalid interface number: 2 but max is 0
> >>> usb 2-1: config 1 has no interface number 0
> >>> usb 2-1: New USB device found, idVendor=0cf3, idProduct=9271, bcdDevice=
> >>> 1.08
> >>> usb 2-1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
> >>> general protection fault, probably for non-canonical address
> >>> 0xdffffc0000000015: 0000 [#1] SMP KASAN
> >>> KASAN: null-ptr-deref in range [0x00000000000000a8-0x00000000000000af]
> >>> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.6.0-rc5-syzkaller #0
> >>>
> >>> Call Trace
> >>> __usb_hcd_giveback_urb+0x29a/0x550 drivers/usb/core/hcd.c:1650
> >>> usb_hcd_giveback_urb+0x368/0x420 drivers/usb/core/hcd.c:1716
> >>> dummy_timer+0x1258/0x32ae drivers/usb/gadget/udc/dummy_hcd.c:1966
> >>> call_timer_fn+0x195/0x6f0 kernel/time/timer.c:1404
> >>> expire_timers kernel/time/timer.c:1449 [inline]
> >>> __run_timers kernel/time/timer.c:1773 [inline]
> >>> __run_timers kernel/time/timer.c:1740 [inline]
> >>> run_timer_softirq+0x5f9/0x1500 kernel/time/timer.c:1786
> >>> __do_softirq+0x21e/0x950 kernel/softirq.c:292
> >>> invoke_softirq kernel/softirq.c:373 [inline]
> >>> irq_exit+0x178/0x1a0 kernel/softirq.c:413
> >>> exiting_irq arch/x86/include/asm/apic.h:546 [inline]
> >>> smp_apic_timer_interrupt+0x141/0x540 arch/x86/kernel/apic/apic.c:1146
> >>> apic_timer_interrupt+0xf/0x20 arch/x86/entry/entry_64.S:829
> >>>
> >>> Reported-and-tested-by: syzbot+40d5d2e8a4680952f042@syzkaller.appspotmail.com
> >>> Signed-off-by: Qiujun Huang <hqjagain@gmail.com>
> >>
> >> This causes complete breakage of ath9k operation across all the stable kernel
> >> series it got backported to, and I guess the mainline as well. Please see:
> >> https://bugzilla.kernel.org/show_bug.cgi?id=208251
> >> https://bugzilla.redhat.com/show_bug.cgi?id=1848631
> > 
> > So there's no fix for this? I was under impression that someone fixed
> > this, but maybe I'm mixing with something else.
> > 
> > If this is not fixed can someone please submit a patch to revert the
> > offending commit (or commits) so that we get ath9k working again?
> > 
> 
> This reverts commit 2bbcaaee1fcbd83272e29f31e2bb7e70d8c49e05 ("ath9k: Fix general protection fault
> in ath9k_hif_usb_rx_cb") because the driver gets stuck like this:
> 
>   [    5.778803] usb 1-5: Manufacturer: ATHEROS
>   [   21.697488] usb 1-5: ath9k_htc: Firmware ath9k_htc/htc_9271-1.4.0.fw requested
>   [   21.701377] usbcore: registered new interface driver ath9k_htc
>   [   22.053705] usb 1-5: ath9k_htc: Transferred FW: ath9k_htc/htc_9271-1.4.0.fw, size: 51008
>   [   22.306182] ath9k_htc 1-5:1.0: ath9k_htc: HTC initialized with 33 credits
>   [  115.708513] ath9k_htc: Failed to initialize the device
>   [  115.708683] usb 1-5: ath9k_htc: USB layer deinitialized
> 
> Reported-by: Roman Mamedov <rm@romanrm.net>
> Ref: https://bugzilla.kernel.org/show_bug.cgi?id=208251
> Fixes: 2bbcaaee1fcb ("ath9k: Fix general protection fault in ath9k_hif_usb_rx_cb")
> Tested-by: Viktor Jägersküpper <viktor_jaegerskuepper@freenet.de>
> Signed-off-by: Viktor Jägersküpper <viktor_jaegerskuepper@freenet.de>
> ---
> 
> I couldn't find any fix for this, so here is the patch which reverts the
> offending commit. I have tested it with 5.8.0-rc3 and with 5.7.4.
> 
> Feel free to change the commit message if it is necessary or appropriate, I am
> just a user affected by this bug.
> 
> diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.c b/drivers/net/wireless/ath/ath9k/hif_usb.c
> index 4ed21dad6a8e..6049d3766c64 100644
> --- a/drivers/net/wireless/ath/ath9k/hif_usb.c
> +++ b/drivers/net/wireless/ath/ath9k/hif_usb.c
> @@ -643,9 +643,9 @@ static void ath9k_hif_usb_rx_stream(struct hif_device_usb *hif_dev,
>  
>  static void ath9k_hif_usb_rx_cb(struct urb *urb)
>  {
> -       struct rx_buf *rx_buf = (struct rx_buf *)urb->context;
> -       struct hif_device_usb *hif_dev = rx_buf->hif_dev;
> -       struct sk_buff *skb = rx_buf->skb;
> +       struct sk_buff *skb = (struct sk_buff *) urb->context;
> +       struct hif_device_usb *hif_dev =
> +               usb_get_intfdata(usb_ifnum_to_if(urb->dev, 0));

It seems like line indentations are wrong in this one, you always use spaces
to indent, and not what the file or the patch being reverted originally used.

It does not apply for me, compared to reverting the offending commit outright:

$ patch --dry-run -p1 -R < bad-commit.patch 
checking file drivers/net/wireless/ath/ath9k/hif_usb.c
checking file drivers/net/wireless/ath/ath9k/hif_usb.h

$ patch --dry-run -p1 < viktor.patch 
checking file drivers/net/wireless/ath/ath9k/hif_usb.c
Hunk #1 FAILED at 643.
Hunk #2 FAILED at 685.
Hunk #3 FAILED at 751.
Hunk #4 FAILED at 797.
Hunk #5 FAILED at 834.
Hunk #6 FAILED at 844.
Hunk #7 FAILED at 864.
Hunk #8 FAILED at 897.
Hunk #9 FAILED at 910.
Hunk #10 FAILED at 939.
Hunk #11 FAILED at 972.
11 out of 11 hunks FAILED
checking file drivers/net/wireless/ath/ath9k/hif_usb.h
Reversed (or previously applied) patch detected!  Assume -R? [n] ^C

If you will submit a fixed one, perhaps it should be a "v2" now, and doesn't
need to quote all the prior conversation (patches don't generally do that) or
add extra comments after "Signed-off-by" (already done). But I'm also mostly
just a user and not an expert on these matters. :)

-- 
With respect,
Roman

[PATCH v2] Revert "ath9k: Fix general protection fault in ath9k_hif_usb_rx_cb"

[Viktor Jägersküpper]

This reverts commit 2bbcaaee1fcb ("ath9k: Fix general protection fault
in ath9k_hif_usb_rx_cb") because the driver gets stuck like this:

  [    5.778803] usb 1-5: Manufacturer: ATHEROS
  [   21.697488] usb 1-5: ath9k_htc: Firmware ath9k_htc/htc_9271-1.4.0.fw requested
  [   21.701377] usbcore: registered new interface driver ath9k_htc
  [   22.053705] usb 1-5: ath9k_htc: Transferred FW: ath9k_htc/htc_9271-1.4.0.fw, size: 51008
  [   22.306182] ath9k_htc 1-5:1.0: ath9k_htc: HTC initialized with 33 credits
  [  115.708513] ath9k_htc: Failed to initialize the device
  [  115.708683] usb 1-5: ath9k_htc: USB layer deinitialized

Reported-by: Roman Mamedov <rm@romanrm.net>
Ref: https://bugzilla.kernel.org/show_bug.cgi?id=208251
Fixes: 2bbcaaee1fcb ("ath9k: Fix general protection fault in ath9k_hif_usb_rx_cb")
Tested-by: Viktor Jägersküpper <viktor_jaegerskuepper@freenet.de>
Signed-off-by: Viktor Jägersküpper <viktor_jaegerskuepper@freenet.de>
---
Changes:
  - Use correct line indentations (Thanks, Roman!)
  - Use shorter commit reference in commit log
---
 drivers/net/wireless/ath/ath9k/hif_usb.c | 48 ++++++------------------
 drivers/net/wireless/ath/ath9k/hif_usb.h |  5 ---
 2 files changed, 11 insertions(+), 42 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.c b/drivers/net/wireless/ath/ath9k/hif_usb.c
index 4ed21dad6a8e..6049d3766c64 100644
--- a/drivers/net/wireless/ath/ath9k/hif_usb.c
+++ b/drivers/net/wireless/ath/ath9k/hif_usb.c
@@ -643,9 +643,9 @@ static void ath9k_hif_usb_rx_stream(struct hif_device_usb *hif_dev,
 
 static void ath9k_hif_usb_rx_cb(struct urb *urb)
 {
-	struct rx_buf *rx_buf = (struct rx_buf *)urb->context;
-	struct hif_device_usb *hif_dev = rx_buf->hif_dev;
-	struct sk_buff *skb = rx_buf->skb;
+	struct sk_buff *skb = (struct sk_buff *) urb->context;
+	struct hif_device_usb *hif_dev =
+		usb_get_intfdata(usb_ifnum_to_if(urb->dev, 0));
 	int ret;
 
 	if (!skb)
@@ -685,15 +685,14 @@ static void ath9k_hif_usb_rx_cb(struct urb *urb)
 	return;
 free:
 	kfree_skb(skb);
-	kfree(rx_buf);
 }
 
 static void ath9k_hif_usb_reg_in_cb(struct urb *urb)
 {
-	struct rx_buf *rx_buf = (struct rx_buf *)urb->context;
-	struct hif_device_usb *hif_dev = rx_buf->hif_dev;
-	struct sk_buff *skb = rx_buf->skb;
+	struct sk_buff *skb = (struct sk_buff *) urb->context;
 	struct sk_buff *nskb;
+	struct hif_device_usb *hif_dev =
+		usb_get_intfdata(usb_ifnum_to_if(urb->dev, 0));
 	int ret;
 
 	if (!skb)
@@ -751,7 +750,6 @@ static void ath9k_hif_usb_reg_in_cb(struct urb *urb)
 	return;
 free:
 	kfree_skb(skb);
-	kfree(rx_buf);
 	urb->context = NULL;
 }
 
@@ -797,7 +795,7 @@ static int ath9k_hif_usb_alloc_tx_urbs(struct hif_device_usb *hif_dev)
 	init_usb_anchor(&hif_dev->mgmt_submitted);
 
 	for (i = 0; i < MAX_TX_URB_NUM; i++) {
-		tx_buf = kzalloc(sizeof(*tx_buf), GFP_KERNEL);
+		tx_buf = kzalloc(sizeof(struct tx_buf), GFP_KERNEL);
 		if (!tx_buf)
 			goto err;
 
@@ -834,9 +832,8 @@ static void ath9k_hif_usb_dealloc_rx_urbs(struct hif_device_usb *hif_dev)
 
 static int ath9k_hif_usb_alloc_rx_urbs(struct hif_device_usb *hif_dev)
 {
-	struct rx_buf *rx_buf = NULL;
-	struct sk_buff *skb = NULL;
 	struct urb *urb = NULL;
+	struct sk_buff *skb = NULL;
 	int i, ret;
 
 	init_usb_anchor(&hif_dev->rx_submitted);
@@ -844,12 +841,6 @@ static int ath9k_hif_usb_alloc_rx_urbs(struct hif_device_usb *hif_dev)
 
 	for (i = 0; i < MAX_RX_URB_NUM; i++) {
 
-		rx_buf = kzalloc(sizeof(*rx_buf), GFP_KERNEL);
-		if (!rx_buf) {
-			ret = -ENOMEM;
-			goto err_rxb;
-		}
-
 		/* Allocate URB */
 		urb = usb_alloc_urb(0, GFP_KERNEL);
 		if (urb == NULL) {
@@ -864,14 +855,11 @@ static int ath9k_hif_usb_alloc_rx_urbs(struct hif_device_usb *hif_dev)
 			goto err_skb;
 		}
 
-		rx_buf->hif_dev = hif_dev;
-		rx_buf->skb = skb;
-
 		usb_fill_bulk_urb(urb, hif_dev->udev,
 				  usb_rcvbulkpipe(hif_dev->udev,
 						  USB_WLAN_RX_PIPE),
 				  skb->data, MAX_RX_BUF_SIZE,
-				  ath9k_hif_usb_rx_cb, rx_buf);
+				  ath9k_hif_usb_rx_cb, skb);
 
 		/* Anchor URB */
 		usb_anchor_urb(urb, &hif_dev->rx_submitted);
@@ -897,8 +885,6 @@ static int ath9k_hif_usb_alloc_rx_urbs(struct hif_device_usb *hif_dev)
 err_skb:
 	usb_free_urb(urb);
 err_urb:
-	kfree(rx_buf);
-err_rxb:
 	ath9k_hif_usb_dealloc_rx_urbs(hif_dev);
 	return ret;
 }
@@ -910,21 +896,14 @@ static void ath9k_hif_usb_dealloc_reg_in_urbs(struct hif_device_usb *hif_dev)
 
 static int ath9k_hif_usb_alloc_reg_in_urbs(struct hif_device_usb *hif_dev)
 {
-	struct rx_buf *rx_buf = NULL;
-	struct sk_buff *skb = NULL;
 	struct urb *urb = NULL;
+	struct sk_buff *skb = NULL;
 	int i, ret;
 
 	init_usb_anchor(&hif_dev->reg_in_submitted);
 
 	for (i = 0; i < MAX_REG_IN_URB_NUM; i++) {
 
-		rx_buf = kzalloc(sizeof(*rx_buf), GFP_KERNEL);
-		if (!rx_buf) {
-			ret = -ENOMEM;
-			goto err_rxb;
-		}
-
 		/* Allocate URB */
 		urb = usb_alloc_urb(0, GFP_KERNEL);
 		if (urb == NULL) {
@@ -939,14 +918,11 @@ static int ath9k_hif_usb_alloc_reg_in_urbs(struct hif_device_usb *hif_dev)
 			goto err_skb;
 		}
 
-		rx_buf->hif_dev = hif_dev;
-		rx_buf->skb = skb;
-
 		usb_fill_int_urb(urb, hif_dev->udev,
 				  usb_rcvintpipe(hif_dev->udev,
 						  USB_REG_IN_PIPE),
 				  skb->data, MAX_REG_IN_BUF_SIZE,
-				  ath9k_hif_usb_reg_in_cb, rx_buf, 1);
+				  ath9k_hif_usb_reg_in_cb, skb, 1);
 
 		/* Anchor URB */
 		usb_anchor_urb(urb, &hif_dev->reg_in_submitted);
@@ -972,8 +948,6 @@ static int ath9k_hif_usb_alloc_reg_in_urbs(struct hif_device_usb *hif_dev)
 err_skb:
 	usb_free_urb(urb);
 err_urb:
-	kfree(rx_buf);
-err_rxb:
 	ath9k_hif_usb_dealloc_reg_in_urbs(hif_dev);
 	return ret;
 }
diff --git a/drivers/net/wireless/ath/ath9k/hif_usb.h b/drivers/net/wireless/ath/ath9k/hif_usb.h
index 5985aa15ca93..a94e7e1c86e9 100644
--- a/drivers/net/wireless/ath/ath9k/hif_usb.h
+++ b/drivers/net/wireless/ath/ath9k/hif_usb.h
@@ -86,11 +86,6 @@ struct tx_buf {
 	struct list_head list;
 };
 
-struct rx_buf {
-	struct sk_buff *skb;
-	struct hif_device_usb *hif_dev;
-};
-
 #define HIF_USB_TX_STOP  BIT(0)
 #define HIF_USB_TX_FLUSH BIT(1)
 
--
2.27.0

Re: [PATCH] Revert "ath9k: Fix general protection fault in ath9k_hif_usb_rx_cb"

[Kalle Valo]

Viktor Jägersküpper <viktor_jaegerskuepper@freenet.de> writes:

> Kalle Valo writes:
>> Roman Mamedov <rm@romanrm.net> writes:
>> 
>>> On Sat,  4 Apr 2020 12:18:38 +0800
>>> Qiujun Huang <hqjagain@gmail.com> wrote:
>>>
>>>> In ath9k_hif_usb_rx_cb interface number is assumed to be 0.
>>>> usb_ifnum_to_if(urb->dev, 0)
>>>> But it isn't always true.
>>>>
>>>> The case reported by syzbot:
>>>> https://lore.kernel.org/linux-usb/000000000000666c9c05a1c05d12@google.com
>>>> usb 2-1: new high-speed USB device number 2 using dummy_hcd
>>>> usb 2-1: config 1 has an invalid interface number: 2 but max is 0
>>>> usb 2-1: config 1 has no interface number 0
>>>> usb 2-1: New USB device found, idVendor=0cf3, idProduct=9271, bcdDevice=
>>>> 1.08
>>>> usb 2-1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
>>>> general protection fault, probably for non-canonical address
>>>> 0xdffffc0000000015: 0000 [#1] SMP KASAN
>>>> KASAN: null-ptr-deref in range [0x00000000000000a8-0x00000000000000af]
>>>> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.6.0-rc5-syzkaller #0
>>>>
>>>> Call Trace
>>>> __usb_hcd_giveback_urb+0x29a/0x550 drivers/usb/core/hcd.c:1650
>>>> usb_hcd_giveback_urb+0x368/0x420 drivers/usb/core/hcd.c:1716
>>>> dummy_timer+0x1258/0x32ae drivers/usb/gadget/udc/dummy_hcd.c:1966
>>>> call_timer_fn+0x195/0x6f0 kernel/time/timer.c:1404
>>>> expire_timers kernel/time/timer.c:1449 [inline]
>>>> __run_timers kernel/time/timer.c:1773 [inline]
>>>> __run_timers kernel/time/timer.c:1740 [inline]
>>>> run_timer_softirq+0x5f9/0x1500 kernel/time/timer.c:1786
>>>> __do_softirq+0x21e/0x950 kernel/softirq.c:292
>>>> invoke_softirq kernel/softirq.c:373 [inline]
>>>> irq_exit+0x178/0x1a0 kernel/softirq.c:413
>>>> exiting_irq arch/x86/include/asm/apic.h:546 [inline]
>>>> smp_apic_timer_interrupt+0x141/0x540 arch/x86/kernel/apic/apic.c:1146
>>>> apic_timer_interrupt+0xf/0x20 arch/x86/entry/entry_64.S:829
>>>>
>>>> Reported-and-tested-by: syzbot+40d5d2e8a4680952f042@syzkaller.appspotmail.com
>>>> Signed-off-by: Qiujun Huang <hqjagain@gmail.com>
>>>
>>> This causes complete breakage of ath9k operation across all the stable kernel
>>> series it got backported to, and I guess the mainline as well. Please see:
>>> https://bugzilla.kernel.org/show_bug.cgi?id=208251
>>> https://bugzilla.redhat.com/show_bug.cgi?id=1848631
>> 
>> So there's no fix for this? I was under impression that someone fixed
>> this, but maybe I'm mixing with something else.
>> 
>> If this is not fixed can someone please submit a patch to revert the
>> offending commit (or commits) so that we get ath9k working again?
>> 
>
> This reverts commit 2bbcaaee1fcbd83272e29f31e2bb7e70d8c49e05 ("ath9k: Fix general protection fault
> in ath9k_hif_usb_rx_cb") because the driver gets stuck like this:
>
>   [    5.778803] usb 1-5: Manufacturer: ATHEROS
>   [   21.697488] usb 1-5: ath9k_htc: Firmware ath9k_htc/htc_9271-1.4.0.fw requested
>   [   21.701377] usbcore: registered new interface driver ath9k_htc
>   [   22.053705] usb 1-5: ath9k_htc: Transferred FW: ath9k_htc/htc_9271-1.4.0.fw, size: 51008
>   [   22.306182] ath9k_htc 1-5:1.0: ath9k_htc: HTC initialized with 33 credits
>   [  115.708513] ath9k_htc: Failed to initialize the device
>   [  115.708683] usb 1-5: ath9k_htc: USB layer deinitialized
>
> Reported-by: Roman Mamedov <rm@romanrm.net>
> Ref: https://bugzilla.kernel.org/show_bug.cgi?id=208251
> Fixes: 2bbcaaee1fcb ("ath9k: Fix general protection fault in ath9k_hif_usb_rx_cb")
> Tested-by: Viktor Jägersküpper <viktor_jaegerskuepper@freenet.de>
> Signed-off-by: Viktor Jägersküpper <viktor_jaegerskuepper@freenet.de>
> ---
>
> I couldn't find any fix for this, so here is the patch which reverts the
> offending commit. I have tested it with 5.8.0-rc3 and with 5.7.4.
>
> Feel free to change the commit message if it is necessary or appropriate, I am
> just a user affected by this bug.

This was badly formatted:

https://patchwork.kernel.org/patch/11636783/

But v2 looks correct:

https://patchwork.kernel.org/patch/11637341/

Thanks, I'll take a closer look at this as soon as I can.

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

Re: [PATCH] Revert "ath9k: Fix general protection fault in ath9k_hif_usb_rx_cb"

[Viktor Jägersküpper]

Kalle Valo wrote:
> Viktor Jägersküpper <viktor_jaegerskuepper@freenet.de> writes:
> 
>> Kalle Valo writes:
>>> Roman Mamedov <rm@romanrm.net> writes:
>>>
>>>> On Sat,  4 Apr 2020 12:18:38 +0800
>>>> Qiujun Huang <hqjagain@gmail.com> wrote:
>>>>
>>>>> In ath9k_hif_usb_rx_cb interface number is assumed to be 0.
>>>>> usb_ifnum_to_if(urb->dev, 0)
>>>>> But it isn't always true.
>>>>>
>>>>> The case reported by syzbot:
>>>>> https://lore.kernel.org/linux-usb/000000000000666c9c05a1c05d12@google.com
>>>>> usb 2-1: new high-speed USB device number 2 using dummy_hcd
>>>>> usb 2-1: config 1 has an invalid interface number: 2 but max is 0
>>>>> usb 2-1: config 1 has no interface number 0
>>>>> usb 2-1: New USB device found, idVendor=0cf3, idProduct=9271, bcdDevice=
>>>>> 1.08
>>>>> usb 2-1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
>>>>> general protection fault, probably for non-canonical address
>>>>> 0xdffffc0000000015: 0000 [#1] SMP KASAN
>>>>> KASAN: null-ptr-deref in range [0x00000000000000a8-0x00000000000000af]
>>>>> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.6.0-rc5-syzkaller #0
>>>>>
>>>>> Call Trace
>>>>> __usb_hcd_giveback_urb+0x29a/0x550 drivers/usb/core/hcd.c:1650
>>>>> usb_hcd_giveback_urb+0x368/0x420 drivers/usb/core/hcd.c:1716
>>>>> dummy_timer+0x1258/0x32ae drivers/usb/gadget/udc/dummy_hcd.c:1966
>>>>> call_timer_fn+0x195/0x6f0 kernel/time/timer.c:1404
>>>>> expire_timers kernel/time/timer.c:1449 [inline]
>>>>> __run_timers kernel/time/timer.c:1773 [inline]
>>>>> __run_timers kernel/time/timer.c:1740 [inline]
>>>>> run_timer_softirq+0x5f9/0x1500 kernel/time/timer.c:1786
>>>>> __do_softirq+0x21e/0x950 kernel/softirq.c:292
>>>>> invoke_softirq kernel/softirq.c:373 [inline]
>>>>> irq_exit+0x178/0x1a0 kernel/softirq.c:413
>>>>> exiting_irq arch/x86/include/asm/apic.h:546 [inline]
>>>>> smp_apic_timer_interrupt+0x141/0x540 arch/x86/kernel/apic/apic.c:1146
>>>>> apic_timer_interrupt+0xf/0x20 arch/x86/entry/entry_64.S:829
>>>>>
>>>>> Reported-and-tested-by: syzbot+40d5d2e8a4680952f042@syzkaller.appspotmail.com
>>>>> Signed-off-by: Qiujun Huang <hqjagain@gmail.com>
>>>>
>>>> This causes complete breakage of ath9k operation across all the stable kernel
>>>> series it got backported to, and I guess the mainline as well. Please see:
>>>> https://bugzilla.kernel.org/show_bug.cgi?id=208251
>>>> https://bugzilla.redhat.com/show_bug.cgi?id=1848631
>>>
>>> So there's no fix for this? I was under impression that someone fixed
>>> this, but maybe I'm mixing with something else.
>>>
>>> If this is not fixed can someone please submit a patch to revert the
>>> offending commit (or commits) so that we get ath9k working again?
>>>
>>
>> This reverts commit 2bbcaaee1fcbd83272e29f31e2bb7e70d8c49e05 ("ath9k: Fix general protection fault
>> in ath9k_hif_usb_rx_cb") because the driver gets stuck like this:
>>
>>   [    5.778803] usb 1-5: Manufacturer: ATHEROS
>>   [   21.697488] usb 1-5: ath9k_htc: Firmware ath9k_htc/htc_9271-1.4.0.fw requested
>>   [   21.701377] usbcore: registered new interface driver ath9k_htc
>>   [   22.053705] usb 1-5: ath9k_htc: Transferred FW: ath9k_htc/htc_9271-1.4.0.fw, size: 51008
>>   [   22.306182] ath9k_htc 1-5:1.0: ath9k_htc: HTC initialized with 33 credits
>>   [  115.708513] ath9k_htc: Failed to initialize the device
>>   [  115.708683] usb 1-5: ath9k_htc: USB layer deinitialized
>>
>> Reported-by: Roman Mamedov <rm@romanrm.net>
>> Ref: https://bugzilla.kernel.org/show_bug.cgi?id=208251
>> Fixes: 2bbcaaee1fcb ("ath9k: Fix general protection fault in ath9k_hif_usb_rx_cb")
>> Tested-by: Viktor Jägersküpper <viktor_jaegerskuepper@freenet.de>
>> Signed-off-by: Viktor Jägersküpper <viktor_jaegerskuepper@freenet.de>
>> ---
>>
>> I couldn't find any fix for this, so here is the patch which reverts the
>> offending commit. I have tested it with 5.8.0-rc3 and with 5.7.4.
>>
>> Feel free to change the commit message if it is necessary or appropriate, I am
>> just a user affected by this bug.
> 
> This was badly formatted:
> 
> https://patchwork.kernel.org/patch/11636783/
> 
> But v2 looks correct:
> 
> https://patchwork.kernel.org/patch/11637341/
> 
> Thanks, I'll take a closer look at this as soon as I can.
> 

Hi Kalle,

it seems you didn't have time for this so far. If you don't have time at the
moment, is there someone else who can fix this? Reverting the commit is just the
first and easy option and fixing this properly can be done after that.

Thanks,
Viktor

Re: [PATCH] Revert "ath9k: Fix general protection fault in ath9k_hif_usb_rx_cb"

[Kalle Valo]

Viktor Jägersküpper <viktor_jaegerskuepper@freenet.de> writes:

> Kalle Valo wrote:
>> Viktor Jägersküpper <viktor_jaegerskuepper@freenet.de> writes:
>> 
>>> Kalle Valo writes:
>>>> Roman Mamedov <rm@romanrm.net> writes:
>>>>
>>>>> On Sat,  4 Apr 2020 12:18:38 +0800
>>>>> Qiujun Huang <hqjagain@gmail.com> wrote:
>>>>>
>>>>>> In ath9k_hif_usb_rx_cb interface number is assumed to be 0.
>>>>>> usb_ifnum_to_if(urb->dev, 0)
>>>>>> But it isn't always true.
>>>>>>
>>>>>> The case reported by syzbot:
>>>>>> https://lore.kernel.org/linux-usb/000000000000666c9c05a1c05d12@google.com
>>>>>> usb 2-1: new high-speed USB device number 2 using dummy_hcd
>>>>>> usb 2-1: config 1 has an invalid interface number: 2 but max is 0
>>>>>> usb 2-1: config 1 has no interface number 0
>>>>>> usb 2-1: New USB device found, idVendor=0cf3, idProduct=9271, bcdDevice=
>>>>>> 1.08
>>>>>> usb 2-1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
>>>>>> general protection fault, probably for non-canonical address
>>>>>> 0xdffffc0000000015: 0000 [#1] SMP KASAN
>>>>>> KASAN: null-ptr-deref in range [0x00000000000000a8-0x00000000000000af]
>>>>>> CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.6.0-rc5-syzkaller #0
>>>>>>
>>>>>> Call Trace
>>>>>> __usb_hcd_giveback_urb+0x29a/0x550 drivers/usb/core/hcd.c:1650
>>>>>> usb_hcd_giveback_urb+0x368/0x420 drivers/usb/core/hcd.c:1716
>>>>>> dummy_timer+0x1258/0x32ae drivers/usb/gadget/udc/dummy_hcd.c:1966
>>>>>> call_timer_fn+0x195/0x6f0 kernel/time/timer.c:1404
>>>>>> expire_timers kernel/time/timer.c:1449 [inline]
>>>>>> __run_timers kernel/time/timer.c:1773 [inline]
>>>>>> __run_timers kernel/time/timer.c:1740 [inline]
>>>>>> run_timer_softirq+0x5f9/0x1500 kernel/time/timer.c:1786
>>>>>> __do_softirq+0x21e/0x950 kernel/softirq.c:292
>>>>>> invoke_softirq kernel/softirq.c:373 [inline]
>>>>>> irq_exit+0x178/0x1a0 kernel/softirq.c:413
>>>>>> exiting_irq arch/x86/include/asm/apic.h:546 [inline]
>>>>>> smp_apic_timer_interrupt+0x141/0x540 arch/x86/kernel/apic/apic.c:1146
>>>>>> apic_timer_interrupt+0xf/0x20 arch/x86/entry/entry_64.S:829
>>>>>>
>>>>>> Reported-and-tested-by: syzbot+40d5d2e8a4680952f042@syzkaller.appspotmail.com
>>>>>> Signed-off-by: Qiujun Huang <hqjagain@gmail.com>
>>>>>
>>>>> This causes complete breakage of ath9k operation across all the stable kernel
>>>>> series it got backported to, and I guess the mainline as well. Please see:
>>>>> https://bugzilla.kernel.org/show_bug.cgi?id=208251
>>>>> https://bugzilla.redhat.com/show_bug.cgi?id=1848631
>>>>
>>>> So there's no fix for this? I was under impression that someone fixed
>>>> this, but maybe I'm mixing with something else.
>>>>
>>>> If this is not fixed can someone please submit a patch to revert the
>>>> offending commit (or commits) so that we get ath9k working again?
>>>>
>>>
>>> This reverts commit 2bbcaaee1fcbd83272e29f31e2bb7e70d8c49e05
>>> ("ath9k: Fix general protection fault
>>> in ath9k_hif_usb_rx_cb") because the driver gets stuck like this:
>>>
>>>   [    5.778803] usb 1-5: Manufacturer: ATHEROS
>>>   [   21.697488] usb 1-5: ath9k_htc: Firmware ath9k_htc/htc_9271-1.4.0.fw requested
>>>   [   21.701377] usbcore: registered new interface driver ath9k_htc
>>>   [ 22.053705] usb 1-5: ath9k_htc: Transferred FW:
>>> ath9k_htc/htc_9271-1.4.0.fw, size: 51008
>>>   [   22.306182] ath9k_htc 1-5:1.0: ath9k_htc: HTC initialized with 33 credits
>>>   [  115.708513] ath9k_htc: Failed to initialize the device
>>>   [  115.708683] usb 1-5: ath9k_htc: USB layer deinitialized
>>>
>>> Reported-by: Roman Mamedov <rm@romanrm.net>
>>> Ref: https://bugzilla.kernel.org/show_bug.cgi?id=208251
>>> Fixes: 2bbcaaee1fcb ("ath9k: Fix general protection fault in ath9k_hif_usb_rx_cb")
>>> Tested-by: Viktor Jägersküpper <viktor_jaegerskuepper@freenet.de>
>>> Signed-off-by: Viktor Jägersküpper <viktor_jaegerskuepper@freenet.de>
>>> ---
>>>
>>> I couldn't find any fix for this, so here is the patch which reverts the
>>> offending commit. I have tested it with 5.8.0-rc3 and with 5.7.4.
>>>
>>> Feel free to change the commit message if it is necessary or appropriate, I am
>>> just a user affected by this bug.
>> 
>> This was badly formatted:
>> 
>> https://patchwork.kernel.org/patch/11636783/
>> 
>> But v2 looks correct:
>> 
>> https://patchwork.kernel.org/patch/11637341/
>> 
>> Thanks, I'll take a closer look at this as soon as I can.
>> 
>
> Hi Kalle,
>
> it seems you didn't have time for this so far. If you don't have time at the
> moment, is there someone else who can fix this? Reverting the commit is just the
> first and easy option and fixing this properly can be done after that.

I was on vacation, I will get to your patch in the next few days.

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