On Mon, 2025-07-14 at 11:09 -0700, Jeff Johnson wrote:
> On 7/14/2025 5:24 AM, Johannes Berg wrote:
> > From: Johannes Berg <johannes.berg@xxxxxxxxx>
> >
> > This (partially) reverts commits
> > - 838c7b8f1f27 ("wifi: nl80211: Avoid address calculations via out of bounds array indexing")
> > - f1d3334d604c ("wifi: cfg80211: sme: init n_channels before channels[] access")
> > - 82bbe02b2500 ("wifi: mac80211: Set n_channels after allocating struct cfg80211_scan_request")
> >
> > These commits all set the structure to be in an inconsistent
> > state, setting n_channels to some value before them actually
> > being filled in. That's fine for what the code does now, but
> > with the removal of __counted_by() it's no longer needed and
> > it does leave a bit of a landmine there since breaking out of
> > some code to send the scan or something would leave it wrong.
>
> Perhaps also refer to:
> <https://lore.kernel.org/all/20250714142130.9b0bbb7e1f07.I09112ccde72d445e11348fc2bef68942cb2ffc94@changeid/>
>
> Since (as of now) __counted_by() is still there in wireless-next
Well I can't merge this until the other one comes back to wireless-next.
> > +++ b/net/mac80211/main.c
> > @@ -1359,7 +1359,6 @@ int ieee80211_register_hw(struct ieee80211_hw *hw)
> > GFP_KERNEL);
> > if (!local->int_scan_req)
> > return -ENOMEM;
> > - local->int_scan_req->n_channels = channels;
>
> so i'm looking at how int_scan_req is used.
> is there ever a possibility that at the time it is used that the underlying
> wiphy information may have changed such that the number of channels trying to
> be filled exceed the number calculated and allocated in this function?
> just want to make sure there isn't a reason to keep track of the allocation
> size somewhere.
I don't think so, the supported bands/channels aren't supposed to change
at runtime, and we can't really scan more channels than we have. Of
course in theory we could always get it wrong somewhere, but ...
johannes
