Re: [PATCH ath-next v2 13/17] wifi: ath12k: use CMEM write for PPT on hybrid bus devices

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] wifi: ath12k: OOB array read in ath12k_dp_cmem_init() due to mixed MLO mem modes
--

commit b4813e2e272b2801865377c6c50ae123db10e5d7
Author: Aaradhana Sahu <aaradhana.sahu@oss.qualcomm.com>

wifi: ath12k: use CMEM write for PPT on hybrid bus devices

This commit updates the Primary Page Table (PPT) initialization to use
CMEM-specific write operations on hybrid bus devices. It also propagates
errors from these CMEM writes during PPT and partner copy cache
initialization, aborting MLO setup if a failure occurs.

> diff --git a/drivers/net/wireless/ath/ath12k/dp.c b/drivers/net/wireless/ath/ath12k/dp.c
> index 83f9edfabf696..6cf254b2d2a78 100644
> --- a/drivers/net/wireless/ath/ath12k/dp.c
> +++ b/drivers/net/wireless/ath/ath12k/dp.c
[ ... ]
> @@ -1328,24 +1328,38 @@ static int ath12k_dp_cmem_init(struct ath12k_base *ab,
>  	}
>  
>  	/* Write to PPT in CMEM */
> -	for (i = start; i < end; i++)
> -		ath12k_hif_write32(ab, cmem_base + ATH12K_PPT_ADDR_OFFSET(i),
> -				   dp->spt_info[i].paddr >> ATH12K_SPT_4K_ALIGN_OFFSET);
> +	for (i = start; i < end; i++) {
> +		addr = cmem_base + ATH12K_PPT_ADDR_OFFSET(i);
> +		val = dp->spt_info[i].paddr >> ATH12K_SPT_4K_ALIGN_OFFSET;

[Severity: High]
Can this result in an out-of-bounds array read on hybrid bus MLO devices?

The loop limits start and end are derived from the current device's
profile (ab->profile_param->dp_params). However, the array being indexed,
dp->spt_info[i], belongs to the partner device.

On systems with mixed-bus MLO configurations (like an AHB radio and a PCI
radio in the same MLO group) and constrained memory, the devices might have
different memory profiles. For example, if the AHB device has the DEFAULT
profile but the PCI partner is using the LOW_512_M profile, the AHB device
will calculate a larger end value than the size of the partner's allocated
spt_info array.

Does this mean the loop will read past the end of the spt_info array and
write garbage memory values to the device's CMEM Primary Page Table?

> +
> +		if (ab->hw_params->hybrid_bus_type) {
> +			ret = ath12k_hif_cmem_write32(ab, addr, val);
> +			if (ret)
> +				return ret;
> +		} else {
> +			ath12k_hif_write32(ab, addr, val);
> +		}
> +	}
>  
>  	return 0;
>  }

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