Received-SPF: pass (google.com: domain of linux-wireless+bounces-6869-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Thu, 25 Apr 2024 11:13:42 -0700
From: Nathan Chancellor <nathan@kernel.org>
To: Kees Cook <keescook@chromium.org>
Cc: Johannes Berg <johannes@sipsolutions.net>,
	"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: <20240425181342.GA657080@dev-arch.thelio-3990X>
References: <20240424220057.work.819-kees@kernel.org>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240424220057.work.819-kees@kernel.org>

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>

> ---
> Cc: Johannes Berg <johannes@sipsolutions.net>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Jakub Kicinski <kuba@kernel.org>
> Cc: Paolo Abeni <pabeni@redhat.com>
> Cc: linux-wireless@vger.kernel.org
> Cc: netdev@vger.kernel.org
> ---
>  net/wireless/nl80211.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> index f391b4055944..f1ed0981147e 100644
> --- a/net/wireless/nl80211.c
> +++ b/net/wireless/nl80211.c
> @@ -9163,6 +9163,7 @@ static int nl80211_trigger_scan(struct sk_buff *skb, struct genl_info *info)
>  	struct wiphy *wiphy;
>  	int err, tmp, n_ssids = 0, n_channels, i;
>  	size_t ie_len, size;
> +	size_t ssids_offset, ie_offset;
>  
>  	wiphy = &rdev->wiphy;
>  
> @@ -9208,21 +9209,20 @@ static int nl80211_trigger_scan(struct sk_buff *skb, struct genl_info *info)
>  		return -EINVAL;
>  
>  	size = struct_size(request, channels, n_channels);
> +	ssids_offset = size;
>  	size = size_add(size, array_size(sizeof(*request->ssids), n_ssids));
> +	ie_offset = size;
>  	size = size_add(size, ie_len);
>  	request = kzalloc(size, GFP_KERNEL);
>  	if (!request)
>  		return -ENOMEM;
> +	request->n_channels = n_channels;
>  
>  	if (n_ssids)
> -		request->ssids = (void *)&request->channels[n_channels];
> +		request->ssids = (void *)request + ssids_offset;
>  	request->n_ssids = n_ssids;
> -	if (ie_len) {
> -		if (n_ssids)
> -			request->ie = (void *)(request->ssids + n_ssids);
> -		else
> -			request->ie = (void *)(request->channels + n_channels);
> -	}
> +	if (ie_len)
> +		request->ie = (void *)request + ie_offset;
>  
>  	i = 0;
>  	if (scan_freqs) {
> -- 
> 2.34.1
>