[PATCH] wifi: wfx: reserve zeroed slack for the bus alignment padding on TX

[PATCH] wifi: wfx: reserve zeroed slack for the bus alignment padding on TX

[Chengyu Liu]

tx_helper() rounds the transfer length up to the bus alignment (4
bytes for SPI, 64 bytes for SDIO) and passes the rounded length to
wfx_data_write(), which reads the padding from the supplied buffer.
However, none of the TX paths guarantees that this space is available
and zeroed. wfx_alloc_hif() allocates only
sizeof(struct wfx_hif_msg) + body_len, wfx_send_hif_msg_write() copies
exactly count bytes with memdup_user(), and data frames are sent
directly from the skb, whose tailroom is not guaranteed to be
sufficient or zeroed.

so the padding is an out-of-bounds read leaking heap data to the
device, the same defect class as the cw1200 fix "wifi: cw1200: fix TX
padding OOB read leaking heap data to device".

Reserve WFX_BUS_ALIGN_SLACK (64, the SDIO block size; SPI only needs
4) zeroed bytes at the end of every TX buffer: extend the two exact
allocations, and in wfx_tx_inner() ensure the skb tailroom and zero
it, re-deriving hdr since pskb_expand_head() may reallocate the
buffer.  Cloned skbs are uncloned as well, since the slack memset
must not modify shared data, following the same check used by
__skb_pad().

Found by static analysis.

Fixes: 4a5fb1bbcdf1 ("wfx: get out from the staging area")
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/all/7982b4a7e64f74e2068790ec389ca680a081f730@linux.dev/
Signed-off-by: Chengyu Liu <lui2012qwq@gmail.com>
---
 drivers/net/wireless/silabs/wfx/data_tx.c | 14 ++++++++++++++
 drivers/net/wireless/silabs/wfx/debug.c   | 11 ++++++++---
 drivers/net/wireless/silabs/wfx/hif_tx.c  |  4 +++-
 drivers/net/wireless/silabs/wfx/wfx.h     |  8 ++++++++
 4 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/silabs/wfx/data_tx.c b/drivers/net/wireless/silabs/wfx/data_tx.c
index a44a7403ce..985eba3378 100644
--- a/drivers/net/wireless/silabs/wfx/data_tx.c
+++ b/drivers/net/wireless/silabs/wfx/data_tx.c
@@ -357,8 +357,22 @@ static int wfx_tx_inner(struct wfx_vif *wvif, struct ieee80211_sta *sta, struct
 	WARN(skb_headroom(skb) < wmsg_len, "not enough space in skb");
 	WARN(offset & 1, "attempt to transmit an unaligned frame");
 	skb_put(skb, tx_priv->icv_size);
+	/*
+	 * The bus layer pads the transfer up to the bus alignment and
+	 * the padding is transferred from the skb.  Guarantee tailroom
+	 * for the slack, uncloning the skb if needed: the memset below
+	 * must not write into a shared buffer.
+	 */
+	if (skb_cloned(skb) || skb_tailroom(skb) < WFX_BUS_ALIGN_SLACK) {
+		int ntail = WFX_BUS_ALIGN_SLACK - skb_tailroom(skb);
+
+		if (pskb_expand_head(skb, 0, max(ntail, 0), GFP_ATOMIC))
+			return -ENOMEM;
+	}
+	memset(skb_tail_pointer(skb), 0, WFX_BUS_ALIGN_SLACK);
 	skb_push(skb, wmsg_len);
 	memset(skb->data, 0, wmsg_len);
+	hdr = (struct ieee80211_hdr *)(skb->data + wmsg_len);
 	hif_msg = (struct wfx_hif_msg *)skb->data;
 	hif_msg->len = cpu_to_le16(skb->len);
 	hif_msg->id = HIF_REQ_ID_TX;
diff --git a/drivers/net/wireless/silabs/wfx/debug.c b/drivers/net/wireless/silabs/wfx/debug.c
index 454ec59fdc..f87f0e059c 100644
--- a/drivers/net/wireless/silabs/wfx/debug.c
+++ b/drivers/net/wireless/silabs/wfx/debug.c
@@ -255,9 +255,14 @@ static ssize_t wfx_send_hif_msg_write(struct file *file, const char __user *user
 	 * memory pattern may help user.
 	 */
 	memset(context->reply, 0xFF, sizeof(context->reply));
-	request = memdup_user(user_buf, count);
-	if (IS_ERR(request))
-		return PTR_ERR(request);
+	/* Reserve room for the bus alignment slack */
+	request = kzalloc(count + WFX_BUS_ALIGN_SLACK, GFP_KERNEL);
+	if (!request)
+		return -ENOMEM;
+	if (copy_from_user(request, user_buf, count)) {
+		kfree(request);
+		return -EFAULT;
+	}
 	if (le16_to_cpu(request->len) != count) {
 		kfree(request);
 		return -EINVAL;
diff --git a/drivers/net/wireless/silabs/wfx/hif_tx.c b/drivers/net/wireless/silabs/wfx/hif_tx.c
index 9efb108bd8..45db1cf7f7 100644
--- a/drivers/net/wireless/silabs/wfx/hif_tx.c
+++ b/drivers/net/wireless/silabs/wfx/hif_tx.c
@@ -38,7 +38,9 @@ static void wfx_fill_header(struct wfx_hif_msg *hif, int if_id, unsigned int cmd
 
 static void *wfx_alloc_hif(size_t body_len, struct wfx_hif_msg **hif)
 {
-	*hif = kzalloc(sizeof(struct wfx_hif_msg) + body_len, GFP_KERNEL);
+	/* The kzalloc() also zeroes the bus alignment slack */
+	*hif = kzalloc(sizeof(struct wfx_hif_msg) + body_len +
+		       WFX_BUS_ALIGN_SLACK, GFP_KERNEL);
 	if (*hif)
 		return (*hif)->body;
 	else
diff --git a/drivers/net/wireless/silabs/wfx/wfx.h b/drivers/net/wireless/silabs/wfx/wfx.h
index bd0df2e1ea..6a6c8a47a8 100644
--- a/drivers/net/wireless/silabs/wfx/wfx.h
+++ b/drivers/net/wireless/silabs/wfx/wfx.h
@@ -25,6 +25,14 @@
 #define USEC_PER_TXOP 32 /* see struct ieee80211_tx_queue_params */
 #define USEC_PER_TU 1024
 
+/*
+ * The bus layer pads TX transfers up to the bus alignment and the
+ * padding is transferred from the message buffer, so every buffer
+ * handed to tx_helper() must carry this much zeroed slack (the SDIO
+ * block size is 64 bytes, SPI transfers are aligned to 4).
+ */
+#define WFX_BUS_ALIGN_SLACK 64
+
 struct wfx_hwbus_ops;
 
 struct wfx_dev {
-- 
2.25.1