* [PATCH net] mt76: mt7615: Fix memory leak in mt7615_mcu_wtbl_sta_add()
@ 2025-11-13 6:24 Zilin Guan
2025-11-13 7:17 ` Lorenzo Bianconi
0 siblings, 1 reply; 4+ messages in thread
From: Zilin Guan @ 2025-11-13 6:24 UTC (permalink / raw)
To: nbd
Cc: lorenzo, ryder.lee, shayne.chen, sean.wang, matthias.bgg,
angelogioacchino.delregno, linux-wireless, linux-kernel,
linux-arm-kernel, linux-mediatek, jianhao.xu, Zilin Guan
In mt7615_mcu_wtbl_sta_add(), an skb sskb is allocated. If the
subsequent call to mt76_connac_mcu_alloc_wtbl_req() fails, the function
returns an error without freeing sskb, leading to a memory leak.
Fix this by calling dev_kfree_skb() on sskb in the error handling path
to ensure it is properly released.
Fixes: 99c457d902cf9 ("mt76: mt7615: move mt7615_mcu_set_bmc to mt7615_mcu_ops")
Signed-off-by: Zilin Guan <zilin@seu.edu.cn>
---
drivers/net/wireless/mediatek/mt76/mt7615/mcu.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
index 4064e193d4de..08ee2e861c4e 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c
@@ -874,8 +874,10 @@ mt7615_mcu_wtbl_sta_add(struct mt7615_phy *phy, struct ieee80211_vif *vif,
wtbl_hdr = mt76_connac_mcu_alloc_wtbl_req(&dev->mt76, &msta->wcid,
WTBL_RESET_AND_SET, NULL,
&wskb);
- if (IS_ERR(wtbl_hdr))
+ if (IS_ERR(wtbl_hdr)) {
+ dev_kfree_skb(sskb);
return PTR_ERR(wtbl_hdr);
+ }
if (enable) {
mt76_connac_mcu_wtbl_generic_tlv(&dev->mt76, wskb, vif, sta,
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH net] mt76: mt7615: Fix memory leak in mt7615_mcu_wtbl_sta_add() 2025-11-13 6:24 [PATCH net] mt76: mt7615: Fix memory leak in mt7615_mcu_wtbl_sta_add() Zilin Guan @ 2025-11-13 7:17 ` Lorenzo Bianconi 2025-11-13 9:41 ` Zilin Guan 0 siblings, 1 reply; 4+ messages in thread From: Lorenzo Bianconi @ 2025-11-13 7:17 UTC (permalink / raw) To: Zilin Guan Cc: nbd, ryder.lee, shayne.chen, sean.wang, matthias.bgg, angelogioacchino.delregno, linux-wireless, linux-kernel, linux-arm-kernel, linux-mediatek, jianhao.xu [-- Attachment #1: Type: text/plain, Size: 1475 bytes --] > In mt7615_mcu_wtbl_sta_add(), an skb sskb is allocated. If the > subsequent call to mt76_connac_mcu_alloc_wtbl_req() fails, the function > returns an error without freeing sskb, leading to a memory leak. > > Fix this by calling dev_kfree_skb() on sskb in the error handling path > to ensure it is properly released. > > Fixes: 99c457d902cf9 ("mt76: mt7615: move mt7615_mcu_set_bmc to mt7615_mcu_ops") > Signed-off-by: Zilin Guan <zilin@seu.edu.cn> > --- > drivers/net/wireless/mediatek/mt76/mt7615/mcu.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c > index 4064e193d4de..08ee2e861c4e 100644 > --- a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c > +++ b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c > @@ -874,8 +874,10 @@ mt7615_mcu_wtbl_sta_add(struct mt7615_phy *phy, struct ieee80211_vif *vif, > wtbl_hdr = mt76_connac_mcu_alloc_wtbl_req(&dev->mt76, &msta->wcid, > WTBL_RESET_AND_SET, NULL, > &wskb); > - if (IS_ERR(wtbl_hdr)) > + if (IS_ERR(wtbl_hdr)) { > + dev_kfree_skb(sskb); Hi Zilin, I can't see how this is useful since if mt76_connac_mcu_alloc_wtbl_req returns an error, wskb will not be allocated. Regards, Lorenzo > return PTR_ERR(wtbl_hdr); > + } > > if (enable) { > mt76_connac_mcu_wtbl_generic_tlv(&dev->mt76, wskb, vif, sta, > -- > 2.34.1 > [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net] mt76: mt7615: Fix memory leak in mt7615_mcu_wtbl_sta_add() 2025-11-13 7:17 ` Lorenzo Bianconi @ 2025-11-13 9:41 ` Zilin Guan 2025-11-13 10:57 ` Lorenzo Bianconi 0 siblings, 1 reply; 4+ messages in thread From: Zilin Guan @ 2025-11-13 9:41 UTC (permalink / raw) To: lorenzo Cc: angelogioacchino.delregno, jianhao.xu, linux-arm-kernel, linux-kernel, linux-mediatek, linux-wireless, matthias.bgg, nbd, ryder.lee, sean.wang, shayne.chen, zilin On Thu, Nov 13, 2025 at 08:17:09AM +0100, Lorenzo Bianconi wrote: > [-- Attachment #1: Type: text/plain, Size: 1475 bytes --] > > > In mt7615_mcu_wtbl_sta_add(), an skb sskb is allocated. If the > > subsequent call to mt76_connac_mcu_alloc_wtbl_req() fails, the function > > returns an error without freeing sskb, leading to a memory leak. > > > > Fix this by calling dev_kfree_skb() on sskb in the error handling path > > to ensure it is properly released. > > > > Fixes: 99c457d902cf9 ("mt76: mt7615: move mt7615_mcu_set_bmc to mt7615_mcu_ops") > > Signed-off-by: Zilin Guan <zilin@seu.edu.cn> > > --- > > drivers/net/wireless/mediatek/mt76/mt7615/mcu.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c > > index 4064e193d4de..08ee2e861c4e 100644 > > --- a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c > > +++ b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c > > @@ -874,8 +874,10 @@ mt7615_mcu_wtbl_sta_add(struct mt7615_phy *phy, struct ieee80211_vif *vif, > > wtbl_hdr = mt76_connac_mcu_alloc_wtbl_req(&dev->mt76, &msta->wcid, > > WTBL_RESET_AND_SET, NULL, > > &wskb); > > - if (IS_ERR(wtbl_hdr)) > > + if (IS_ERR(wtbl_hdr)) { > > + dev_kfree_skb(sskb); > > Hi Zilin, > > I can't see how this is useful since if mt76_connac_mcu_alloc_wtbl_req returns > an error, wskb will not be allocated. > > Regards, > Lorenzo Hi Lorenzo, Thanks for your review. You are correct that 'wskb' is not allocated in this error path. However, my patch is intended to free 'sskb', which was allocated earlier in the function. Without this change, 'sskb' is leaked if mt76_connac_mcu_alloc_wtbl_req() fails. This approach is similar to the error handling logic later in the function, where a failure in sending one skb results in the other one being freed. Hope this clarifies. > > return PTR_ERR(wtbl_hdr); > > + } > > > > if (enable) { > > mt76_connac_mcu_wtbl_generic_tlv(&dev->mt76, wskb, vif, sta, > > -- > > 2.34.1 > > Best Regards, Zilin Guan ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net] mt76: mt7615: Fix memory leak in mt7615_mcu_wtbl_sta_add() 2025-11-13 9:41 ` Zilin Guan @ 2025-11-13 10:57 ` Lorenzo Bianconi 0 siblings, 0 replies; 4+ messages in thread From: Lorenzo Bianconi @ 2025-11-13 10:57 UTC (permalink / raw) To: Zilin Guan Cc: angelogioacchino.delregno, jianhao.xu, linux-arm-kernel, linux-kernel, linux-mediatek, linux-wireless, matthias.bgg, nbd, ryder.lee, sean.wang, shayne.chen [-- Attachment #1: Type: text/plain, Size: 2415 bytes --] > On Thu, Nov 13, 2025 at 08:17:09AM +0100, Lorenzo Bianconi wrote: > > [-- Attachment #1: Type: text/plain, Size: 1475 bytes --] > > > > > In mt7615_mcu_wtbl_sta_add(), an skb sskb is allocated. If the > > > subsequent call to mt76_connac_mcu_alloc_wtbl_req() fails, the function > > > returns an error without freeing sskb, leading to a memory leak. > > > > > > Fix this by calling dev_kfree_skb() on sskb in the error handling path > > > to ensure it is properly released. > > > > > > Fixes: 99c457d902cf9 ("mt76: mt7615: move mt7615_mcu_set_bmc to mt7615_mcu_ops") > > > Signed-off-by: Zilin Guan <zilin@seu.edu.cn> > > > --- > > > drivers/net/wireless/mediatek/mt76/mt7615/mcu.c | 4 +++- > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c > > > index 4064e193d4de..08ee2e861c4e 100644 > > > --- a/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c > > > +++ b/drivers/net/wireless/mediatek/mt76/mt7615/mcu.c > > > @@ -874,8 +874,10 @@ mt7615_mcu_wtbl_sta_add(struct mt7615_phy *phy, struct ieee80211_vif *vif, > > > wtbl_hdr = mt76_connac_mcu_alloc_wtbl_req(&dev->mt76, &msta->wcid, > > > WTBL_RESET_AND_SET, NULL, > > > &wskb); > > > - if (IS_ERR(wtbl_hdr)) > > > + if (IS_ERR(wtbl_hdr)) { > > > + dev_kfree_skb(sskb); > > > > Hi Zilin, > > > > I can't see how this is useful since if mt76_connac_mcu_alloc_wtbl_req returns > > an error, wskb will not be allocated. > > > > Regards, > > Lorenzo > > Hi Lorenzo, > > Thanks for your review. > > You are correct that 'wskb' is not allocated in this error path. > However, my patch is intended to free 'sskb', which was allocated > earlier in the function. Without this change, 'sskb' is leaked if > mt76_connac_mcu_alloc_wtbl_req() fails. > > This approach is similar to the error handling logic later in the > function, where a failure in sending one skb results in the other one > being freed. > > Hope this clarifies. yes, right. I misread the code. I agree with the fix. Acked-by: Lorenzo Bianconi <lorenzo@kernel.org> > > > > return PTR_ERR(wtbl_hdr); > > > + } > > > > > > if (enable) { > > > mt76_connac_mcu_wtbl_generic_tlv(&dev->mt76, wskb, vif, sta, > > > -- > > > 2.34.1 > > > > > Best Regards, > Zilin Guan [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-11-13 10:58 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-11-13 6:24 [PATCH net] mt76: mt7615: Fix memory leak in mt7615_mcu_wtbl_sta_add() Zilin Guan 2025-11-13 7:17 ` Lorenzo Bianconi 2025-11-13 9:41 ` Zilin Guan 2025-11-13 10:57 ` Lorenzo Bianconi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).