Re: [PATCH rtw-next] wifi: rtl8xxxu: isolate the USB register buffer from CPU state

[Bitterblue Smith]

On 10/10/2026 01:39, Kim Wooseok via B4 Relay wrote:
> From: Kim Wooseok <5mghybrid@xxxxxxxxx>
> 
> An RTL8192EU failed to reconnect on a Raspberry Pi after an AP power
> cycle. RF_MODE contained a corrupted value, and restoring its initialized
> value restored reception.
> 
> The register access helpers pass usb_buf directly to usb_control_msg(),
> but the buffer is embedded next to usb_buf_mutex. On the affected Pi,
> it shares a cache line with the mutex's waiter state. A contending task
> updates that state while the USB transfer uses the buffer. This violates
> the cache-line isolation required for non-coherent DMA.
> 

Perhaps it's a silly question, but doesn't this just move the problem
elsewhere? Or does kmalloc_obj() guarantee that nothing else will access
the memory near usb_buf?

> Allocate the buffer once in probe and reuse it, avoiding an allocation
> for every register access.

An allocation for every register access was not happening even before
this patch. I guess your LLM wrote this...

> Keep the existing register access helpers and
> mutex protection. Free the buffer on probe failure and after power_off()
> in disconnect, since shutdown still uses the register access helpers.
> 
> Fixes: 26f1fad29ad9 ("New driver: rtl8xxxu (mac80211)")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Kim Wooseok <5mghybrid@xxxxxxxxx>
> ---
> I tested the separate-buffer approach on an RTL8192EU connected to a
> Raspberry Pi 5, using the existing Linux 6.18.34 kernel. While repeatedly
> requesting scans, wireless ping traffic kept receiving replies and the
> RF_MODE and HSSI register values remained stable.
> 
> The submission is based on rtw-next. An ARM64 build of the rtl8xxxu
> driver with W=1 completed without warnings, and sparse reported no
> diagnostics in core.c.
> ---
>  drivers/net/wireless/realtek/rtl8xxxu/core.c     | 35 ++++++++++++++++--------
>  drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h |  2 +-
>  2 files changed, 24 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/core.c b/drivers/net/wireless/realtek/rtl8xxxu/core.c
> index 3d0c22db3..604bb1e5b 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/core.c
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/core.c
> @@ -622,9 +622,9 @@ u8 rtl8xxxu_read8(struct rtl8xxxu_priv *priv, u16 addr)
>  	mutex_lock(&priv->usb_buf_mutex);
>  	len = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
>  			      REALTEK_USB_CMD_REQ, REALTEK_USB_READ,
> -			      addr, 0, &priv->usb_buf.val8, sizeof(u8),
> +			      addr, 0, &priv->usb_buf->val8, sizeof(u8),
>  			      RTW_USB_CONTROL_MSG_TIMEOUT);
> -	data = priv->usb_buf.val8;
> +	data = priv->usb_buf->val8;
>  	mutex_unlock(&priv->usb_buf_mutex);
>  
>  	if (rtl8xxxu_debug & RTL8XXXU_DEBUG_REG_READ)
> @@ -645,9 +645,9 @@ u16 rtl8xxxu_read16(struct rtl8xxxu_priv *priv, u16 addr)
>  	mutex_lock(&priv->usb_buf_mutex);
>  	len = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
>  			      REALTEK_USB_CMD_REQ, REALTEK_USB_READ,
> -			      addr, 0, &priv->usb_buf.val16, sizeof(u16),
> +			      addr, 0, &priv->usb_buf->val16, sizeof(u16),
>  			      RTW_USB_CONTROL_MSG_TIMEOUT);
> -	data = le16_to_cpu(priv->usb_buf.val16);
> +	data = le16_to_cpu(priv->usb_buf->val16);
>  	mutex_unlock(&priv->usb_buf_mutex);
>  
>  	if (rtl8xxxu_debug & RTL8XXXU_DEBUG_REG_READ)
> @@ -668,9 +668,9 @@ u32 rtl8xxxu_read32(struct rtl8xxxu_priv *priv, u16 addr)
>  	mutex_lock(&priv->usb_buf_mutex);
>  	len = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
>  			      REALTEK_USB_CMD_REQ, REALTEK_USB_READ,
> -			      addr, 0, &priv->usb_buf.val32, sizeof(u32),
> +			      addr, 0, &priv->usb_buf->val32, sizeof(u32),
>  			      RTW_USB_CONTROL_MSG_TIMEOUT);
> -	data = le32_to_cpu(priv->usb_buf.val32);
> +	data = le32_to_cpu(priv->usb_buf->val32);
>  	mutex_unlock(&priv->usb_buf_mutex);
>  
>  	if (rtl8xxxu_debug & RTL8XXXU_DEBUG_REG_READ)
> @@ -688,10 +688,10 @@ int rtl8xxxu_write8(struct rtl8xxxu_priv *priv, u16 addr, u8 val)
>  		addr |= 0x8000;
>  
>  	mutex_lock(&priv->usb_buf_mutex);
> -	priv->usb_buf.val8 = val;
> +	priv->usb_buf->val8 = val;
>  	ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
>  			      REALTEK_USB_CMD_REQ, REALTEK_USB_WRITE,
> -			      addr, 0, &priv->usb_buf.val8, sizeof(u8),
> +			      addr, 0, &priv->usb_buf->val8, sizeof(u8),
>  			      RTW_USB_CONTROL_MSG_TIMEOUT);
>  
>  	mutex_unlock(&priv->usb_buf_mutex);
> @@ -711,10 +711,10 @@ int rtl8xxxu_write16(struct rtl8xxxu_priv *priv, u16 addr, u16 val)
>  		addr |= 0x8000;
>  
>  	mutex_lock(&priv->usb_buf_mutex);
> -	priv->usb_buf.val16 = cpu_to_le16(val);
> +	priv->usb_buf->val16 = cpu_to_le16(val);
>  	ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
>  			      REALTEK_USB_CMD_REQ, REALTEK_USB_WRITE,
> -			      addr, 0, &priv->usb_buf.val16, sizeof(u16),
> +			      addr, 0, &priv->usb_buf->val16, sizeof(u16),
>  			      RTW_USB_CONTROL_MSG_TIMEOUT);
>  	mutex_unlock(&priv->usb_buf_mutex);
>  
> @@ -733,10 +733,10 @@ int rtl8xxxu_write32(struct rtl8xxxu_priv *priv, u16 addr, u32 val)
>  		addr |= 0x8000;
>  
>  	mutex_lock(&priv->usb_buf_mutex);
> -	priv->usb_buf.val32 = cpu_to_le32(val);
> +	priv->usb_buf->val32 = cpu_to_le32(val);
>  	ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
>  			      REALTEK_USB_CMD_REQ, REALTEK_USB_WRITE,
> -			      addr, 0, &priv->usb_buf.val32, sizeof(u32),
> +			      addr, 0, &priv->usb_buf->val32, sizeof(u32),
>  			      RTW_USB_CONTROL_MSG_TIMEOUT);
>  	mutex_unlock(&priv->usb_buf_mutex);
>  
> @@ -7851,6 +7851,13 @@ static int rtl8xxxu_probe(struct usb_interface *interface,
>  
>  	usb_set_intfdata(interface, hw);
>  
> +	/* Keep the USB register buffer separate from driver state. */
> +	priv->usb_buf = kzalloc_obj(*priv->usb_buf);
> +	if (!priv->usb_buf) {
> +		ret = -ENOMEM;
> +		goto err_set_intfdata;
> +	}
> +
>  	ret = rtl8xxxu_parse_usb(priv, interface);
>  	if (ret)
>  		goto err_set_intfdata;
> @@ -7971,7 +7978,9 @@ static int rtl8xxxu_probe(struct usb_interface *interface,
>  err_set_intfdata:
>  	usb_set_intfdata(interface, NULL);
>  
> +	kfree(priv->usb_buf);
>  	kfree(priv->fw_data);
> +
>  	mutex_destroy(&priv->usb_buf_mutex);
>  	mutex_destroy(&priv->syson_indirect_access_mutex);
>  	mutex_destroy(&priv->h2c_mutex);
> @@ -7999,7 +8008,9 @@ static void rtl8xxxu_disconnect(struct usb_interface *interface)
>  
>  	dev_info(&priv->udev->dev, "disconnecting\n");
>  
> +	kfree(priv->usb_buf);
>  	kfree(priv->fw_data);
> +
>  	mutex_destroy(&priv->usb_buf_mutex);
>  	mutex_destroy(&priv->syson_indirect_access_mutex);
>  	mutex_destroy(&priv->h2c_mutex);
> diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> index c6953051d..8cbadcf71 100644
> --- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> +++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu.h
> @@ -1891,7 +1891,7 @@ struct rtl8xxxu_priv {
>  		__le32 val32;
>  		__le16 val16;
>  		u8 val8;
> -	} usb_buf;
> +	} *usb_buf;
>  	union {
>  		u8 raw[EFUSE_MAP_LEN];
>  		struct rtl8723au_efuse efuse8723;
>