Hmm.
On Tue, 2024-07-16 at 20:40 +0300, Dmitry Antipov wrote:
> diff --git a/net/wireless/sme.c b/net/wireless/sme.c
> index a8ad55f11133..f5da45331847 100644
> --- a/net/wireless/sme.c
> +++ b/net/wireless/sme.c
> @@ -77,12 +77,16 @@ static int cfg80211_conn_scan(struct wireless_dev *wdev)
> else
> n_channels = ieee80211_get_num_supported_channels(wdev->wiphy);
>
> - request = kzalloc(sizeof(*request) + sizeof(request->ssids[0]) +
> - sizeof(request->channels[0]) * n_channels,
> - GFP_KERNEL);
> + request = kzalloc(struct_size(request, channels, n_channels) +
> + sizeof(request->ssids[0]), GFP_KERNEL);
That makes sense, sure.
> if (!request)
> return -ENOMEM;
>
> + /* None of the channels are actually set
> + * up but let UBSAN know the boundaries.
> + */
> + request->n_channels = n_channels;
Also makes sense, so we tell it how many we allocated early.
Note netdev we dropped the special comment style requirement, so
wouldn't mind
/*
* None of ...
* ...
*/
here either.
> +
> if (wdev->conn->params.channel) {
> enum nl80211_band band = wdev->conn->params.channel->band;
> struct ieee80211_supported_band *sband =
> @@ -112,9 +116,9 @@ static int cfg80211_conn_scan(struct wireless_dev *wdev)
> }
> request->rates[band] = (1 << bands->n_bitrates) - 1;
> }
> - n_channels = i;
> + request->n_channels = i;
So this tells it how many were actually used, in this branch, makes
sense.
Functionally, all of this seems OK.
However,
> request->ssids = (void *)&request->channels[n_channels];
is this not checked? I mean, if you have n_channels=5 and then take
&channels[5] I can see how that makes sense, but arguably the compiler
might complain if you have &channels[10] for an array you told it has 5
entries?
johannes
