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;
From:   Minsuk Kang <linuxlovemin@yonsei.ac.kr>
To:     arend.vanspriel@broadcom.com
Cc:     linux-wireless@vger.kernel.org, kvalo@kernel.org,
        dokyungs@yonsei.ac.kr, jisoo.jang@yonsei.ac.kr,
        Minsuk Kang <linuxlovemin@yonsei.ac.kr>
Subject: Re: [PATCH v2] wifi: brcmfmac: Check the count value of channel spec to prevent out-of-bounds reads
Date:   Tue, 29 Nov 2022 23:56:33 +0900
Message-Id: <20221129145632.1072379-1-linuxlovemin@yonsei.ac.kr>
In-Reply-To: <20221116142952.518241-1-linuxlovemin@yonsei.ac.kr>
References: <20221116142952.518241-1-linuxlovemin@yonsei.ac.kr>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

Minsuk Kang <linuxlovemin@yonsei.ac.kr> wrote:

> v1->v2: Added a macro BRCMF_MAX_CHANSPEC_LIST
> 
>  .../broadcom/brcm80211/brcmfmac/cfg80211.c      | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> index ae9507dec74a..bff3128c2f26 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> @@ -101,6 +101,9 @@
>  #define BRCMF_ASSOC_PARAMS_FIXED_SIZE \
>  	(sizeof(struct brcmf_assoc_params_le) - sizeof(u16))
>  
> +#define BRCMF_MAX_CHANSPEC_LIST \
> +	(BRCMF_DCMD_MEDLEN / sizeof(__le32) - 1)
> +
>  struct brcmf_dump_survey {
>  	u32 obss;
>  	u32 ibss;
> @@ -6840,6 +6843,13 @@ static int brcmf_construct_chaninfo(struct brcmf_cfg80211_info *cfg,
>  			band->channels[i].flags = IEEE80211_CHAN_DISABLED;
>  
>  	total = le32_to_cpu(list->count);
> +	if (total > BRCMF_MAX_CHANSPEC_LIST) {
> +		bphy_err(drvr, "Invalid count of channel Spec. (%u)\n",
> +			 total);
> +		err = -EINVAL;
> +		goto fail_pbuf;
> +	}
> +
>  	for (i = 0; i < total; i++) {
>  		ch.chspec = (u16)le32_to_cpu(list->element[i]);
>  		cfg->d11inf.decchspec(&ch);
> @@ -6985,6 +6995,13 @@ static int brcmf_enable_bw40_2g(struct brcmf_cfg80211_info *cfg)
>  		band = cfg_to_wiphy(cfg)->bands[NL80211_BAND_2GHZ];
>  		list = (struct brcmf_chanspec_list *)pbuf;
>  		num_chan = le32_to_cpu(list->count);
> +		if (num_chan > BRCMF_MAX_CHANSPEC_LIST) {
> +			bphy_err(drvr, "Invalid count of channel Spec. (%u)\n",
> +				 num_chan);
> +			kfree(pbuf);
> +			return -EINVAL;
> +		}
> +
>  		for (i = 0; i < num_chan; i++) {
>  			ch.chspec = (u16)le32_to_cpu(list->element[i]);
>  			cfg->d11inf.decchspec(&ch);

I have incorporated your review in this patch.
Please let me know if any further changes are needed.