[PATCH v6] wifi: mt76: Fix NULL Dereference caused by mt76_connac_get_he_phy_cap()

[PATCH v6] wifi: mt76: Fix NULL Dereference caused by mt76_connac_get_he_phy_cap()

[Gax-c]

From: Zichen Xie <zichenxie0106@gmail.com>

mt76_connac_get_he_phy_cap() may return a NULL pointer,
leading to NULL Pointer Dereference.
Add a NULL check for the returned pointer.

Fixes: a5c372f77aa7 ("wifi: mt76: mt7925: extend mt7925_mcu_bss_he_tlv for per-link BSS")
Fixes: e6d557a78b60 ("mt76: mt7915: rely on mt76_connac_get_phy utilities")
Fixes: 98686cd21624 ("wifi: mt76: mt7996: add driver for MediaTek Wi-Fi 7 (802.11be) devices")
Signed-off-by: Zichen Xie <zichenxie0106@gmail.com>
---
v5: Add version tag.
v6: Adjust NULL check position for readability.
---
 drivers/net/wireless/mediatek/mt76/mt7915/mcu.c | 5 +++++
 drivers/net/wireless/mediatek/mt76/mt7925/mcu.c | 2 ++
 drivers/net/wireless/mediatek/mt76/mt7996/mcu.c | 2 ++
 3 files changed, 9 insertions(+)

diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c
index 87d0dd040001..4d53f819c5f1 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c
@@ -551,6 +551,8 @@ mt7915_mcu_bss_he_tlv(struct sk_buff *skb, struct ieee80211_vif *vif,
 	struct tlv *tlv;
 
 	cap = mt76_connac_get_he_phy_cap(phy->mt76, vif);
+	if (!cap)
+		return;
 
 	tlv = mt76_connac_mcu_add_tlv(skb, BSS_INFO_HE_BASIC, sizeof(*he));
 
@@ -1140,6 +1142,9 @@ mt7915_mcu_sta_bfer_he(struct ieee80211_sta *sta, struct ieee80211_vif *vif,
 	struct ieee80211_he_cap_elem *pe = &pc->he_cap_elem;
 	const struct ieee80211_sta_he_cap *vc =
 		mt76_connac_get_he_phy_cap(phy->mt76, vif);
+	if (!vc)
+		return;
+
 	const struct ieee80211_he_cap_elem *ve = &vc->he_cap_elem;
 	u16 mcs_map = le16_to_cpu(pc->he_mcs_nss_supp.rx_mcs_80);
 	u8 nss_mcs = mt7915_mcu_get_sta_nss(mcs_map);
diff --git a/drivers/net/wireless/mediatek/mt76/mt7925/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7925/mcu.c
index 748ea6adbc6b..55e4cda2f20f 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7925/mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7925/mcu.c
@@ -2508,6 +2508,8 @@ mt7925_mcu_bss_he_tlv(struct sk_buff *skb, struct ieee80211_bss_conf *link_conf,
 	struct tlv *tlv;
 
 	cap = mt76_connac_get_he_phy_cap(phy->mt76, link_conf->vif);
+	if (!cap)
+		return;
 
 	tlv = mt76_connac_mcu_add_tlv(skb, UNI_BSS_INFO_HE_BASIC, sizeof(*he));
 
diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
index 6c445a9dbc03..55bb2d0e67e5 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7996/mcu.c
@@ -798,6 +798,8 @@ mt7996_mcu_bss_he_tlv(struct sk_buff *skb, struct ieee80211_vif *vif,
 	struct tlv *tlv;
 
 	cap = mt76_connac_get_he_phy_cap(phy->mt76, vif);
+	if (!cap)
+		return;
 
 	tlv = mt7996_mcu_add_uni_tlv(skb, UNI_BSS_INFO_HE_BASIC, sizeof(*he));
 
-- 
2.25.1

Re: [PATCH v6] wifi: mt76: Fix NULL Dereference caused by mt76_connac_get_he_phy_cap()

[Krzysztof Kozlowski]

On 08/10/2024 01:35, Gax-c wrote:
> From: Zichen Xie <zichenxie0106@gmail.com>
> 
> mt76_connac_get_he_phy_cap() may return a NULL pointer,
> leading to NULL Pointer Dereference.
> Add a NULL check for the returned pointer.
> 
> Fixes: a5c372f77aa7 ("wifi: mt76: mt7925: extend mt7925_mcu_bss_he_tlv for per-link BSS")
> Fixes: e6d557a78b60 ("mt76: mt7915: rely on mt76_connac_get_phy utilities")
> Fixes: 98686cd21624 ("wifi: mt76: mt7996: add driver for MediaTek Wi-Fi 7 (802.11be) devices")
> Signed-off-by: Zichen Xie <zichenxie0106@gmail.com>
> ---
> v5: Add version tag.
> v6: Adjust NULL check position for readability.
> ---
>  drivers/net/wireless/mediatek/mt76/mt7915/mcu.c | 5 +++++
>  drivers/net/wireless/mediatek/mt76/mt7925/mcu.c | 2 ++
>  drivers/net/wireless/mediatek/mt76/mt7996/mcu.c | 2 ++
>  3 files changed, 9 insertions(+)
> 
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c
> index 87d0dd040001..4d53f819c5f1 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7915/mcu.c
> @@ -551,6 +551,8 @@ mt7915_mcu_bss_he_tlv(struct sk_buff *skb, struct ieee80211_vif *vif,
>  	struct tlv *tlv;
>  
>  	cap = mt76_connac_get_he_phy_cap(phy->mt76, vif);
> +	if (!cap)
> +		return;
>  
>  	tlv = mt76_connac_mcu_add_tlv(skb, BSS_INFO_HE_BASIC, sizeof(*he));
>  
> @@ -1140,6 +1142,9 @@ mt7915_mcu_sta_bfer_he(struct ieee80211_sta *sta, struct ieee80211_vif *vif,
>  	struct ieee80211_he_cap_elem *pe = &pc->he_cap_elem;
>  	const struct ieee80211_sta_he_cap *vc =
>  		mt76_connac_get_he_phy_cap(phy->mt76, vif);
> +	if (!vc)
> +		return;
> +

Nope. Splitting declarations is a no.

One of your patches did not even build, although for few emails you were
insisting everything is fine. This is not even close to Linux coding style.

Please carefully read Documentation/process before posting new patches.

Best regards,
Krzysztof

Re: [PATCH v6] wifi: mt76: Fix NULL Dereference caused by mt76_connac_get_he_phy_cap()

[Krzysztof Kozlowski]

On 08/10/2024 16:34, Zichen Xie wrote:
> Hi,
> 
> But Matthias suggested checking for NULL immediately after the declaration.

Why do you keep top-posting? Matthias also asked you to stop, so that
part you ignore? Responding to such HTML, top-post emails is a pain, so
that is the last time I am writing here. I will be NAK-ing other totally
bogus and untested patches, though.

> Re: [PATCH v5] wifi: mt76: Fix NULL Dereference caused by
> mt76_connac_get_he_phy_cap() - Matthias Brugger (kernel.org)
> <https://lore.kernel.org/all/7daf8976-91ee-4045-a9a7-d9d14d53c6dd@gmail.com/>

Don't send HTML. Anyway I do not see anything indicating that this
should be before declarations. I see clearly: "before any use of struct
vc." which is not before declarations.

Best regards,
Krzysztof