2024-05-07 14:24:45

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [PATCH] wifi: nl80211: Avoid address calculations via out of bounds array indexing

On Tue, May 07, 2024 at 12:46:46PM +0200, Johannes Berg wrote:
> On Thu, 2024-04-25 at 11:13 -0700, Nathan Chancellor wrote:
> > On Wed, Apr 24, 2024 at 03:01:01PM -0700, Kees Cook wrote:
> > > Before request->channels[] can be used, request->n_channels must be set.
> > > Additionally, address calculations for memory after the "channels" array
> > > need to be calculated from the allocation base ("request") rather than
> > > via the first "out of bounds" index of "channels", otherwise run-time
> > > bounds checking will throw a warning.
> > >
> > > Reported-by: Nathan Chancellor <[email protected]>
> > > Fixes: e3eac9f32ec0 ("wifi: cfg80211: Annotate struct cfg80211_scan_request with __counted_by")
> > > Signed-off-by: Kees Cook <[email protected]>
> >
> > Tested-by: Nathan Chancellor <[email protected]>
> >
>
> How do you get this tested? We have the same, and more, bugs in
> cfg80211_scan_6ghz() which I'm fixing right now, but no idea how to
> actually get the checks done?

You'll need a toolchain with __counted_by support, which I believe is
only clang 18+ at this point (I have prebuilts available at [1]), and
CONFIG_UBSAN_BOUNDS enabled, then they should just pop up in dmesg.

[1]: https://mirrors.edge.kernel.org/pub/tools/llvm/

Cheers,
Nathan