Re: [PATCH ath-next v2 08/17] wifi: ath12k: Add hif operation support for QCN6432

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] net: ath12k: unconfigured dynamic window in pcic read32/write32 causes out-of-bounds MMIO
- [High] wifi: ath12k: use-after-free in timer callback ath12k_ce_rx_replenish_retry()
--

commit d724e1834e7dbd2c47e113791169e389fe945179
Author: Sowmiya Sree Elavalagan <quic_ssreeela@quicinc.com>
wifi: ath12k: Add hif operation support for QCN6432

This commit adds HIF operations for the QCN6432 Wi-Fi driver by reusing
existing methods from IPQ5332 and QCN9274 where applicable. It implements
QCN6432-specific operations in a new pcic.c file.

> diff --git a/drivers/net/wireless/ath/ath12k/pcic.c b/drivers/net/wireless/ath/ath12k/pcic.c
> new file mode 100644
> index 0000000000000..e3c8476af459c
> --- /dev/null
> +++ b/drivers/net/wireless/ath/ath12k/pcic.c

[ ... ]

> +void ath12k_pcic_stop(struct ath12k_base *ab)
> +{
> +	ath12k_pci_ce_irq_disable_sync(ab);
> +	timer_delete_sync(&ab->rx_replenish_retry);
> +	ath12k_ce_cleanup_pipes(ab);
> +}

[Severity: High]
Is there a mechanism preventing ab->rx_replenish_retry from re-arming itself
here?

If ath12k_pcic_stop() executes while the timer callback
ath12k_ce_rx_replenish_retry() is running concurrently, timer_delete_sync()
will wait. However, the callback invokes ath12k_ce_rx_post_buf(), which blindly
re-arms the timer on allocation failures (-ENOMEM):

drivers/net/wireless/ath/ath12k/ce.c:ath12k_ce_rx_post_buf() {
    ...
    ret = ath12k_ce_rx_post_pipe(pipe);
    if (ret) {
        if (ret == -ENOSPC)
            continue;
        ...
        mod_timer(&ab->rx_replenish_retry, ...);
        return;
    }
}

Could this cause the timer to be re-enqueued after timer_delete_sync()
unblocks, leading to a use-after-free regression when the timer fires after the
structures are freed?

[ ... ]

> +u32 ath12k_pcic_read32(struct ath12k_base *ab, u32 offset)
> +{
> +	u32 window_start;
> +
> +	if (offset < WINDOW_START)
> +		return readl_relaxed(ab->mem + offset);
> +
> +	window_start = ath12k_pcic_get_window_start(ab, offset);
> +	return readl_relaxed(ab->mem + window_start + (offset & WINDOW_RANGE_MASK));
> +}

[Severity: Critical]
Does this read from an unconfigured dynamic window?

When offset does not fall into the statically mapped UMAC or CE ranges,
ath12k_pcic_get_window_start() returns the fallback dynamic window address
(WINDOW_START, which is Window 1).

However, unlike the cmem accessors or the PCI equivalent (ath12k_pci_read32)
which explicitly lock and program Window 1 to the high bits of the target
offset, this function skips configuration entirely and blindly performs the
read.

Could this result in out-of-bounds MMIO reads, directing the read to whatever
physical memory address the dynamic window was previously configured to map?

> +
> +void ath12k_pcic_write32(struct ath12k_base *ab, u32 offset, u32 value)
> +{
> +	u32 window_start;
> +
> +	if (offset < WINDOW_START) {
> +		writel_relaxed(value, ab->mem + offset);
> +		return;
> +	}
> +
> +	window_start = ath12k_pcic_get_window_start(ab, offset);
> +	writel_relaxed(value, ab->mem + window_start + (offset & WINDOW_RANGE_MASK));
> +}

[Severity: Critical]
Similarly, does this write to the dynamic window without first programming the
hardware register to map the target offset?

Like the read path above, ath12k_pcic_write32() uses the dynamic window without
calling a window selection function such as ath12k_pcic_select_static_window().

Will this regression corrupt memory or hardware state by writing to a previously
configured but currently unrelated memory address?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016-mpp-bringup-v2-0-f510c33bfcf5@oss.qualcomm.com?part=8