[PATCH 3/3] wifi: mt76: mt7925: accept testmode replies shorter than the buffer

JB Tsai <[email protected]>
Newsgroups org.kernel.vger.linux-wireless,org.infradead.lists.linux-mediatek
Message-ID <[email protected]>
From: Tony-TC Lee <[email protected]>

The length check guarding the memcpy requires every event to carry a
full MT7925_EVT_RSP_LEN (512) byte payload. That suits
MCU_UNI_QUERY(TESTMODE_RX_STAT), whose reply is described by
union testmode_evt, but a MCU_UNI_QUERY(TESTMODE_CTRL) reply - an AT
command read - is laid out as struct uni_cmd_testmode_ctrl: an 8 byte
header followed by a union testmode_data of 84 bytes, 92 in total.
Measured on MT7925, cid 0x0046 answers with skb->len = 92, so every RF
test read fails with -EINVAL even though the firmware answered
correctly. The failure is invisible from userspace: a dump ending in an
error carries it in the NLMSG_DONE payload, which netlink libraries
routinely discard, so it looks like an empty reply.

Keep the over-read fixed, but step over the header with skb_pull() so
the bounds check and the advance are one operation and the offset comes
from offsetof() rather than a literal, then copy whatever payload did
arrive. Zero evt_resp at its declaration in the caller, which owns it
and hands it to userspace at full MT7925_EVT_RSP_LEN however much the
firmware sent, so the part left unfilled cannot leak stack content.

Fixes: c7369a00860a ("wifi: mt76: mt7925: validate skb length in testmode query")
Signed-off-by: Tony-TC Lee <[email protected]>
---
 drivers/net/wireless/mediatek/mt76/mt7925/testmode.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt7925/testmode.c b/drivers/net/wireless/mediatek/mt76/mt7925/testmode.c
index bf9516bc7bd1..dbf1159effeb 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7925/testmode.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7925/testmode.c
@@ -87,6 +87,7 @@ static int
 mt7925_tm_query(struct mt792x_dev *dev, struct mt7925_tm_cmd *req,
 		char *evt_resp)
 {
+	const unsigned int hdr = offsetof(struct uni_cmd_testmode_ctrl, data);
 	struct mt7925_rftest_cmd cmd;
 	struct mt7925_rftest_cmd *pcmd = &cmd;
 	struct sk_buff *skb = NULL;
@@ -107,12 +108,12 @@ mt7925_tm_query(struct mt792x_dev *dev, struct mt7925_tm_cmd *req,
 	if (ret)
 		goto out;
 
-	if (skb->len < MT7925_EVT_RSP_LEN + 8) {
+	if (!skb_pull(skb, hdr)) {
 		ret = -EINVAL;
 		goto out;
 	}
 
-	memcpy((char *)evt_resp, (char *)skb->data + 8, MT7925_EVT_RSP_LEN);
+	memcpy(evt_resp, skb->data, min_t(u32, skb->len, MT7925_EVT_RSP_LEN));
 
 out:
 	dev_kfree_skb(skb);
@@ -192,7 +193,7 @@ int mt7925_testmode_dump(struct ieee80211_hw *hw, struct sk_buff *msg,
 
 		data = drv_tb[MT7925_TM_ATTR_QUERY];
 		if (data) {
-			char evt_resp[MT7925_EVT_RSP_LEN];
+			char evt_resp[MT7925_EVT_RSP_LEN] = {};
 
 			err = mt7925_tm_query(phy->dev, nla_data(data),
 					      evt_resp);
-- 
2.45.2
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.