[lvc-project] [PATCH 1/3] [v2] wifi: ath11k: use ath11k_mac_get_ar_by_pdev_id() consistently

[Jeff Johnson]

On 2/29/2024 12:40 AM, Dmitry Antipov wrote:
> Since 'ath11k_mac_get_ar_by_pdev_id()' can return NULL, check
> the return value in 'ath11k_wmi_tlv_rssi_chain_parse()' as well
> as in 'ath11k_wmi_tlv_fw_stats_data_parse()', and return -EINVAL
> in case of error. Compile tested only.
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Signed-off-by: Dmitry Antipov <dmantipov at yandex.ru>
> ---
> v2: aggregate to the series

In the future please don't use In-Reply-To: when submitting a new
version of a patch/series. Each version should start its own thread.
Having In-Reference-To: is OK

> ---
>  drivers/net/wireless/ath/ath11k/wmi.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/net/wireless/ath/ath11k/wmi.c b/drivers/net/wireless/ath/ath11k/wmi.c
> index 34ab9631ff36..2d93e4e78a37 100644
> --- a/drivers/net/wireless/ath/ath11k/wmi.c
> +++ b/drivers/net/wireless/ath/ath11k/wmi.c
> @@ -6498,6 +6498,12 @@ static int ath11k_wmi_tlv_rssi_chain_parse(struct ath11k_base *ab,
>  	rcu_read_lock();
>  
>  	ar = ath11k_mac_get_ar_by_pdev_id(ab, ev->pdev_id);
> +	if (!ar) {
> +		ath11k_warn(ab, "%s: invalid pdev_id %d\n",
> +			    __func__, ev->pdev_id);

it is currently not the convention to use __func__ in ath11k logs.

there are currently 6 instances where the NULL check is present, and
each has a custom log message

I personally would be ok having this new format, but want to get Kalle's
approval for that. But if we do that I'd also want a follow-up patch to
modify those 6 existing logs.

FWIW in all of ath11k there are only 3 instances of __func__:
mac.c:                     "%s: rsn ie found\n", __func__);
mac.c:                     "%s: wpa ie found\n", __func__);
wmi.c:                     __func__);

> +		ret = -EINVAL;
> +		goto exit;
> +	}
>  	stats->stats_id = WMI_REQUEST_RSSI_PER_CHAIN_STAT;
>  
>  	ath11k_dbg(ab, ATH11K_DBG_WMI,
> @@ -6570,6 +6576,12 @@ static int ath11k_wmi_tlv_fw_stats_data_parse(struct ath11k_base *ab,
>  	rcu_read_lock();
>  
>  	ar = ath11k_mac_get_ar_by_pdev_id(ab, ev->pdev_id);
> +	if (!ar) {
> +		ath11k_warn(ab, "%s: invalid pdev_id %d\n",
> +			    __func__, ev->pdev_id);
> +		ret = -EINVAL;
> +		goto exit;
> +	}
>  
>  	for (i = 0; i < ev->num_pdev_stats; i++) {
>  		const struct wmi_pdev_stats *src;

The actual logic is correct.

/jeff