All of lore.kernel.org
 help / color / mirror / Atom feed
From: luka.gejak@linux.dev
To: Ping-Ke Shih <pkshih@realtek.com>
Cc: linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
	Michael Straube <straube.linux@gmail.com>,
	Peter Robinson <pbrobinson@gmail.com>,
	Bitterblue Smith <rtl8821cerfe2@gmail.com>,
	Luka Gejak <luka.gejak@linux.dev>
Subject: [PATCH v11 4/7] wifi: rtw88: sdio: zero the padding added to a TX transfer
Date: Wed,  9 Sep 2026 09:45:53 +0200	[thread overview]
Message-ID: <20260909074556.55709-5-luka.gejak@linux.dev> (raw)
In-Reply-To: <20260909074556.55709-1-luka.gejak@linux.dev>

From: Luka Gejak <luka.gejak@linux.dev>

rtw_sdio_write_port() rounds the transfer up with sdio_align_size() and
then hands that length to sdio_memcpy_toio() while the skb still only
holds skb->len bytes. The difference, between one and 511 bytes, is read
from beyond the end of the frame and transmitted. Whether it stays
inside the skb's allocation depends on how much tailroom the skb happens
to have, so this is at best sending uninitialised memory over the air.

Pad the skb up to the transfer size first. __skb_pad() zeroes the added
bytes, reallocates a cloned skb rather than writing into a buffer a
clone still shares, and leaves skb->len alone, so nothing else in the
transmit path has to change.

It must not free the skb on failure: rtw_sdio_write_data() frees the skb
itself and rtw_sdio_process_tx_queue() requeues it, so both callers
still own it and would double free.

Found while reworking this path for the RTL8723BS. Measured on RTL8723BS
hardware, padding the transfer costs nothing observable: uplink is
19.5 to 19.8 Mbit/s padded against 20.9 to 21.1 Mbit/s unpadded in an
interleaved A/B, with scans, reconnection and a UDP flood clean in both.
The other SDIO parts sharing this path are untested; I have only the
RTL8723BS.

Fixes: 65371a3f14e7 ("wifi: rtw88: sdio: Add HCI implementation for SDIO based chipsets")
Signed-off-by: Luka Gejak <luka.gejak@linux.dev>
---

Notes:
    New in v11.
    
      Ping-Ke asked for this to come before the RTL8723BS accounting patch
      rather than after the series, so that it backports on its own and so
      that it is clear it is an existing problem rather than something the
      RTL8723BS work introduced.
    
      The pad_size local is the shape he asked for on v9: declared at the
      top, computed unconditionally, and tested with if (pad_size > 0).

 drivers/net/wireless/realtek/rtw88/sdio.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/net/wireless/realtek/rtw88/sdio.c b/drivers/net/wireless/realtek/rtw88/sdio.c
index 5b40d74b16ee..8466abad972a 100644
--- a/drivers/net/wireless/realtek/rtw88/sdio.c
+++ b/drivers/net/wireless/realtek/rtw88/sdio.c
@@ -636,6 +636,7 @@ static int rtw_sdio_write_port(struct rtw_dev *rtwdev, struct sk_buff *skb,
 			       enum rtw_tx_queue_type queue)
 {
 	struct rtw_sdio *rtwsdio = (struct rtw_sdio *)rtwdev->priv;
+	size_t pad_size;
 	bool bus_claim;
 	size_t txsize;
 	u32 txaddr;
@@ -646,6 +647,17 @@ static int rtw_sdio_write_port(struct rtw_dev *rtwdev, struct sk_buff *skb,
 		return -EINVAL;
 
 	txsize = sdio_align_size(rtwsdio->sdio_func, skb->len);
+	pad_size = txsize - skb->len;
+
+	if (pad_size > 0) {
+		/*
+		 * __skb_pad() must not free the skb on failure: both callers
+		 * still own it, one requeues it and the other frees it.
+		 */
+		ret = __skb_pad(skb, pad_size, false);
+		if (ret)
+			return ret;
+	}
 
 	ret = rtw_sdio_check_free_txpg(rtwdev, queue, txsize);
 	if (ret)
-- 
2.55.0


  parent reply	other threads:[~2026-09-09  7:46 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-09  7:45 [PATCH v11 0/7] wifi: rtw88: preparations for RTL8723B/RTL8723BS luka.gejak
2026-09-09  7:45 ` [PATCH v11 1/7] wifi: rtw88: add the RTL8723B chip type and SDIO helper luka.gejak
2026-09-09  7:45 ` [PATCH v11 2/7] wifi: rtw88: rx: mark zero length packets on RTL8723BS luka.gejak
2026-09-09  7:45 ` [PATCH v11 3/7] wifi: rtw88: tx: extend the TX report purge timeout to RTL8723BS luka.gejak
2026-09-09  7:45 ` luka.gejak [this message]
2026-09-10  2:24   ` [PATCH v11 4/7] wifi: rtw88: sdio: zero the padding added to a TX transfer Ping-Ke Shih
2026-09-09  7:45 ` [PATCH v11 5/7] wifi: rtw88: sdio: track free TX pages and OQT credits for RTL8723BS luka.gejak
2026-09-10  2:40   ` Ping-Ke Shih
2026-09-09  7:45 ` [PATCH v11 6/7] wifi: rtw88: sdio: set up RX aggregation and interrupts " luka.gejak
2026-09-09  7:45 ` [PATCH v11 7/7] wifi: rtw88: sdio: add TX back-pressure and retry on page starvation luka.gejak

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260909074556.55709-5-luka.gejak@linux.dev \
    --to=luka.gejak@linux.dev \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=pbrobinson@gmail.com \
    --cc=pkshih@realtek.com \
    --cc=rtl8821cerfe2@gmail.com \
    --cc=straube.linux@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.