2023-06-16 09:03:15

by Dmitry Antipov

[permalink] [raw]
Subject: [PATCH 1/3] [v3] wifi: rtw88: delete timer and free skb queue when unloading

Fix possible crash and memory leak on driver unload by deleting
TX purge timer and freeing C2H queue in 'rtw_core_deinit()',
shrink critical section in the latter by freeing COEX queue
out of TX report lock scope.

Signed-off-by: Dmitry Antipov <[email protected]>
---
v3: shrink critical section in rtw_core_deinit() (Ping-Ke Shih)
v2: fix title and commit message (Kalle Valo)
---
drivers/net/wireless/realtek/rtw88/main.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtw88/main.c b/drivers/net/wireless/realtek/rtw88/main.c
index 9447a3aae3b5..c190598c47c3 100644
--- a/drivers/net/wireless/realtek/rtw88/main.c
+++ b/drivers/net/wireless/realtek/rtw88/main.c
@@ -2180,10 +2180,12 @@ void rtw_core_deinit(struct rtw_dev *rtwdev)
release_firmware(wow_fw->firmware);

destroy_workqueue(rtwdev->tx_wq);
+ timer_delete_sync(&rtwdev->tx_report.purge_timer);
spin_lock_irqsave(&rtwdev->tx_report.q_lock, flags);
skb_queue_purge(&rtwdev->tx_report.queue);
- skb_queue_purge(&rtwdev->coex.queue);
spin_unlock_irqrestore(&rtwdev->tx_report.q_lock, flags);
+ skb_queue_purge(&rtwdev->coex.queue);
+ skb_queue_purge(&rtwdev->c2h_queue);

