| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <b947e130-707c-45f2-aba9-de9c12b73be2@oss.qualcomm.com>
Date: Mon, 19 Jan 2026 16:48:49 +0800
From: Baochen Qiang <baochen.qiang@....qualcomm.com>
To: Alexander Wilhelm <alexander.wilhelm@...termo.com>,
Jeff Johnson <jjohnson@...nel.org>
Cc: ath12k@...ts.infradead.org, linux-wireless@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] wifi: ath12k: fix endianness handling for SRNG ring
pointer accesses
On 1/19/2026 3:39 PM, Alexander Wilhelm wrote:
> The SRNG head and tail ring pointers are stored in device memory as
> little-endian values. On big-endian systems, direct dereferencing of these
> pointers leads to incorrect values being read or written, causing ring
> management issues and potentially breaking data flow.
>
> This patch ensures all accesses to SRNG ring pointers use the appropriate
> endianness conversions. This affects both read and write paths for source
> and destination rings, as well as debug output. The changes guarantee
> correct operation on both little- and big-endian architectures.
>
> Signed-off-by: Alexander Wilhelm <alexander.wilhelm@...termo.com>
> ---
> Changes in v3:
> - Rebase on latest 'ath' master
> - Use always 'le32_to_cpu()' macro for conversions
>
> Changes in v2:
> - Set '__le32 *' type for 'hp_addr/tp_addr' in both 'dst_ring' and 'src_ring'
> ---
> drivers/net/wireless/ath/ath12k/hal.c | 37 +++++++++++++++------------
> drivers/net/wireless/ath/ath12k/hal.h | 8 +++---
> 2 files changed, 25 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath12k/hal.c b/drivers/net/wireless/ath/ath12k/hal.c
> index a164563fff28..28d0ff524fc6 100644
> --- a/drivers/net/wireless/ath/ath12k/hal.c
> +++ b/drivers/net/wireless/ath/ath12k/hal.c
> @@ -355,7 +355,7 @@ int ath12k_hal_srng_dst_num_free(struct ath12k_base *ab, struct hal_srng *srng,
> tp = srng->u.dst_ring.tp;
>
> if (sync_hw_ptr) {
> - hp = *srng->u.dst_ring.hp_addr;
> + hp = le32_to_cpu(*srng->u.dst_ring.hp_addr);
> srng->u.dst_ring.cached_hp = hp;
> } else {
> hp = srng->u.dst_ring.cached_hp;
> @@ -379,7 +379,7 @@ int ath12k_hal_srng_src_num_free(struct ath12k_base *ab, struct hal_srng *srng,
> hp = srng->u.src_ring.hp;
>
> if (sync_hw_ptr) {
> - tp = *srng->u.src_ring.tp_addr;
> + tp = le32_to_cpu(*srng->u.src_ring.tp_addr);
> srng->u.src_ring.cached_tp = tp;
> } else {
> tp = srng->u.src_ring.cached_tp;
> @@ -500,10 +500,10 @@ void ath12k_hal_srng_access_begin(struct ath12k_base *ab, struct hal_srng *srng)
> lockdep_assert_held(&srng->lock);
>
> if (srng->ring_dir == HAL_SRNG_DIR_SRC) {
> - srng->u.src_ring.cached_tp =
> - *(volatile u32 *)srng->u.src_ring.tp_addr;
> + srng->u.src_ring.cached_tp = le32_to_cpu(
> + *(volatile __le32 *)srng->u.src_ring.tp_addr);
> } else {
> - hp = READ_ONCE(*srng->u.dst_ring.hp_addr);
> + hp = le32_to_cpu(READ_ONCE(*srng->u.dst_ring.hp_addr));
>
> if (hp != srng->u.dst_ring.cached_hp) {
> srng->u.dst_ring.cached_hp = hp;
> @@ -528,25 +528,28 @@ void ath12k_hal_srng_access_end(struct ath12k_base *ab, struct hal_srng *srng)
> * hence written to a shared memory location that is read by FW
> */
> if (srng->ring_dir == HAL_SRNG_DIR_SRC) {
> - srng->u.src_ring.last_tp =
> - *(volatile u32 *)srng->u.src_ring.tp_addr;
> + srng->u.src_ring.last_tp = le32_to_cpu(
> + *(volatile __le32 *)srng->u.src_ring.tp_addr);
> /* Make sure descriptor is written before updating the
> * head pointer.
> */
> dma_wmb();
> - WRITE_ONCE(*srng->u.src_ring.hp_addr, srng->u.src_ring.hp);
> + WRITE_ONCE(*srng->u.src_ring.hp_addr,
> + cpu_to_le32(srng->u.src_ring.hp));
> } else {
> - srng->u.dst_ring.last_hp = *srng->u.dst_ring.hp_addr;
> + srng->u.dst_ring.last_hp =
> + le32_to_cpu(*srng->u.dst_ring.hp_addr);
> /* Make sure descriptor is read before updating the
> * tail pointer.
> */
> dma_mb();
> - WRITE_ONCE(*srng->u.dst_ring.tp_addr, srng->u.dst_ring.tp);
> + WRITE_ONCE(*srng->u.dst_ring.tp_addr,
> + cpu_to_le32(srng->u.dst_ring.tp));
> }
> } else {
> if (srng->ring_dir == HAL_SRNG_DIR_SRC) {
> - srng->u.src_ring.last_tp =
> - *(volatile u32 *)srng->u.src_ring.tp_addr;
> + srng->u.src_ring.last_tp = le32_to_cpu(
> + *(volatile __le32 *)srng->u.src_ring.tp_addr);
> /* Assume implementation use an MMIO write accessor
> * which has the required wmb() so that the descriptor
> * is written before the updating the head pointer.
> @@ -556,7 +559,8 @@ void ath12k_hal_srng_access_end(struct ath12k_base *ab, struct hal_srng *srng)
> (unsigned long)ab->mem,
> srng->u.src_ring.hp);
> } else {
> - srng->u.dst_ring.last_hp = *srng->u.dst_ring.hp_addr;
> + srng->u.dst_ring.last_hp =
> + le32_to_cpu(*srng->u.dst_ring.hp_addr);
> /* Make sure descriptor is read before updating the
> * tail pointer.
> */
> @@ -711,7 +715,7 @@ void ath12k_hal_srng_shadow_update_hp_tp(struct ath12k_base *ab,
> * HP only when then ring isn't' empty.
> */
> if (srng->ring_dir == HAL_SRNG_DIR_SRC &&
> - *srng->u.src_ring.tp_addr != srng->u.src_ring.hp)
> + le32_to_cpu(*srng->u.src_ring.tp_addr) != srng->u.src_ring.hp)
> ath12k_hal_srng_access_end(ab, srng);
> }
>
> @@ -810,14 +814,15 @@ void ath12k_hal_dump_srng_stats(struct ath12k_base *ab)
> "src srng id %u hp %u, reap_hp %u, cur tp %u, cached tp %u last tp %u napi processed before %ums\n",
> srng->ring_id, srng->u.src_ring.hp,
> srng->u.src_ring.reap_hp,
> - *srng->u.src_ring.tp_addr, srng->u.src_ring.cached_tp,
> + le32_to_cpu(*srng->u.src_ring.tp_addr),
> + srng->u.src_ring.cached_tp,
> srng->u.src_ring.last_tp,
> jiffies_to_msecs(jiffies - srng->timestamp));
> else if (srng->ring_dir == HAL_SRNG_DIR_DST)
> ath12k_err(ab,
> "dst srng id %u tp %u, cur hp %u, cached hp %u last hp %u napi processed before %ums\n",
> srng->ring_id, srng->u.dst_ring.tp,
> - *srng->u.dst_ring.hp_addr,
> + le32_to_cpu(*srng->u.dst_ring.hp_addr),
> srng->u.dst_ring.cached_hp,
> srng->u.dst_ring.last_hp,
> jiffies_to_msecs(jiffies - srng->timestamp));
> diff --git a/drivers/net/wireless/ath/ath12k/hal.h b/drivers/net/wireless/ath/ath12k/hal.h
> index 43e3880f8257..ff6148be94c8 100644
> --- a/drivers/net/wireless/ath/ath12k/hal.h
> +++ b/drivers/net/wireless/ath/ath12k/hal.h
> @@ -914,7 +914,7 @@ struct hal_srng {
> u32 tp;
>
> /* Shadow head pointer location to be updated by HW */
> - volatile u32 *hp_addr;
> + volatile __le32 *hp_addr;
>
> /* Cached head pointer */
> u32 cached_hp;
> @@ -923,7 +923,7 @@ struct hal_srng {
> * will be a register address and need not be
> * accessed through SW structure
> */
> - u32 *tp_addr;
> + __le32 *tp_addr;
>
> /* Current SW loop cnt */
> u32 loop_cnt;
> @@ -943,7 +943,7 @@ struct hal_srng {
> u32 reap_hp;
>
> /* Shadow tail pointer location to be updated by HW */
> - u32 *tp_addr;
> + __le32 *tp_addr;
>
> /* Cached tail pointer */
> u32 cached_tp;
> @@ -952,7 +952,7 @@ struct hal_srng {
> * will be a register address and need not be accessed
> * through SW structure
> */
> - u32 *hp_addr;
> + __le32 *hp_addr;
>
> /* Low threshold - in number of ring entries */
> u32 low_threshold;
>
> base-commit: 758064145fe77e06d07661b27dfa9c24fe0309a3
Reviewed-by: Baochen Qiang <baochen.qiang@....qualcomm.com>
Powered by blists - more mailing lists