Re: [PATCH ath-next 05/16] wifi: ath11k: Add ability to set BDF and M3 dump memory addresses

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] wifi: ath11k: CALDB memory mapping corrupted by shared 'res' variable clobbering
- [High] wifi: ath11k: BDF and HOST_DDR memory regions overlap at the same physical address
--

commit 12078c78e9f034dda1106d3aea7be9725dcdf38d
Author: George Moussalem <george.moussalem@outlook.com>

wifi: ath11k: Add ability to set BDF and M3 dump memory addresses

This commit adds the ability to look up reserved memory regions from the device
tree and use those addresses for the BDF and M3 dump instead of relying on
hardcoded hardware parameter values.

> diff --git a/drivers/net/wireless/ath/ath11k/qmi.c b/drivers/net/wireless/ath/ath11k/qmi.c
> index bd81e8d3d5f7d..771ef9b5304f2 100644
> --- a/drivers/net/wireless/ath/ath11k/qmi.c
> +++ b/drivers/net/wireless/ath/ath11k/qmi.c
[ ... ]
> @@ -2073,7 +2112,11 @@ static int ath11k_qmi_assign_target_mem_chunk(struct ath11k_base *ab)
>  			idx++;
>  			break;
>  		case BDF_MEM_REGION_TYPE:
> -			ab->qmi.target_mem[idx].paddr = ab->hw_params.bdf_addr;
> +			ret = ath11k_qmi_reserved_memory_to_resource(ab, BDF_MEM_REGION_TYPE, &res);

[Severity: High]
Does overwriting the shared 'res' variable here (and later in the
M3_DUMP_REGION_TYPE case) corrupt the mapping for CALDB_MEM_REGION_TYPE?

If the firmware requests M3_DUMP_REGION_TYPE or BDF_MEM_REGION_TYPE before
CALDB_MEM_REGION_TYPE, 'res' will be overwritten. When CALDB_MEM_REGION_TYPE
is subsequently processed in ath11k_qmi_assign_target_mem_chunk(), it
calculates its physical address using the shared 'res' variable:

    ab->qmi.target_mem[idx].paddr = res.start + host_ddr_sz;

This would incorrectly use the clobbered region's base address (such as the
m3-dump address) instead of the HOST_DDR base address, mapping CALDB to an
invalid physical address.

> +			if (!ret && !(ab->qmi.target_mem[i].size > resource_size(&res)))
> +				ab->qmi.target_mem[idx].paddr = res.start;

[Severity: High]
Does this cause the BDF memory region to overlap exactly with the HOST_DDR
memory region?

Both HOST_DDR_REGION_TYPE and BDF_MEM_REGION_TYPE are mapped to the "q6-region"
memory space. By assigning res.start directly without any offset, the BDF
memory region would start at the exact same physical address as the HOST_DDR
chunk.

Previous hardware applied a hardcoded offset for the BDF address to separate
it from the HOST_DDR base, which is intended for firmware code and data.

> +			else
> +				ab->qmi.target_mem[idx].paddr = ab->hw_params.bdf_addr;
>  			ab->qmi.target_mem[idx].iaddr = NULL;
>  			ab->qmi.target_mem[idx].size = ab->qmi.target_mem[i].size;
>  			ab->qmi.target_mem[idx].type = ab->qmi.target_mem[i].type;
[ ... ]
> @@ -2107,6 +2150,16 @@ static int ath11k_qmi_assign_target_mem_chunk(struct ath11k_base *ab)
>  			ab->qmi.target_mem[idx].type = ab->qmi.target_mem[i].type;
>  			idx++;
>  			break;
> +		case M3_DUMP_REGION_TYPE:
> +			ret = ath11k_qmi_reserved_memory_to_resource(ab, M3_DUMP_REGION_TYPE, &res);

[Severity: High]
Could this also clobber the shared 'res' variable in the same way as the
BDF_MEM_REGION_TYPE case, causing the identical memory corruption issue for
CALDB_MEM_REGION_TYPE?

> +			if (!ret && !(ab->qmi.target_mem[i].size > resource_size(&res))) {
> +				ab->qmi.target_mem[idx].paddr = res.start;
> +				ab->qmi.target_mem[idx].iaddr = NULL;
> +				ab->qmi.target_mem[idx].size = ab->qmi.target_mem[i].size;
> +				ab->qmi.target_mem[idx].type = ab->qmi.target_mem[i].type;
> +				idx++;
> +			}
> +			break;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260923-ipq5018-qcn6122-mpd-v1-0-86729521a75c@outlook.com?part=5