list_for_each_entry_safe(rsvd_pkt, tmp, &rtwdev->rsvd_page_list,
build_list) {
--
2.40.1



2023-06-16 09:03:20

by Dmitry Antipov

[permalink] [raw]
Subject: [PATCH 3/3] [v3] wifi: rtw88: remove unused USB bulkout size set

Drop no longer used 'bulkout_size' of 'struct rtw_usb'
and related macros from usb.h, but preserve sanity check
in 'rtw_usb_parse()'. This follows commit 462c8db6a011
("wifi: rtw88: usb: drop now unnecessary URB size check").

Signed-off-by: Dmitry Antipov <[email protected]>
---
drivers/net/wireless/realtek/rtw88/usb.c | 5 -----
drivers/net/wireless/realtek/rtw88/usb.h | 5 -----
2 files changed, 10 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/usb.c b/drivers/net/wireless/realtek/rtw88/usb.c
index 6862338b1d51..40e614f58349 100644
--- a/drivers/net/wireless/realtek/rtw88/usb.c
+++ b/drivers/net/wireless/realtek/rtw88/usb.c
@@ -186,13 +186,8 @@ static int rtw_usb_parse(struct rtw_dev *rtwdev,
switch (usbd->speed) {
case USB_SPEED_LOW:
case USB_SPEED_FULL:
- rtwusb->bulkout_size = RTW_USB_FULL_SPEED_BULK_SIZE;
- break;
case USB_SPEED_HIGH:
- rtwusb->bulkout_size = RTW_USB_HIGH_SPEED_BULK_SIZE;
- break;
case USB_SPEED_SUPER:
- rtwusb->bulkout_size = RTW_USB_SUPER_SPEED_BULK_SIZE;
break;
default:
rtw_err(rtwdev, "failed to detect usb speed\n");
diff --git a/drivers/net/wireless/realtek/rtw88/usb.h b/drivers/net/wireless/realtek/rtw88/usb.h
index fad998005ec8..86697a5c0103 100644
--- a/drivers/net/wireless/realtek/rtw88/usb.h
+++ b/drivers/net/wireless/realtek/rtw88/usb.h
@@ -18,10 +18,6 @@

#define RTW_USB_VENQT_CMD_IDX 0x00

-#define RTW_USB_SUPER_SPEED_BULK_SIZE 1024
-#define RTW_USB_HIGH_SPEED_BULK_SIZE 512
-#define RTW_USB_FULL_SPEED_BULK_SIZE 64
-
#define RTW_USB_TX_SEL_HQ BIT(0)
#define RTW_USB_TX_SEL_LQ BIT(1)
#define RTW_USB_TX_SEL_NQ BIT(2)
@@ -73,7 +69,6 @@ struct rtw_usb {
__le32 *usb_data;
unsigned int usb_data_index;

- u32 bulkout_size;
u8 pipe_interrupt;
u8 pipe_in;
u8 out_ep[RTW_USB_EP_MAX];
--
2.40.1


2023-06-16 09:03:34

by Dmitry Antipov

[permalink] [raw]
Subject: [PATCH 2/3] [v3] wifi: rtw88: remove unused and set but unused leftovers

Drop unused and set but unused 'last_push' of 'struct rtw_txq',
'wireless_set' of 'struct rtw_sta_info', 'usb_txagg_num' of
'struct rtw_usb' and 'n' of 'struct rx_usb_ctrl_block', adjust
related code.

Signed-off-by: Dmitry Antipov <[email protected]>
---
drivers/net/wireless/realtek/rtw88/main.c | 1 -
drivers/net/wireless/realtek/rtw88/main.h | 3 ---
drivers/net/wireless/realtek/rtw88/tx.c | 2 --
drivers/net/wireless/realtek/rtw88/usb.c | 1 -
drivers/net/wireless/realtek/rtw88/usb.h | 2 --
5 files changed, 9 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/main.c b/drivers/net/wireless/realtek/rtw88/main.c
index c190598c47c3..64fd527286e1 100644
--- a/drivers/net/wireless/realtek/rtw88/main.c
+++ b/drivers/net/wireless/realtek/rtw88/main.c
@@ -1300,7 +1300,6 @@ void rtw_update_sta_info(struct rtw_dev *rtwdev, struct rtw_sta_info *si,
si->stbc_en = stbc_en;
si->ldpc_en = ldpc_en;
si->rf_type = rf_type;
- si->wireless_set = wireless_set;
si->sgi_enable = is_support_sgi;
si->vht_enable = is_vht_enable;
si->ra_mask = ra_mask;
diff --git a/drivers/net/wireless/realtek/rtw88/main.h b/drivers/net/wireless/realtek/rtw88/main.h
index 9e841f6991a9..265ae9456f06 100644
--- a/drivers/net/wireless/realtek/rtw88/main.h
+++ b/drivers/net/wireless/realtek/rtw88/main.h
@@ -734,9 +734,7 @@ struct rtw_ra_report {

struct rtw_txq {
struct list_head list;
-
unsigned long flags;
- unsigned long last_push;
};

#define RTW_BC_MC_MACID 1
@@ -754,7 +752,6 @@ struct rtw_sta_info {
u8 rate_id;
enum rtw_bandwidth bw_mode;
enum rtw_rf_type rf_type;
- enum rtw_wireless_set wireless_set;
u8 stbc_en:2;
u8 ldpc_en:2;
bool sgi_enable;
diff --git a/drivers/net/wireless/realtek/rtw88/tx.c b/drivers/net/wireless/realtek/rtw88/tx.c
index bb5c7492c98b..3a4c37fdfeb1 100644
--- a/drivers/net/wireless/realtek/rtw88/tx.c
+++ b/drivers/net/wireless/realtek/rtw88/tx.c
@@ -592,8 +592,6 @@ static int rtw_txq_push_skb(struct rtw_dev *rtwdev,
rtw_err(rtwdev, "failed to write TX skb to HCI\n");
return ret;
}
- rtwtxq->last_push = jiffies;
-
return 0;
}

diff --git a/drivers/net/wireless/realtek/rtw88/usb.c b/drivers/net/wireless/realtek/rtw88/usb.c
index 976eafa739a2..6862338b1d51 100644
--- a/drivers/net/wireless/realtek/rtw88/usb.c
+++ b/drivers/net/wireless/realtek/rtw88/usb.c
@@ -647,7 +647,6 @@ static int rtw_usb_alloc_rx_bufs(struct rtw_usb *rtwusb)
for (i = 0; i < RTW_USB_RXCB_NUM; i++) {
struct rx_usb_ctrl_block *rxcb = &rtwusb->rx_cb[i];

- rxcb->n = i;
rxcb->rtwdev = rtwusb->rtwdev;
rxcb->rx_urb = usb_alloc_urb(0, GFP_KERNEL);
if (!rxcb->rx_urb)
diff --git a/drivers/net/wireless/realtek/rtw88/usb.h b/drivers/net/wireless/realtek/rtw88/usb.h
index ad1d7955c6a5..fad998005ec8 100644
--- a/drivers/net/wireless/realtek/rtw88/usb.h
+++ b/drivers/net/wireless/realtek/rtw88/usb.h
@@ -58,7 +58,6 @@ struct rx_usb_ctrl_block {
struct rtw_dev *rtwdev;
struct urb *rx_urb;
struct sk_buff *rx_skb;
- int n;
};

struct rtw_usb_tx_data {
@@ -79,7 +78,6 @@ struct rtw_usb {
u8 pipe_in;
u8 out_ep[RTW_USB_EP_MAX];
int qsel_to_ep[TX_DESC_QSEL_MAX];
- u8 usb_txagg_num;

struct workqueue_struct *txwq, *rxwq;

--
2.40.1


2023-06-20 03:59:06

by Ping-Ke Shih

[permalink] [raw]
Subject: RE: [PATCH 3/3] [v3] wifi: rtw88: remove unused USB bulkout size set

// add rtw88 USB author Sascha for his comments.

> -----Original Message-----
> From: Dmitry Antipov <[email protected]>
> Sent: Friday, June 16, 2023 4:59 PM
> To: Ping-Ke Shih <[email protected]>
> Cc: Kalle Valo <[email protected]>; [email protected]; Dmitry Antipov <[email protected]>
> Subject: [PATCH 3/3] [v3] wifi: rtw88: remove unused USB bulkout size set
>
> Drop no longer used 'bulkout_size' of 'struct rtw_usb'
> and related macros from usb.h, but preserve sanity check
> in 'rtw_usb_parse()'. This follows commit 462c8db6a011
> ("wifi: rtw88: usb: drop now unnecessary URB size check").
>
> Signed-off-by: Dmitry Antipov <[email protected]>
> ---
> drivers/net/wireless/realtek/rtw88/usb.c | 5 -----
> drivers/net/wireless/realtek/rtw88/usb.h | 5 -----
> 2 files changed, 10 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtw88/usb.c b/drivers/net/wireless/realtek/rtw88/usb.c
> index 6862338b1d51..40e614f58349 100644
> --- a/drivers/net/wireless/realtek/rtw88/usb.c
> +++ b/drivers/net/wireless/realtek/rtw88/usb.c
> @@ -186,13 +186,8 @@ static int rtw_usb_parse(struct rtw_dev *rtwdev,
> switch (usbd->speed) {
> case USB_SPEED_LOW:
> case USB_SPEED_FULL:
> - rtwusb->bulkout_size = RTW_USB_FULL_SPEED_BULK_SIZE;
> - break;
> case USB_SPEED_HIGH:
> - rtwusb->bulkout_size = RTW_USB_HIGH_SPEED_BULK_SIZE;
> - break;
> case USB_SPEED_SUPER:
> - rtwusb->bulkout_size = RTW_USB_SUPER_SPEED_BULK_SIZE;
> break;
> default:
> rtw_err(rtwdev, "failed to detect usb speed\n");

If we decide to remove rtwusb->bulkout_size, I suggest to remove whole
switch..case chunk.




2023-06-21 08:07:20

by Sascha Hauer

[permalink] [raw]
Subject: Re: [PATCH 3/3] [v3] wifi: rtw88: remove unused USB bulkout size set

On Tue, Jun 20, 2023 at 03:49:19AM +0000, Ping-Ke Shih wrote:
> // add rtw88 USB author Sascha for his comments.
>
> > -----Original Message-----
> > From: Dmitry Antipov <[email protected]>
> > Sent: Friday, June 16, 2023 4:59 PM
> > To: Ping-Ke Shih <[email protected]>
> > Cc: Kalle Valo <[email protected]>; [email protected]; Dmitry Antipov <[email protected]>
> > Subject: [PATCH 3/3] [v3] wifi: rtw88: remove unused USB bulkout size set
> >
> > Drop no longer used 'bulkout_size' of 'struct rtw_usb'
> > and related macros from usb.h, but preserve sanity check
> > in 'rtw_usb_parse()'. This follows commit 462c8db6a011
> > ("wifi: rtw88: usb: drop now unnecessary URB size check").
> >
> > Signed-off-by: Dmitry Antipov <[email protected]>
> > ---
> > drivers/net/wireless/realtek/rtw88/usb.c | 5 -----
> > drivers/net/wireless/realtek/rtw88/usb.h | 5 -----
> > 2 files changed, 10 deletions(-)
> >
> > diff --git a/drivers/net/wireless/realtek/rtw88/usb.c b/drivers/net/wireless/realtek/rtw88/usb.c
> > index 6862338b1d51..40e614f58349 100644
> > --- a/drivers/net/wireless/realtek/rtw88/usb.c
> > +++ b/drivers/net/wireless/realtek/rtw88/usb.c
> > @@ -186,13 +186,8 @@ static int rtw_usb_parse(struct rtw_dev *rtwdev,
> > switch (usbd->speed) {
> > case USB_SPEED_LOW:
> > case USB_SPEED_FULL:
> > - rtwusb->bulkout_size = RTW_USB_FULL_SPEED_BULK_SIZE;
> > - break;
> > case USB_SPEED_HIGH:
> > - rtwusb->bulkout_size = RTW_USB_HIGH_SPEED_BULK_SIZE;
> > - break;
> > case USB_SPEED_SUPER:
> > - rtwusb->bulkout_size = RTW_USB_SUPER_SPEED_BULK_SIZE;
> > break;
> > default:
> > rtw_err(rtwdev, "failed to detect usb speed\n");
>
> If we decide to remove rtwusb->bulkout_size, I suggest to remove whole
> switch..case chunk.

I didn't realize rtwusb->bulkout_size becomes unused with 462c8db6a011.
Removing this field makes sense and in that case: +1 for removing the
switch/case as well.

Sascha

--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |