Hi,
On Fri, 2021-07-23 at 17:30 +0800, Dongliang Mu wrote:
> if zhao in the thread is right, we don't need to add this free
> operation to wiphy_free().
Actually, no, that statement is not true.
All that zhao claimed was that the free happens correctly during
unregister (or later), and that is indeed true, since it happens from
ieee80211_unregister_hw()
-> wiphy_unregister()
-> wiphy_regulatory_deregister()
However, syzbot of course is also correct. Abstracting a bit and
ignoring mac80211, the problem is that here we assign it before
wiphy_register(), then wiphy_register() doesn't get called or fails, and
therefore we don't call wiphy_unregister(), only wiphy_free().
Hence the leak.
But you can also easily see from that description that it's not related
to hwsim - we should add a secondary round of cleanups in wiphy_free()
or even move the call to wiphy_regulatory_deregister() into
wiphy_free(), we need to look what else this does to see if we can move
it or not.
johannes
On Fri, Jul 23, 2021 at 5:42 PM Johannes Berg <[email protected]> wrote:
>
> Hi,
>
> On Fri, 2021-07-23 at 17:30 +0800, Dongliang Mu wrote:
> > if zhao in the thread is right, we don't need to add this free
> > operation to wiphy_free().
>
> Actually, no, that statement is not true.
>
> All that zhao claimed was that the free happens correctly during
> unregister (or later), and that is indeed true, since it happens from
>
> ieee80211_unregister_hw()
> -> wiphy_unregister()
> -> wiphy_regulatory_deregister()
>
Thanks for your explanation. Now the situation is more clear.
>
> However, syzbot of course is also correct. Abstracting a bit and
> ignoring mac80211, the problem is that here we assign it before
> wiphy_register(), then wiphy_register() doesn't get called or fails, and
> therefore we don't call wiphy_unregister(), only wiphy_free().
Yes, you're right. In this case, wiphy_register is not called. We
should not call wiphy_unregister() to clean up anything.
>
> Hence the leak.
>
> But you can also easily see from that description that it's not related
> to hwsim - we should add a secondary round of cleanups in wiphy_free()
> or even move the call to wiphy_regulatory_deregister() into
> wiphy_free(), we need to look what else this does to see if we can move
> it or not.
I agree to move the cleanup operation of regd to wiphy_free API.
That's the partial functionability of this function.
>
> johannes
>