On Fri, Jul 23, 2021 at 5:42 PM Johannes Berg [off-list ref] wrote:

Hi,

On Fri, 2021-07-23 at 17:30 +0800, Dongliang Mu wrote:

quoted

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