Re: [PATCH] mt76: mt7921: fix the coredump is being truncated

From: Sean Wang <sean.wang@mediatek.com>

quoted

From: Sean Wang <sean.wang@mediatek.com>

Fix the maximum size of the coredump generated with current mt7921
firmware. Otherwise, a truncated coredump would be reported to
userland via dev_coredumpv.

Also, there is an additional error handling enhanced in the patch to
avoid the possible invalid buffer access when the system failed to
create the buffer to hold the coredump.

Fixes: 0da3c795d07b ("mt76: mt7921: add coredump support")
Co-developed-by: YN Chen <redacted>
Signed-off-by: YN Chen <redacted>
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
 drivers/net/wireless/mediatek/mt76/mt76_connac.h | 2 +-
drivers/net/wireless/mediatek/mt76/mt7921/mac.c  | 9 ++++++---
 2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt76_connac.h

b/drivers/net/wireless/mediatek/mt76/mt76_connac.h
index 9b3f8d22f17e..d93ab1ece8ae 100644

--- a/drivers/net/wireless/mediatek/mt76/mt76_connac.h
+++ b/drivers/net/wireless/mediatek/mt76/mt76_connac.h

@@ -13,7 +13,7 @@
 #define MT76_CONNAC_MAX_SCAN_MATCH		16

 #define MT76_CONNAC_COREDUMP_TIMEOUT		(HZ / 20)
-#define MT76_CONNAC_COREDUMP_SZ			(128 * 1024)
+#define MT76_CONNAC_COREDUMP_SZ			(1300 * 1024)

 enum {

CMD_CBW_20MHZ = IEEE80211_STA_RX_BW_20, diff --git
a/drivers/net/wireless/mediatek/mt76/mt7921/mac.c
b/drivers/net/wireless/mediatek/mt76/mt7921/mac.c
index fb4de73df701..905dddbfbb0b 100644

--- a/drivers/net/wireless/mediatek/mt76/mt7921/mac.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7921/mac.c

@@ -1557,7 +1557,7 @@ void mt7921_coredump_work(struct work_struct *work)

		break;

	skb_pull(skb, sizeof(struct mt7921_mcu_rxd));
-		if (data + skb->len - dump > MT76_CONNAC_COREDUMP_SZ) {
+		if (!dump || data + skb->len - dump > MT76_CONNAC_COREDUMP_SZ) {

why not just return if dump allocation fails? Doing so we will reset the device even if dump is NULL, do you think it is a real use-case?

We cannot just return if dump allocation fails because we still must properly free skb in coredump.msg_list to avoid
the memory leak.

Reset the device is eventually required even dump is NULL because mt7921 cannot work anymore in case coredump happens
so that needs the following reset to recover it back in time.

Regards,
Lorenzo

quoted

		dev_kfree_skb(skb);
		continue;
	}

@@ -1567,7 +1567,10 @@ void mt7921_coredump_work(struct work_struct

*work)

	dev_kfree_skb(skb);
}
-	dev_coredumpv(dev->mt76.dev, dump, MT76_CONNAC_COREDUMP_SZ,
-		      GFP_KERNEL);
+
+	if (dump)
+		dev_coredumpv(dev->mt76.dev, dump, MT76_CONNAC_COREDUMP_SZ,
+			      GFP_KERNEL);
+
mt7921_reset(&dev->mt76);
 }
--
2.25.1

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek