| Message ID | 20210620194110.7520-1-Larry.Finger@lwfinger.net (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | [v2] rtw88: Fix some memory leaks | expand |
> -----Original Message----- > From: Larry Finger [mailto:larry.finger@gmail.com] On Behalf Of Larry Finger > Sent: Monday, June 21, 2021 3:41 AM > To: kvalo@codeaurora.org > Cc: linux-wireless@vger.kernel.org; Larry Finger; Stable > Subject: [PATCH v2] rtw88: Fix some memory leaks > > There are memory leaks of skb's from routines rtw_fw_c2h_cmd_rx_irqsafe() > and rtw_coex_info_response(), both arising from C2H operations. There are > no leaks from the buffers sent to mac80211. > > Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net> > Cc: Stable <stable@vger.kernel.org> > --- > v2 - add the missinf changelog. > > --- > drivers/net/wireless/realtek/rtw88/coex.c | 4 +++- > drivers/net/wireless/realtek/rtw88/fw.c | 2 ++ > 2 files changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/wireless/realtek/rtw88/coex.c b/drivers/net/wireless/realtek/rtw88/coex.c > index cedbf3825848..e81bf5070183 100644 > --- a/drivers/net/wireless/realtek/rtw88/coex.c > +++ b/drivers/net/wireless/realtek/rtw88/coex.c > @@ -591,8 +591,10 @@ void rtw_coex_info_response(struct rtw_dev *rtwdev, struct sk_buff *skb) > struct rtw_coex *coex = &rtwdev->coex; > u8 *payload = get_payload_from_coex_resp(skb); > > - if (payload[0] != COEX_RESP_ACK_BY_WL_FW) > + if (payload[0] != COEX_RESP_ACK_BY_WL_FW) { > + dev_kfree_skb_any(skb); > return; > + } > > skb_queue_tail(&coex->queue, skb); > wake_up(&coex->wait); > diff --git a/drivers/net/wireless/realtek/rtw88/fw.c b/drivers/net/wireless/realtek/rtw88/fw.c > index 797b08b2a494..43525ad8543f 100644 > --- a/drivers/net/wireless/realtek/rtw88/fw.c > +++ b/drivers/net/wireless/realtek/rtw88/fw.c > @@ -231,9 +231,11 @@ void rtw_fw_c2h_cmd_rx_irqsafe(struct rtw_dev *rtwdev, u32 pkt_offset, > switch (c2h->id) { > case C2H_BT_MP_INFO: > rtw_coex_info_response(rtwdev, skb); > + dev_kfree_skb_any(skb); The rtw_coex_info_response() puts skb into a skb_queue, so we can't free it here. Instead, we should free it after we dequeue and do thing. So, we send another patch: https://lore.kernel.org/linux-wireless/20210621103023.41928-1-pkshih@realtek.com/T/#u I hope this isn't confusing you. > break; > case C2H_WLAN_RFON: > complete(&rtwdev->lps_leave_check); > + dev_kfree_skb_any(skb); > break; > default: > /* pass offset for further operation */ > -- > 2.32.0 -- Ping-Ke
On 6/21/21 5:40 AM, Pkshih wrote: > > >> -----Original Message----- >> From: Larry Finger [mailto:larry.finger@gmail.com] On Behalf Of Larry Finger >> Sent: Monday, June 21, 2021 3:41 AM >> To: kvalo@codeaurora.org >> Cc: linux-wireless@vger.kernel.org; Larry Finger; Stable >> Subject: [PATCH v2] rtw88: Fix some memory leaks >> >> There are memory leaks of skb's from routines rtw_fw_c2h_cmd_rx_irqsafe() >> and rtw_coex_info_response(), both arising from C2H operations. There are >> no leaks from the buffers sent to mac80211. >> >> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net> >> Cc: Stable <stable@vger.kernel.org> >> --- >> v2 - add the missinf changelog. >> >> --- >> drivers/net/wireless/realtek/rtw88/coex.c | 4 +++- >> drivers/net/wireless/realtek/rtw88/fw.c | 2 ++ >> 2 files changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/net/wireless/realtek/rtw88/coex.c b/drivers/net/wireless/realtek/rtw88/coex.c >> index cedbf3825848..e81bf5070183 100644 >> --- a/drivers/net/wireless/realtek/rtw88/coex.c >> +++ b/drivers/net/wireless/realtek/rtw88/coex.c >> @@ -591,8 +591,10 @@ void rtw_coex_info_response(struct rtw_dev *rtwdev, struct sk_buff *skb) >> struct rtw_coex *coex = &rtwdev->coex; >> u8 *payload = get_payload_from_coex_resp(skb); >> >> - if (payload[0] != COEX_RESP_ACK_BY_WL_FW) >> + if (payload[0] != COEX_RESP_ACK_BY_WL_FW) { >> + dev_kfree_skb_any(skb); >> return; >> + } >> >> skb_queue_tail(&coex->queue, skb); >> wake_up(&coex->wait); >> diff --git a/drivers/net/wireless/realtek/rtw88/fw.c b/drivers/net/wireless/realtek/rtw88/fw.c >> index 797b08b2a494..43525ad8543f 100644 >> --- a/drivers/net/wireless/realtek/rtw88/fw.c >> +++ b/drivers/net/wireless/realtek/rtw88/fw.c >> @@ -231,9 +231,11 @@ void rtw_fw_c2h_cmd_rx_irqsafe(struct rtw_dev *rtwdev, u32 pkt_offset, >> switch (c2h->id) { >> case C2H_BT_MP_INFO: >> rtw_coex_info_response(rtwdev, skb); >> + dev_kfree_skb_any(skb); > > The rtw_coex_info_response() puts skb into a skb_queue, so we can't free it here. > Instead, we should free it after we dequeue and do thing. > So, we send another patch: > https://lore.kernel.org/linux-wireless/20210621103023.41928-1-pkshih@realtek.com/T/#u > > I hope this isn't confusing you. No, it is not confusing. T=You fixed some leaks the I did not know existed. Kalle: Take P-K's patch and discard mine. Thanks, Larry
Larry Finger <Larry.Finger@lwfinger.net> writes: > On 6/21/21 5:40 AM, Pkshih wrote: >> >> >>> -----Original Message----- >>> From: Larry Finger [mailto:larry.finger@gmail.com] On Behalf Of Larry Finger >>> Sent: Monday, June 21, 2021 3:41 AM >>> To: kvalo@codeaurora.org >>> Cc: linux-wireless@vger.kernel.org; Larry Finger; Stable >>> Subject: [PATCH v2] rtw88: Fix some memory leaks >>> >>> There are memory leaks of skb's from routines rtw_fw_c2h_cmd_rx_irqsafe() >>> and rtw_coex_info_response(), both arising from C2H operations. There are >>> no leaks from the buffers sent to mac80211. >>> >>> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net> >>> Cc: Stable <stable@vger.kernel.org> >>> --- >>> v2 - add the missinf changelog. >>> >>> --- >>> drivers/net/wireless/realtek/rtw88/coex.c | 4 +++- >>> drivers/net/wireless/realtek/rtw88/fw.c | 2 ++ >>> 2 files changed, 5 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/net/wireless/realtek/rtw88/coex.c b/drivers/net/wireless/realtek/rtw88/coex.c >>> index cedbf3825848..e81bf5070183 100644 >>> --- a/drivers/net/wireless/realtek/rtw88/coex.c >>> +++ b/drivers/net/wireless/realtek/rtw88/coex.c >>> @@ -591,8 +591,10 @@ void rtw_coex_info_response(struct rtw_dev *rtwdev, struct sk_buff *skb) >>> struct rtw_coex *coex = &rtwdev->coex; >>> u8 *payload = get_payload_from_coex_resp(skb); >>> >>> - if (payload[0] != COEX_RESP_ACK_BY_WL_FW) >>> + if (payload[0] != COEX_RESP_ACK_BY_WL_FW) { >>> + dev_kfree_skb_any(skb); >>> return; >>> + } >>> >>> skb_queue_tail(&coex->queue, skb); >>> wake_up(&coex->wait); >>> diff --git a/drivers/net/wireless/realtek/rtw88/fw.c b/drivers/net/wireless/realtek/rtw88/fw.c >>> index 797b08b2a494..43525ad8543f 100644 >>> --- a/drivers/net/wireless/realtek/rtw88/fw.c >>> +++ b/drivers/net/wireless/realtek/rtw88/fw.c >>> @@ -231,9 +231,11 @@ void rtw_fw_c2h_cmd_rx_irqsafe(struct rtw_dev *rtwdev, u32 pkt_offset, >>> switch (c2h->id) { >>> case C2H_BT_MP_INFO: >>> rtw_coex_info_response(rtwdev, skb); >>> + dev_kfree_skb_any(skb); >> >> The rtw_coex_info_response() puts skb into a skb_queue, so we can't free it here. >> Instead, we should free it after we dequeue and do thing. >> So, we send another patch: >> https://lore.kernel.org/linux-wireless/20210621103023.41928-1-pkshih@realtek.com/T/#u >> >> I hope this isn't confusing you. > > No, it is not confusing. T=You fixed some leaks the I did not know existed. > > Kalle: Take P-K's patch and discard mine. Will do, thank you for clear instructions :)
diff --git a/drivers/net/wireless/realtek/rtw88/coex.c b/drivers/net/wireless/realtek/rtw88/coex.c index cedbf3825848..e81bf5070183 100644 --- a/drivers/net/wireless/realtek/rtw88/coex.c +++ b/drivers/net/wireless/realtek/rtw88/coex.c @@ -591,8 +591,10 @@ void rtw_coex_info_response(struct rtw_dev *rtwdev, struct sk_buff *skb) struct rtw_coex *coex = &rtwdev->coex; u8 *payload = get_payload_from_coex_resp(skb); - if (payload[0] != COEX_RESP_ACK_BY_WL_FW) + if (payload[0] != COEX_RESP_ACK_BY_WL_FW) { + dev_kfree_skb_any(skb); return; + } skb_queue_tail(&coex->queue, skb); wake_up(&coex->wait); diff --git a/drivers/net/wireless/realtek/rtw88/fw.c b/drivers/net/wireless/realtek/rtw88/fw.c index 797b08b2a494..43525ad8543f 100644 --- a/drivers/net/wireless/realtek/rtw88/fw.c +++ b/drivers/net/wireless/realtek/rtw88/fw.c @@ -231,9 +231,11 @@ void rtw_fw_c2h_cmd_rx_irqsafe(struct rtw_dev *rtwdev, u32 pkt_offset, switch (c2h->id) { case C2H_BT_MP_INFO: rtw_coex_info_response(rtwdev, skb); + dev_kfree_skb_any(skb); break; case C2H_WLAN_RFON: complete(&rtwdev->lps_leave_check); + dev_kfree_skb_any(skb); break; default: /* pass offset for further operation */
There are memory leaks of skb's from routines rtw_fw_c2h_cmd_rx_irqsafe() and rtw_coex_info_response(), both arising from C2H operations. There are no leaks from the buffers sent to mac80211. Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net> Cc: Stable <stable@vger.kernel.org> --- v2 - add the missinf changelog. --- drivers/net/wireless/realtek/rtw88/coex.c | 4 +++- drivers/net/wireless/realtek/rtw88/fw.c | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-)