Re: [PATCH ath-next 08/15] wifi: ath12k: add Wi-Fi 8 HAL ops and descriptor definitions for QCN9575

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] wifi: ath12k: undersized srng_config in
  ath12k_hal_srng_create_config_qcn9575()
--

commit a0803bb7ba2b3bd95c6970f62088631be5af1b5d
Author: Madhavan Ganesan <madhavan.ganesan@oss.qualcomm.com>

wifi: ath12k: add Wi-Fi 8 HAL ops and descriptor definitions for QCN9575

This commit adds Wi-Fi 8 HAL operation callbacks, CE descriptor
definitions, and wires hal_qcn9575_ops into the QCN9575 hardware-version
map.

> diff --git a/drivers/net/wireless/ath/ath12k/wifi8/hal_qcn9575.c b/drivers/net/wireless/ath/ath12k/wifi8/hal_qcn9575.c
> index 56d45785ecd44..076a5a609f8b6 100644
> --- a/drivers/net/wireless/ath/ath12k/wifi8/hal_qcn9575.c
> +++ b/drivers/net/wireless/ath/ath12k/wifi8/hal_qcn9575.c
> @@ -2,8 +2,37 @@
>  /*
>   * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
>   */
> +#include "hal.h"
> +#include "hal_desc.h"
>  #include "hal_qcn9575.h"
>  
> +static const struct hal_srng_config hw_srng_config_template[] = {
> +	[HAL_CE_SRC] = {
> +		.start_ring_id = HAL_SRNG_RING_ID_CE0_SRC,
> +		.max_rings = 16,
> +		.entry_size = sizeof(struct hal_ce_srng_src_desc) >> 2,
> +		.mac_type = ATH12K_HAL_SRNG_UMAC,
> +		.ring_dir = HAL_SRNG_DIR_SRC,
> +		.max_size = HAL_CE_SRC_RING_BASE_MSB_RING_SIZE,
> +	},
> +	[HAL_CE_DST] = {
> +		.start_ring_id = HAL_SRNG_RING_ID_CE0_DST,
> +		.max_rings = 16,
> +		.entry_size = sizeof(struct hal_ce_srng_dest_desc) >> 2,
> +		.mac_type = ATH12K_HAL_SRNG_UMAC,
> +		.ring_dir = HAL_SRNG_DIR_SRC,
> +		.max_size = HAL_CE_DST_RING_BASE_MSB_RING_SIZE,
> +	},
> +	[HAL_CE_DST_STATUS] = {
> +		.start_ring_id = HAL_SRNG_RING_ID_CE0_DST_STATUS,
> +		.max_rings = 16,
> +		.entry_size = sizeof(struct hal_ce_srng_dst_status_desc) >> 2,
> +		.mac_type = ATH12K_HAL_SRNG_UMAC,
> +		.ring_dir = HAL_SRNG_DIR_DST,
> +		.max_size = HAL_CE_DST_STATUS_RING_BASE_MSB_RING_SIZE,
> +	},
> +};
> +
>  const struct ath12k_hw_regs qcn9575_regs = {
>  	/* SW2TCL(x) R0 ring configuration address */
>  	.tcl1_ring_id = 0x00001604,
> @@ -40,3 +69,55 @@ const struct ath12k_hw_regs qcn9575_regs = {
>  
>  	.qrtr_node_id = 0x01e03300,
>  };
> +
> +static int ath12k_hal_srng_create_config_qcn9575(struct ath12k_hal *hal)
> +{
> +	struct hal_srng_config *s;
> +
> +	hal->srng_config = kmemdup(hw_srng_config_template,
> +				   sizeof(hw_srng_config_template),
> +				   GFP_KERNEL);

[Severity: High]
Does sizing hw_srng_config_template[] implicitly cause out-of-bounds reads
on hal->srng_config?

Because the highest designated initializer is [HAL_CE_DST_STATUS] (10),
hw_srng_config_template[] has only 11 entries, and kmemdup() allocates an
11-entry buffer for hal->srng_config.  Common HAL code such as
ath12k_hal_srng_shadow_config() iterates up to HAL_MAX_RING_TYPES (25):

drivers/net/wireless/ath/ath12k/hal.c:ath12k_hal_srng_shadow_config() {
	for (ring_type = 0; ring_type < HAL_MAX_RING_TYPES; ring_type++) {
		struct hal_srng_config *srng_config = &hal->srng_config[ring_type];
    ...
}

This reads past the end of the kmemdup() allocation for ring types above
HAL_CE_DST_STATUS.  Declaring
hw_srng_config_template[HAL_MAX_RING_TYPES] would ensure all ring types
are allocated and zero-initialized.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112-wifi8_qcn9575_bringup-v1-0-3f30fbc2bb69@oss.qualcomm.com?part=8