[PATCH] wifi: ath12k: fix ring-buffer corruption
Miaoqing Pan
quic_miaoqing at quicinc.com
Tue May 6 17:17:38 PDT 2025
On 3/21/2025 5:52 PM, Johan Hovold wrote:
> Users of the Lenovo ThinkPad X13s have reported that Wi-Fi sometimes
> breaks and the log fills up with errors like:
>
> ath11k_pci 0006:01:00.0: HTC Rx: insufficient length, got 1484, expected 1492
> ath11k_pci 0006:01:00.0: HTC Rx: insufficient length, got 1460, expected 1484
>
> which based on a quick look at the ath11k driver seemed to indicate some
> kind of ring-buffer corruption.
>
> Miaoqing Pan tracked it down to the host seeing the updated destination
> ring head pointer before the updated descriptor, and the error handling
> for that in turn leaves the ring buffer in an inconsistent state.
>
> While this has not yet been observed with ath12k, the ring-buffer
> implementation is very similar to the ath11k one and it suffers from the
> same bugs.
>
> Add the missing memory barrier to make sure that the descriptor is read
> after the head pointer to address the root cause of the corruption while
> fixing up the error handling in case there are ever any (ordering) bugs
> on the device side.
>
> Note that the READ_ONCE() are only needed to avoid compiler mischief in
> case the ring-buffer helpers are ever inlined.
>
> Tested-on: WCN7850 hw2.0 WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
>
> Fixes: d889913205cf ("wifi: ath12k: driver for Qualcomm Wi-Fi 7 devices")
> Cc: stable at vger.kernel.org # 6.3
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=218623
> Link: https://lore.kernel.org/20250310010217.3845141-3-quic_miaoqing@quicinc.com
> Cc: Miaoqing Pan <quic_miaoqing at quicinc.com>
> Signed-off-by: Johan Hovold <johan+linaro at kernel.org>
> ---
> drivers/net/wireless/ath/ath12k/ce.c | 11 +++++------
> drivers/net/wireless/ath/ath12k/hal.c | 4 ++--
> 2 files changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath12k/ce.c b/drivers/net/wireless/ath/ath12k/ce.c
> index be0d669d31fc..740586fe49d1 100644
> --- a/drivers/net/wireless/ath/ath12k/ce.c
> +++ b/drivers/net/wireless/ath/ath12k/ce.c
> @@ -343,11 +343,10 @@ static int ath12k_ce_completed_recv_next(struct ath12k_ce_pipe *pipe,
> goto err;
> }
>
> + /* Make sure descriptor is read after the head pointer. */
> + dma_rmb();
> +
> *nbytes = ath12k_hal_ce_dst_status_get_length(desc);
> - if (*nbytes == 0) {
> - ret = -EIO;
> - goto err;
> - }
>
> *skb = pipe->dest_ring->skb[sw_index];
> pipe->dest_ring->skb[sw_index] = NULL;
> @@ -380,8 +379,8 @@ static void ath12k_ce_recv_process_cb(struct ath12k_ce_pipe *pipe)
> dma_unmap_single(ab->dev, ATH12K_SKB_RXCB(skb)->paddr,
> max_nbytes, DMA_FROM_DEVICE);
>
> - if (unlikely(max_nbytes < nbytes)) {
> - ath12k_warn(ab, "rxed more than expected (nbytes %d, max %d)",
> + if (unlikely(max_nbytes < nbytes || nbytes == 0)) {
> + ath12k_warn(ab, "unexpected rx length (nbytes %d, max %d)",
> nbytes, max_nbytes);
> dev_kfree_skb_any(skb);
> continue;
> diff --git a/drivers/net/wireless/ath/ath12k/hal.c b/drivers/net/wireless/ath/ath12k/hal.c
> index cd59ff8e6c7b..91d5126ca149 100644
> --- a/drivers/net/wireless/ath/ath12k/hal.c
> +++ b/drivers/net/wireless/ath/ath12k/hal.c
> @@ -1962,7 +1962,7 @@ u32 ath12k_hal_ce_dst_status_get_length(struct hal_ce_srng_dst_status_desc *desc
> {
> u32 len;
>
> - len = le32_get_bits(desc->flags, HAL_CE_DST_STATUS_DESC_FLAGS_LEN);
> + len = le32_get_bits(READ_ONCE(desc->flags), HAL_CE_DST_STATUS_DESC_FLAGS_LEN);
> desc->flags &= ~cpu_to_le32(HAL_CE_DST_STATUS_DESC_FLAGS_LEN);
>
> return len;
> @@ -2132,7 +2132,7 @@ void ath12k_hal_srng_access_begin(struct ath12k_base *ab, struct hal_srng *srng)
> srng->u.src_ring.cached_tp =
> *(volatile u32 *)srng->u.src_ring.tp_addr;
> else
> - srng->u.dst_ring.cached_hp = *srng->u.dst_ring.hp_addr;
> + srng->u.dst_ring.cached_hp = READ_ONCE(*srng->u.dst_ring.hp_addr);
> }
>
> /* Update cached ring head/tail pointers to HW. ath12k_hal_srng_access_begin()
Reviewed-by: Miaoqing Pan <quic_miaoqing at quicinc.com>
More information about the ath12k
mailing list