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

[Nathan Chancellor <nathan@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
>