linux-kernel - [PATCH] wifi: rtw88: schedule rx work after everything is set up

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20240527170137.455671-1-marcin.slusarz@gmail.com>
Date: Mon, 27 May 2024 19:01:37 +0200
From: marcin.slusarz@...il.com
To: linux-wireless@...r.kernel.org
Cc: Marcin Ślusarz <mslusarz@...au.com>,
	Ping-Ke Shih <pkshih@...ltek.com>,
	Larry Finger <Larry.Finger@...inger.net>,
	Kalle Valo <kvalo@...nel.org>,
	linux-kernel@...r.kernel.org
Subject: [PATCH] wifi: rtw88: schedule rx work after everything is set up

From: Marcin Ślusarz <mslusarz@...au.com>

Right now it's possible to hit NULL pointer dereference in
rtw_rx_fill_rx_status on hw object and/or its fields because
initialization routine can start getting USB replies before
rtw_dev is fully setup.

The stack trace looks like this:

rtw_rx_fill_rx_status
rtw8821c_query_rx_desc
rtw_usb_rx_handler
..
queue_work
rtw_usb_read_port_complete
..
usb_submit_urb
rtw_usb_rx_resubmit
rtw_usb_init_rx
rtw_usb_probe

So while we do the async stuff rtw_usb_probe continues and calls
rtw_register_hw, which does all kinds of initialization (e.g.
via ieee80211_register_hw) that rtw_rx_fill_rx_status relies on.

Fix this by moving the first usb_submit_urb after everything
is set up.

For me, this bug manifested as:
[    8.893177] rtw_8821cu 1-1:1.2: band wrong, packet dropped
[    8.910904] rtw_8821cu 1-1:1.2: hw->conf.chandef.chan NULL in rtw_rx_fill_rx_status
because I'm using Larry's backport of rtw88 driver with the NULL
checks in rtw_rx_fill_rx_status.

This should fix:
https://marc.info/?l=linux-wireless&m=167907688311943&w=2

Signed-off-by: Marcin Ślusarz <mslusarz@...au.com>
Cc: Ping-Ke Shih <pkshih@...ltek.com>
Cc: Larry Finger <Larry.Finger@...inger.net>
Cc: Kalle Valo <kvalo@...nel.org>
Cc: linux-wireless@...r.kernel.org
Cc: linux-kernel@...r.kernel.org
---
 drivers/net/wireless/realtek/rtw88/usb.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/usb.c b/drivers/net/wireless/realtek/rtw88/usb.c
index a0188511099a..98f81e3ae13e 100644
--- a/drivers/net/wireless/realtek/rtw88/usb.c
+++ b/drivers/net/wireless/realtek/rtw88/usb.c
@@ -740,7 +740,6 @@ static struct rtw_hci_ops rtw_usb_ops = {
 static int rtw_usb_init_rx(struct rtw_dev *rtwdev)
 {
 	struct rtw_usb *rtwusb = rtw_get_usb_priv(rtwdev);
-	int i;

 	rtwusb->rxwq = create_singlethread_workqueue("rtw88_usb: rx wq");
 	if (!rtwusb->rxwq) {
@@ -752,13 +751,19 @@ static int rtw_usb_init_rx(struct rtw_dev *rtwdev)

 	INIT_WORK(&rtwusb->rx_work, rtw_usb_rx_handler);

+	return 0;
+}
+
+static void rtw_usb_setup_rx(struct rtw_dev *rtwdev)
+{
+	struct rtw_usb *rtwusb = rtw_get_usb_priv(rtwdev);
+	int i;
+
 	for (i = 0; i < RTW_USB_RXCB_NUM; i++) {
 		struct rx_usb_ctrl_block *rxcb = &rtwusb->rx_cb[i];

 		rtw_usb_rx_resubmit(rtwusb, rxcb);
 	}
-
-	return 0;
 }

 static void rtw_usb_deinit_rx(struct rtw_dev *rtwdev)
@@ -895,6 +900,8 @@ int rtw_usb_probe(struct usb_interface *intf, const struct usb_device_id *id)
 		goto err_destroy_rxwq;
 	}

+	rtw_usb_setup_rx(rtwdev);
+
 	return 0;

 err_destroy_rxwq:
-- 
2.25.1