[PATCH] wifi: mt76: mt7996: Fix possible token leak in mt7996_tx_prepare_skb()

[PATCH] wifi: mt76: mt7996: Fix possible token leak in mt7996_tx_prepare_skb()

[Lorenzo Bianconi]

If link_conf or link_sta lookup fails in mt7996_tx_prepare_skb routine,
mt7996 driver leaks an already allocated tx token. Fix the issue
releasing the token in case of error.

Fixes: 7ef0c7ad735b0 ("wifi: mt76: mt7996: Implement MLD address translation for EAPOL")
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 drivers/net/wireless/mediatek/mt76/mt7996/mac.c | 8 ++++++--
 drivers/net/wireless/mediatek/mt76/tx.c         | 2 +-
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/mac.c b/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
index c98446057282..8c56344d211b 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
@@ -1067,11 +1067,11 @@ int mt7996_tx_prepare_skb(struct mt76_dev *mdev, void *txwi_ptr,
 
 		link_conf = rcu_dereference(vif->link_conf[wcid->link_id]);
 		if (!link_conf)
-			return -EINVAL;
+			goto error_relase_token;
 
 		link_sta = rcu_dereference(sta->link[wcid->link_id]);
 		if (!link_sta)
-			return -EINVAL;
+			goto error_relase_token;
 
 		dma_sync_single_for_cpu(mdev->dma_dev, tx_info->buf[1].addr,
 					tx_info->buf[1].len, DMA_TO_DEVICE);
@@ -1176,6 +1176,10 @@ int mt7996_tx_prepare_skb(struct mt76_dev *mdev, void *txwi_ptr,
 	tx_info->nbuf = MT_CT_DMA_BUF_NUM;
 
 	return 0;
+
+error_relase_token:
+	mt76_token_release(mdev, id, NULL);
+	return -EINVAL;
 }
 
 u32 mt7996_wed_init_buf(void *ptr, dma_addr_t phys, int token_id)
diff --git a/drivers/net/wireless/mediatek/mt76/tx.c b/drivers/net/wireless/mediatek/mt76/tx.c
index 22f9690634c9..f96d9c471853 100644
--- a/drivers/net/wireless/mediatek/mt76/tx.c
+++ b/drivers/net/wireless/mediatek/mt76/tx.c
@@ -933,7 +933,7 @@ mt76_token_release(struct mt76_dev *dev, int token, bool *wake)
 #endif
 	}
 
