linux-hardening - Re: [PATCH] wifi: wireless: fix more UBSAN noise in cfg80211_conn

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <202407161121.C00AC44@keescook>
Date: Tue, 16 Jul 2024 11:21:40 -0700
From: Kees Cook <kees@...nel.org>
To: Dmitry Antipov <dmantipov@...dex.ru>
Cc: Johannes Berg <johannes.berg@...el.com>, linux-wireless@...r.kernel.org,
	linux-hardening@...r.kernel.org, lvc-project@...uxtesting.org
Subject: Re: [PATCH] wifi: wireless: fix more UBSAN noise in
 cfg80211_conn_scan()

On Tue, Jul 16, 2024 at 08:40:11PM +0300, Dmitry Antipov wrote:
> Looking at https://syzkaller.appspot.com/bug?extid=d5dc2801166df6d34774
> and trying to reproduce it with CONFIG_UBSAN enabled, I've noticed the
> following:
> 
> UBSAN: array-index-out-of-bounds in net/wireless/sme.c:95:3
> index 0 is out of range for type 'struct ieee80211_channel *[]'
> CPU: 3 PID: 4993 Comm: repro Not tainted 6.10.0-01155-gd67978318827 #5
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS <...>
> Call Trace:
>  <TASK>
>  dump_stack_lvl+0x1c2/0x2a0
>  ? __pfx_dump_stack_lvl+0x10/0x10
>  ? __pfx__printk+0x10/0x10
>  ? __local_bh_enable_ip+0x12e/0x1c0
>  __ubsan_handle_out_of_bounds+0x127/0x150
>  cfg80211_conn_scan+0xd8e/0xf30
>  cfg80211_connect+0x1400/0x1c30
>  nl80211_connect+0x1549/0x1a70
>  ...<the rest is not too useful...>
> 
> This is very similar to 92ecbb3ac6f3 ("wifi: mac80211: fix UBSAN noise
> in ieee80211_prep_hw_scan()"), so just fix it in the same way by setting
> 'request->n_channels' early to help '__counted_by()' work as expected.
> And the same 'kmalloc()' math adjustment is also applicable.
> 
> Signed-off-by: Dmitry Antipov <dmantipov@...dex.ru>

Nice catch! Yes, this looks correct.

Reviewed-by: Kees Cook <kees@...nel.org>

-- 
Kees Cook