Re: UBSAN array-index-out-of-bounds: cfg80211_scan_6ghz

[John Rowley <lkml@johnrowley.me>]

----------------------------------------

From: Kees Cook <kees@kernel.org>
To: Nathan Chancellor <nathan@kernel.org>
CC: John Rowley <lkml@johnrowley.me>; johannes@sipsolutions.net; linux-wireless@vger.kernel.org; stable@vger.kernel.org; gustavoars@kernel.org; linux-hardening@vger.kernel.org
Date: 30 Dec 2024 18:35:34
Subject: Re: UBSAN array-index-out-of-bounds: cfg80211_scan_6ghz

> On Sun, Dec 29, 2024 at 10:38:06PM -0700, Nathan Chancellor wrote:
>> Hi John,
>> 
>> On Sat, Dec 28, 2024 at 11:21:27AM +0000, John Rowley wrote:
>>> Hi, I'm experiencing UBSAN array-index-out-of-bounds errors while using
>>> my Framework 13" AMD laptop with its Mediatek MT7922 wifi adapter
>>> (mt7921e).
>>> 
>>> It seems to happen only once on boot, and occurs with both kernel
>>> versions 6.12.7 and 6.13-rc4, both compiled from vanilla upstream kernel
>>> sources on Fedora 41 using the kernel.org LLVM toolchain (19.1.6).
>>> 
>>> I can try some other kernel series if necessary, and also a bisect if I
>>> find a working version, but that may take me a while.
>> 
>> This looks related to UBSAN_BOUNDS and the fact that version of clang
>> supports the __counted_by attribute. I do not have much time at the
>> moment to look at this but I have added Kees, Gustavo, and
>> linux-hardening for further analysis.
>> 
>> Cheers,
>> Nathan
>> 
>>> I wasn't sure if I should mark this as a regression, as I'm not sure
>>> which/if there is a working kernel version at this point.
>>> 
>>> Thanks.
>>> 
>>> ----
>>> 
>>> [   17.754417] UBSAN: array-index-out-of-bounds in /data/linux/net/wireless/scan.c:766:2
>>> [   17.754423] index 0 is out of range for type 'struct ieee80211_channel *[] __counted_by(n_channels)' (aka 'struct ieee80211_channel *[]')
> 
> This is this line:
> 
>         request->channels[n_channels] = chan;
> 
> and later:
> 
>         request->n_channels++;
> 
> Patch should be:
> 
> diff --git a/net/wireless/scan.c b/net/wireless/scan.c
> index 1c6fd45aa809..ccdbeb604639 100644
> --- a/net/wireless/scan.c
> +++ b/net/wireless/scan.c
> @@ -763,12 +763,11 @@ static  void cfg80211_scan_req_add_chan(struct cfg80211_scan_request *request,
>         }
>     }
> 
> +   request->n_channels++;
>     request->channels[n_channels] = chan;
>     if (add_to_6ghz)
>         request->scan_6ghz_params[request->n_6ghz_params].channel_idx =
>             n_channels;
> -
> -   request->n_channels++;
> }
> 
> static bool cfg80211_find_ssid_match(struct cfg80211_colocated_ap *ap,
> 
> 
> -- 
> Kees Cook
Apologies for not replying sooner but the above patch works perfectly. Thanks!