Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <3d022dad-99b2-0b4e-adab-4ac5147c989c@quicinc.com>
Date:   Wed, 9 Aug 2023 10:32:35 -0700
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v2] wifi: ath12k: add check max message length while
 scanning with extraie
Content-Language: en-US
To:     Wen Gong <quic_wgong@quicinc.com>, <ath12k@lists.infradead.org>
CC:     <linux-wireless@vger.kernel.org>
References: <20230809081657.13858-1-quic_wgong@quicinc.com>
From:   Jeff Johnson <quic_jjohnson@quicinc.com>
In-Reply-To: <20230809081657.13858-1-quic_wgong@quicinc.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 8/9/2023 1:16 AM, Wen Gong wrote:
> Currently the extraie length is directly used to allocate skb buffer. When
> the length of skb is greater than the max message length which firmware
> supports, error will happen in firmware side.
> 
> Hence add check for the skb length and drop extraie when overflow and
> print a message.
> 
> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0-03427-QCAHMTSWPL_V1.0_V2.0_SILICONZ-1.15378.4
> 
> Signed-off-by: Wen Gong <quic_wgong@quicinc.com>

Reviewed-by: Jeff Johnson <quic_jjohnson@quicinc.com>

> ---
> v2: seperate to another patch per johannes.
> 
>   drivers/net/wireless/ath/ath12k/wmi.c | 20 +++++++++++++-------
>   1 file changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath12k/wmi.c b/drivers/net/wireless/ath/ath12k/wmi.c
> index 9ed33e2d6da0..e964d6003ea9 100644
> --- a/drivers/net/wireless/ath/ath12k/wmi.c
> +++ b/drivers/net/wireless/ath/ath12k/wmi.c
> @@ -2240,12 +2240,6 @@ int ath12k_wmi_send_scan_start_cmd(struct ath12k *ar,
>   	if (arg->num_bssid)
>   		len += sizeof(*bssid) * arg->num_bssid;
>   
> -	len += TLV_HDR_SIZE;
> -	if (arg->extraie.len)
> -		extraie_len_with_pad =
> -			roundup(arg->extraie.len, sizeof(u32));
> -	len += extraie_len_with_pad;
> -
>   	if (arg->num_hint_bssid)
>   		len += TLV_HDR_SIZE +
>   		       arg->num_hint_bssid * sizeof(*hint_bssid);
> @@ -2254,6 +2248,18 @@ int ath12k_wmi_send_scan_start_cmd(struct ath12k *ar,
>   		len += TLV_HDR_SIZE +
>   		       arg->num_hint_s_ssid * sizeof(*s_ssid);
>   
> +	len += TLV_HDR_SIZE;
> +	if (arg->extraie.len)
> +		extraie_len_with_pad =
> +			roundup(arg->extraie.len, sizeof(u32));
> +	if (extraie_len_with_pad <= (wmi->wmi_ab->max_msg_len[ar->pdev_idx] - len)) {
> +		len += extraie_len_with_pad;
> +	} else {
> +		ath12k_warn(ar->ab, "discard large size %d bytes extraie for scan start\n",
> +			    arg->extraie.len);
> +		extraie_len_with_pad = 0;
> +	}
> +
>   	skb = ath12k_wmi_alloc_skb(wmi->wmi_ab, len);
>   	if (!skb)
>   		return -ENOMEM;
> @@ -2343,7 +2349,7 @@ int ath12k_wmi_send_scan_start_cmd(struct ath12k *ar,
>   	tlv->header = ath12k_wmi_tlv_hdr(WMI_TAG_ARRAY_BYTE, len);
>   	ptr += TLV_HDR_SIZE;
>   
> -	if (arg->extraie.len)
> +	if (extraie_len_with_pad)
>   		memcpy(ptr, arg->extraie.ptr,
>   		       arg->extraie.len);
>   
> 
> base-commit: 3f257461ab0ab19806bae2bfde4c3cd88dbf050e