linux-kernel - Re: [PATCH net] mt76: mt7615: Fix memory leak in mt7615_mcu_wtbl_sta

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-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20251113094135.348383-1-zilin@seu.edu.cn>
Date: Thu, 13 Nov 2025 09:41:35 +0000
From: Zilin Guan <zilin@....edu.cn>
To: lorenzo@...nel.org
Cc: angelogioacchino.delregno@...labora.com,
	jianhao.xu@....edu.cn,
	linux-arm-kernel@...ts.infradead.org,
	linux-kernel@...r.kernel.org,
	linux-mediatek@...ts.infradead.org,
	linux-wireless@...r.kernel.org,
	matthias.bgg@...il.com,
	nbd@....name,
	ryder.lee@...iatek.com,
	sean.wang@...iatek.com,
	shayne.chen@...iatek.com,
	zilin@....edu.cn
Subject: Re: [PATCH net] mt76: mt7615: Fix memory leak in mt7615_mcu_wtbl_sta_add()

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@....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