2024-03-20 19:27:47

by Ben Greear

[permalink] [raw]
Subject: [PATCH] wifi: mt76: mt7925: Fix invalid chandef access.

From: Ben Greear <[email protected]>

mt76.ctx can be null, and that causes chandef to be null.
Use logic found in similar code paths to use chandef
from the phy object in case ctx is NULL.

With this, my system no longer crashes, but STA will still not
associate.

Signed-off-by: Ben Greear <[email protected]>
---
drivers/net/wireless/mediatek/mt76/mt7925/mcu.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt7925/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7925/mcu.c
index bd37cb8d734b..feb818411a25 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7925/mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7925/mcu.c
@@ -1502,7 +1502,8 @@ mt7925_mcu_sta_phy_tlv(struct sk_buff *skb,
struct ieee80211_vif *vif, struct ieee80211_sta *sta)
{
struct mt792x_vif *mvif = (struct mt792x_vif *)vif->drv_priv;
- struct cfg80211_chan_def *chandef = &mvif->mt76.ctx->def;
+ struct mt76_phy *mphy = mvif->phy->mt76;
+ struct cfg80211_chan_def *chandef = mvif->mt76.ctx ? &mvif->mt76.ctx->def : &mphy->chandef;
struct sta_rec_phy *phy;
struct tlv *tlv;
u8 af = 0, mm = 0;
@@ -1569,7 +1570,8 @@ mt7925_mcu_sta_rate_ctrl_tlv(struct sk_buff *skb,
struct ieee80211_vif *vif, struct ieee80211_sta *sta)
{
struct mt792x_vif *mvif = (struct mt792x_vif *)vif->drv_priv;
- struct cfg80211_chan_def *chandef = &mvif->mt76.ctx->def;
+ struct mt76_phy *mphy = mvif->phy->mt76;
+ struct cfg80211_chan_def *chandef = mvif->mt76.ctx ? &mvif->mt76.ctx->def : &mphy->chandef;
enum nl80211_band band = chandef->chan->band;
struct sta_rec_ra_info *ra_info;
struct tlv *tlv;
--
2.42.0



2024-03-20 23:03:23

by Lorenzo Bianconi

[permalink] [raw]
Subject: Re: [PATCH] wifi: mt76: mt7925: Fix invalid chandef access.

> From: Ben Greear <[email protected]>
>
> mt76.ctx can be null, and that causes chandef to be null.
> Use logic found in similar code paths to use chandef
> from the phy object in case ctx is NULL.
>
> With this, my system no longer crashes, but STA will still not
> associate.

Hi Ben,

can you please add 'Fixes' tag to help people backporting the fix?
I think it is:
Fixes: c948b5da6bbe ("wifi: mt76: mt7925: add Mediatek Wi-Fi7 driver for mt7925 chips")

Regards,
Lorenzo

>
> Signed-off-by: Ben Greear <[email protected]>
> ---
> drivers/net/wireless/mediatek/mt76/mt7925/mcu.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7925/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7925/mcu.c
> index bd37cb8d734b..feb818411a25 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7925/mcu.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7925/mcu.c
> @@ -1502,7 +1502,8 @@ mt7925_mcu_sta_phy_tlv(struct sk_buff *skb,
> struct ieee80211_vif *vif, struct ieee80211_sta *sta)
> {
> struct mt792x_vif *mvif = (struct mt792x_vif *)vif->drv_priv;
> - struct cfg80211_chan_def *chandef = &mvif->mt76.ctx->def;
> + struct mt76_phy *mphy = mvif->phy->mt76;
> + struct cfg80211_chan_def *chandef = mvif->mt76.ctx ? &mvif->mt76.ctx->def : &mphy->chandef;
> struct sta_rec_phy *phy;
> struct tlv *tlv;
> u8 af = 0, mm = 0;
> @@ -1569,7 +1570,8 @@ mt7925_mcu_sta_rate_ctrl_tlv(struct sk_buff *skb,
> struct ieee80211_vif *vif, struct ieee80211_sta *sta)
> {
> struct mt792x_vif *mvif = (struct mt792x_vif *)vif->drv_priv;
> - struct cfg80211_chan_def *chandef = &mvif->mt76.ctx->def;
> + struct mt76_phy *mphy = mvif->phy->mt76;
> + struct cfg80211_chan_def *chandef = mvif->mt76.ctx ? &mvif->mt76.ctx->def : &mphy->chandef;
> enum nl80211_band band = chandef->chan->band;
> struct sta_rec_ra_info *ra_info;
> struct tlv *tlv;
> --
> 2.42.0
>
>


Attachments:
(No filename) (1.96 kB)
signature.asc (235.00 B)
Download all attachments

2024-03-27 16:49:08

by Ben Greear

[permalink] [raw]
Subject: Re: [PATCH] wifi: mt76: mt7925: Fix invalid chandef access.

On 3/20/24 16:03, Lorenzo Bianconi wrote:
>> From: Ben Greear <[email protected]>
>>
>> mt76.ctx can be null, and that causes chandef to be null.
>> Use logic found in similar code paths to use chandef
>> from the phy object in case ctx is NULL.
>>
>> With this, my system no longer crashes, but STA will still not
>> associate.
>
> Hi Ben,
>
> can you please add 'Fixes' tag to help people backporting the fix?
> I think it is:
> Fixes: c948b5da6bbe ("wifi: mt76: mt7925: add Mediatek Wi-Fi7 driver for mt7925 chips")

I'm not able to reproduce the crash on upstream drivers, the crash only happened when
I had a local patch that caused it to not associate.

I still think the patch should go upstream as there may be other paths to hit the bug,
but maybe it doesn't need to be backported?

Let me know if you still want me to do the Fixes thing.

Thanks,
Ben

>
> Regards,
> Lorenzo
>
>>
>> Signed-off-by: Ben Greear <[email protected]>
>> ---
>> drivers/net/wireless/mediatek/mt76/mt7925/mcu.c | 6 ++++--
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/wireless/mediatek/mt76/mt7925/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7925/mcu.c
>> index bd37cb8d734b..feb818411a25 100644
>> --- a/drivers/net/wireless/mediatek/mt76/mt7925/mcu.c
>> +++ b/drivers/net/wireless/mediatek/mt76/mt7925/mcu.c
>> @@ -1502,7 +1502,8 @@ mt7925_mcu_sta_phy_tlv(struct sk_buff *skb,
>> struct ieee80211_vif *vif, struct ieee80211_sta *sta)
>> {
>> struct mt792x_vif *mvif = (struct mt792x_vif *)vif->drv_priv;
>> - struct cfg80211_chan_def *chandef = &mvif->mt76.ctx->def;
>> + struct mt76_phy *mphy = mvif->phy->mt76;
>> + struct cfg80211_chan_def *chandef = mvif->mt76.ctx ? &mvif->mt76.ctx->def : &mphy->chandef;
>> struct sta_rec_phy *phy;
>> struct tlv *tlv;
>> u8 af = 0, mm = 0;
>> @@ -1569,7 +1570,8 @@ mt7925_mcu_sta_rate_ctrl_tlv(struct sk_buff *skb,
>> struct ieee80211_vif *vif, struct ieee80211_sta *sta)
>> {
>> struct mt792x_vif *mvif = (struct mt792x_vif *)vif->drv_priv;
>> - struct cfg80211_chan_def *chandef = &mvif->mt76.ctx->def;
>> + struct mt76_phy *mphy = mvif->phy->mt76;
>> + struct cfg80211_chan_def *chandef = mvif->mt76.ctx ? &mvif->mt76.ctx->def : &mphy->chandef;
>> enum nl80211_band band = chandef->chan->band;
>> struct sta_rec_ra_info *ra_info;
>> struct tlv *tlv;
>> --
>> 2.42.0
>>
>>

--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com