-	if (dev->token_count < dev->token_size - MT76_TOKEN_FREE_THR &&
+	if (wake && dev->token_count < dev->token_size - MT76_TOKEN_FREE_THR &&
 	    dev->phy.q_tx[0]->blocked)
 		*wake = true;
 

---
base-commit: 4913f44167cf35a9536e9eec7352e15b2de0c573
change-id: 20260531-mt7996_tx_prepare_skb-token-leack-82e240d8c66f

Best regards,
-- 
Lorenzo Bianconi <lorenzo@kernel.org>

Re: [PATCH] wifi: mt76: mt7996: Fix possible token leak in mt7996_tx_prepare_skb()

[Dylan Eskew]

Hi Lore,

We have been seeing the token memory leak in our custom kernel. After 
pulling your patch in, we are still getting the leak (validated with 
kmemleak). How did you figure out where this potential leak was? I want 
to determine if we are leaking because of our changes or if there's more 
areas for token leakage.

-- Dylan

On 5/31/26 2:10 AM, Lorenzo Bianconi wrote:
> If link_conf or link_sta lookup fails in mt7996_tx_prepare_skb routine,
> mt7996 driver leaks an already allocated tx token. Fix the issue
> releasing the token in case of error.
>
> Fixes: 7ef0c7ad735b0 ("wifi: mt76: mt7996: Implement MLD address translation for EAPOL")
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
>   drivers/net/wireless/mediatek/mt76/mt7996/mac.c | 8 ++++++--
>   drivers/net/wireless/mediatek/mt76/tx.c         | 2 +-
>   2 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/mac.c b/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
> index c98446057282..8c56344d211b 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
> @@ -1067,11 +1067,11 @@ int mt7996_tx_prepare_skb(struct mt76_dev *mdev, void *txwi_ptr,
>   
>   		link_conf = rcu_dereference(vif->link_conf[wcid->link_id]);
>   		if (!link_conf)
> -			return -EINVAL;
> +			goto error_relase_token;
>   
>   		link_sta = rcu_dereference(sta->link[wcid->link_id]);
>   		if (!link_sta)
> -			return -EINVAL;
> +			goto error_relase_token;
>   
>   		dma_sync_single_for_cpu(mdev->dma_dev, tx_info->buf[1].addr,
>   					tx_info->buf[1].len, DMA_TO_DEVICE);
> @@ -1176,6 +1176,10 @@ int mt7996_tx_prepare_skb(struct mt76_dev *mdev, void *txwi_ptr,
>   	tx_info->nbuf = MT_CT_DMA_BUF_NUM;
>   
>   	return 0;
> +
> +error_relase_token:
> +	mt76_token_release(mdev, id, NULL);
> +	return -EINVAL;
>   }
>   
>   u32 mt7996_wed_init_buf(void *ptr, dma_addr_t phys, int token_id)
> diff --git a/drivers/net/wireless/mediatek/mt76/tx.c b/drivers/net/wireless/mediatek/mt76/tx.c
> index 22f9690634c9..f96d9c471853 100644
> --- a/drivers/net/wireless/mediatek/mt76/tx.c
> +++ b/drivers/net/wireless/mediatek/mt76/tx.c
> @@ -933,7 +933,7 @@ mt76_token_release(struct mt76_dev *dev, int token, bool *wake)
>   #endif
>   	}
>   
> -	if (dev->token_count < dev->token_size - MT76_TOKEN_FREE_THR &&
> +	if (wake && dev->token_count < dev->token_size - MT76_TOKEN_FREE_THR &&
>   	    dev->phy.q_tx[0]->blocked)
>   		*wake = true;
>   
>
> ---
> base-commit: 4913f44167cf35a9536e9eec7352e15b2de0c573
> change-id: 20260531-mt7996_tx_prepare_skb-token-leack-82e240d8c66f
>
> Best regards,

Re: [PATCH] wifi: mt76: mt7996: Fix possible token leak in mt7996_tx_prepare_skb()

[Lorenzo Bianconi]

[-- Attachment #1: Type: text/plain, Size: 2978 bytes --]

> Hi Lore,

Hi Dylan,

> 
> We have been seeing the token memory leak in our custom kernel. After
> pulling your patch in, we are still getting the leak (validated with
> kmemleak). How did you figure out where this potential leak was? I want to
> determine if we are leaking because of our changes or if there's more areas
> for token leakage.

Can you please try to run kmemleak on Felix's tree to check if there are any
leftover leaks not fixed yet?

Regards,
Lorenzo

> 
> -- Dylan
> 
> On 5/31/26 2:10 AM, Lorenzo Bianconi wrote:
> > If link_conf or link_sta lookup fails in mt7996_tx_prepare_skb routine,
> > mt7996 driver leaks an already allocated tx token. Fix the issue
> > releasing the token in case of error.
> > 
> > Fixes: 7ef0c7ad735b0 ("wifi: mt76: mt7996: Implement MLD address translation for EAPOL")
> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > ---
> >   drivers/net/wireless/mediatek/mt76/mt7996/mac.c | 8 ++++++--
> >   drivers/net/wireless/mediatek/mt76/tx.c         | 2 +-
> >   2 files changed, 7 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/mac.c b/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
> > index c98446057282..8c56344d211b 100644
> > --- a/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
> > +++ b/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
> > @@ -1067,11 +1067,11 @@ int mt7996_tx_prepare_skb(struct mt76_dev *mdev, void *txwi_ptr,
> >   		link_conf = rcu_dereference(vif->link_conf[wcid->link_id]);
> >   		if (!link_conf)
> > -			return -EINVAL;
> > +			goto error_relase_token;
> >   		link_sta = rcu_dereference(sta->link[wcid->link_id]);
> >   		if (!link_sta)
> > -			return -EINVAL;
> > +			goto error_relase_token;
> >   		dma_sync_single_for_cpu(mdev->dma_dev, tx_info->buf[1].addr,
> >   					tx_info->buf[1].len, DMA_TO_DEVICE);
> > @@ -1176,6 +1176,10 @@ int mt7996_tx_prepare_skb(struct mt76_dev *mdev, void *txwi_ptr,
> >   	tx_info->nbuf = MT_CT_DMA_BUF_NUM;
> >   	return 0;
> > +
> > +error_relase_token:
> > +	mt76_token_release(mdev, id, NULL);
> > +	return -EINVAL;
> >   }
> >   u32 mt7996_wed_init_buf(void *ptr, dma_addr_t phys, int token_id)
> > diff --git a/drivers/net/wireless/mediatek/mt76/tx.c b/drivers/net/wireless/mediatek/mt76/tx.c
> > index 22f9690634c9..f96d9c471853 100644
> > --- a/drivers/net/wireless/mediatek/mt76/tx.c
> > +++ b/drivers/net/wireless/mediatek/mt76/tx.c
> > @@ -933,7 +933,7 @@ mt76_token_release(struct mt76_dev *dev, int token, bool *wake)
> >   #endif
> >   	}
> > -	if (dev->token_count < dev->token_size - MT76_TOKEN_FREE_THR &&
> > +	if (wake && dev->token_count < dev->token_size - MT76_TOKEN_FREE_THR &&
> >   	    dev->phy.q_tx[0]->blocked)
> >   		*wake = true;
> > 
> > ---
> > base-commit: 4913f44167cf35a9536e9eec7352e15b2de0c573
> > change-id: 20260531-mt7996_tx_prepare_skb-token-leack-82e240d8c66f
> > 
> > Best regards,
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH] wifi: mt76: mt7996: Fix possible token leak in mt7996_tx_prepare_skb()

[Dylan Eskew]

Hi Lore,

>> Hi Lore,
> Hi Dylan,
>
>> We have been seeing the token memory leak in our custom kernel. After
>> pulling your patch in, we are still getting the leak (validated with
>> kmemleak). How did you figure out where this potential leak was? I want to
>> determine if we are leaking because of our changes or if there's more areas
>> for token leakage.
> Can you please try to run kmemleak on Felix's tree to check if there are any
> leftover leaks not fixed yet?

Ran kmemleak with Felix's tree. I brought up only a few stations, no 
traffic run yet and kmemleak flagged a possible leak, same allocation 
location we've seen flagged in our custom kernel.

kmemleak trace:
```
unreferenced object 0xffff88811e6ca380 (size 128):
   comm "mt76-tx phy0", pid 1164, jiffies 4295044455
   hex dump (first 32 bytes):
     44 00 00 1a 0a a0 4c ae 0d 00 00 00 02 78 00 30 D.....L......x.0
     00 00 00 00 01 00 00 00 14 00 27 12 00 00 00 08 ..........'.....
   backtrace (crc fba2c5a3):
     __kmalloc_noprof+0x38e/0x480
     mt76_dma_tx_queue_skb+0x522/0x890 [mt76]
     __mt76_tx_queue_skb+0x3e/0xa0 [mt76]
     mt76_txq_schedule_pending_wcid+0x12b/0x200 [mt76]
     mt76_txq_schedule_pending+0x122/0x1b0 [mt76]
     mt76_tx_worker_run+0x1b/0xc0 [mt76]
     __mt76_worker_fn+0x49/0x90 [mt76]
     kthread+0xdc/0x110
     ret_from_fork+0x190/0x280
     ret_from_fork_asm+0x11/0x20
```

In dma.c, line 19 is the where the kmemleak trace points:
```
10 static struct mt76_txwi_cache *
11 mt76_alloc_txwi(struct mt76_dev *dev)
12 {
13         struct mt76_txwi_cache *t;
14         dma_addr_t addr;
15         u8 *txwi;
16         int size;
17
18         size = L1_CACHE_ALIGN(dev->drv->txwi_size + sizeof(*t));
19         txwi = kzalloc(size, GFP_ATOMIC);
20         if (!txwi)
21                 return NULL;
22
23         addr = dma_map_single(dev->dma_dev, txwi, dev->drv->txwi_size,
24                               DMA_TO_DEVICE);
25         if (unlikely(dma_mapping_error(dev->dma_dev, addr))) {
26                 kfree(txwi);
27                 return NULL;
28         }
29
30         t = (struct mt76_txwi_cache *)(txwi + dev->drv->txwi_size);
31         t->dma_addr = addr;
32
33         return t;
34 }
```

Let me know if you need any other information.

-- Dylan

>
> Regards,
> Lorenzo
>
>> -- Dylan
>>
>> On 5/31/26 2:10 AM, Lorenzo Bianconi wrote:
>>> If link_conf or link_sta lookup fails in mt7996_tx_prepare_skb routine,
>>> mt7996 driver leaks an already allocated tx token. Fix the issue
>>> releasing the token in case of error.
>>>
>>> Fixes: 7ef0c7ad735b0 ("wifi: mt76: mt7996: Implement MLD address translation for EAPOL")
>>> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
>>> ---
>>>    drivers/net/wireless/mediatek/mt76/mt7996/mac.c | 8 ++++++--
>>>    drivers/net/wireless/mediatek/mt76/tx.c         | 2 +-
>>>    2 files changed, 7 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/mac.c b/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
>>> index c98446057282..8c56344d211b 100644
>>> --- a/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
>>> +++ b/drivers/net/wireless/mediatek/mt76/mt7996/mac.c
>>> @@ -1067,11 +1067,11 @@ int mt7996_tx_prepare_skb(struct mt76_dev *mdev, void *txwi_ptr,
>>>    		link_conf = rcu_dereference(vif->link_conf[wcid->link_id]);
>>>    		if (!link_conf)
>>> -			return -EINVAL;
>>> +			goto error_relase_token;
>>>    		link_sta = rcu_dereference(sta->link[wcid->link_id]);
>>>    		if (!link_sta)
>>> -			return -EINVAL;
>>> +			goto error_relase_token;
>>>    		dma_sync_single_for_cpu(mdev->dma_dev, tx_info->buf[1].addr,
>>>    					tx_info->buf[1].len, DMA_TO_DEVICE);
>>> @@ -1176,6 +1176,10 @@ int mt7996_tx_prepare_skb(struct mt76_dev *mdev, void *txwi_ptr,
>>>    	tx_info->nbuf = MT_CT_DMA_BUF_NUM;
>>>    	return 0;
>>> +
>>> +error_relase_token:
>>> +	mt76_token_release(mdev, id, NULL);
>>> +	return -EINVAL;
>>>    }
>>>    u32 mt7996_wed_init_buf(void *ptr, dma_addr_t phys, int token_id)
>>> diff --git a/drivers/net/wireless/mediatek/mt76/tx.c b/drivers/net/wireless/mediatek/mt76/tx.c
>>> index 22f9690634c9..f96d9c471853 100644
>>> --- a/drivers/net/wireless/mediatek/mt76/tx.c
>>> +++ b/drivers/net/wireless/mediatek/mt76/tx.c
>>> @@ -933,7 +933,7 @@ mt76_token_release(struct mt76_dev *dev, int token, bool *wake)
>>>    #endif
>>>    	}
>>> -	if (dev->token_count < dev->token_size - MT76_TOKEN_FREE_THR &&
>>> +	if (wake && dev->token_count < dev->token_size - MT76_TOKEN_FREE_THR &&
>>>    	    dev->phy.q_tx[0]->blocked)
>>>    		*wake = true;
>>>
>>> ---
>>> base-commit: 4913f44167cf35a9536e9eec7352e15b2de0c573
>>> change-id: 20260531-mt7996_tx_prepare_skb-token-leack-82e240d8c66f
>>>
>>> Best regards,