linux-kernel - Re: [PATCH] wifi: mt76: mt7996: avoid potential null deref in mt7996_get_et

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: <CAAMvbhGrJ9b3Aab9+2a5zmvEgf0GZFmHLCC7Hud+egUE28voHQ@mail.gmail.com>
Date: Sun, 23 Mar 2025 11:59:45 +0000
From: James Dutton <james.dutton@...il.com>
To: Johannes Berg <johannes@...solutions.net>
Cc: Markus Elfring <Markus.Elfring@....de>, Qasim Ijaz <qasdev00@...il.com>, 
	linux-wireless@...r.kernel.org, linux-mediatek@...ts.infradead.org, 
	linux-arm-kernel@...ts.infradead.org, LKML <linux-kernel@...r.kernel.org>, 
	Angelo Gioacchino Del Regno <angelogioacchino.delregno@...labora.com>, Bo Jiao <bo.jiao@...iatek.com>, 
	Dan Carpenter <dan.carpenter@...aro.org>, Felix Fietkau <nbd@....name>, 
	Lorenzo Bianconi <lorenzo@...nel.org>, Matthias Brugger <matthias.bgg@...il.com>, 
	Peter Chiu <chui-hao.chiu@...iatek.com>, Ryder Lee <ryder.lee@...iatek.com>, 
	Sean Wang <sean.wang@...iatek.com>, Shayne Chen <shayne.chen@...iatek.com>
Subject: Re: [PATCH] wifi: mt76: mt7996: avoid potential null deref in mt7996_get_et_stats()

As a security side note in relation to the following patch:
diff --git a/drivers/net/wireless/mediatek/mt76/mt7996/main.c
b/drivers/net/wireless/mediatek/mt76/mt7996/main.c
index 66575698aef1..88e013577c0d 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7996/main.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7996/main.c
@@ -68,11 +68,13 @@ static int mt7996_start(struct ieee80211_hw *hw)

 static void mt7996_stop_phy(struct mt7996_phy *phy)
 {
-       struct mt7996_dev *dev = phy->dev;
+       struct mt7996_dev *dev;

        if (!phy || !test_bit(MT76_STATE_RUNNING, &phy->mt76->state))
                return;

+       dev = phy->dev;
+
        cancel_delayed_work_sync(&phy->mt76->mac_work);

        mutex_lock(&dev->mt76.mutex);



Prior to that patch, the code looks like this:
static void mt7996_stop_phy(struct mt7996_phy *phy)
 {
       struct mt7996_dev *dev = phy->dev;

        if (!phy || !test_bit(MT76_STATE_RUNNING, &phy->mt76->state))
                return;


The compiler will completely remove the !phy check entirely because of
the use above it, so it being present in the source code is completely
bogus.
If one actually needs a !phy check to be present in the compiled code,
one must arrange it as per the patch above.

The fact that the !phy check is in the source code, implies to me that
someone, in the past, thought it was necessary, but I think an opinion
could be taken that it is there to obfuscate a security vulnerability.

Kind Regards

James