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

From: Nathan Chancellor
Date: Thu Apr 25 2024 - 14:13:57 EST


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@xxxxxxxxxx>
> Fixes: e3eac9f32ec0 ("wifi: cfg80211: Annotate struct cfg80211_scan_request with __counted_by")
> Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx>

Tested-by: Nathan Chancellor <nathan@xxxxxxxxxx>

> ---
> Cc: Johannes Berg <johannes@xxxxxxxxxxxxxxxx>
> Cc: "David S. Miller" <davem@xxxxxxxxxxxxx>
> Cc: Eric Dumazet <edumazet@xxxxxxxxxx>
> Cc: Jakub Kicinski <kuba@xxxxxxxxxx>
> Cc: Paolo Abeni <pabeni@xxxxxxxxxx>
> Cc: linux-wireless@xxxxxxxxxxxxxxx
> Cc: netdev@xxxxxxxxxxxxxxx
> ---
> 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
>