Received-SPF: pass (google.com: domain of linux-wireless+bounces-7289-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Tue, 7 May 2024 07:21:33 -0700
From: Nathan Chancellor <nathan@kernel.org>
To: Johannes Berg <johannes@sipsolutions.net>
Cc: Kees Cook <keescook@chromium.org>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
	Jeff Johnson <quic_jjohnson@quicinc.com>,
	"Gustavo A. R. Silva" <gustavoars@kernel.org>,
	linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org
Subject: Re: [PATCH] wifi: nl80211: Avoid address calculations via out of
 bounds array indexing
Message-ID: <20240507142133.GB2746430@thelio-3990X>
References: <20240424220057.work.819-kees@kernel.org>
 <20240425181342.GA657080@dev-arch.thelio-3990X>
 <10256004963b6e1a1813c6f07c5d21abfc843070.camel@sipsolutions.net>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <10256004963b6e1a1813c6f07c5d21abfc843070.camel@sipsolutions.net>

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 <nathan@kernel.org>
> > > Fixes: e3eac9f32ec0 ("wifi: cfg80211: Annotate struct cfg80211_scan_request with __counted_by")
> > > Signed-off-by: Kees Cook <keescook@chromium.org>
> > 
> > Tested-by: Nathan Chancellor <nathan@kernel.org>
> > 
> 
> 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