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

On Fri, 2021-07-23 at 13:09 +0800, Dongliang Mu wrote:

quoted

The commit beee24695157 ("cfg80211: Save the regulatory domain when
setting custom regulatory") forgets to free the newly allocated regd
object.

Not really? It's not forgetting it, it just saves it?

Yes, it saves the regd object in the function wiphy_apply_custom_regulatory.

But its parent function - mac80211_hwsim_new_radio forgets to free
this object when the ieee80211_register_hw fails.

+       new_regd = reg_copy_regd(regd);
+       if (IS_ERR(new_regd))
+               return;
+
+       tmp = get_wiphy_regdom(wiphy);
+       rcu_assign_pointer(wiphy->regd, new_regd);
+       rcu_free_regdom(tmp);

quoted

Fix this by freeing the regd object in the error handling code and
deletion function - mac80211_hwsim_del_radio.

This can't be right - the same would affect all other users of that
function, no?

The problem occurs in the error handling code of
mac80211_hwsim_new_radio, not wiphy_apply_custom_regulatory. My commit
message may be not very clear.

So I think the code in the mac80211_hwsim_del_radio paired with
mac80211_hwsim_new_radio should be changed correspondingly. If I miss
any problems, please let me know.

I have successfully tested my patch in the syzbot dashboard [1].

[1] https://syzkaller.appspot.com/bug?extid=1638e7c770eef6b6c0d0

Perhaps somewhere we have a case where wiphy->regd is leaked, but than
that should be fixed more generally in cfg80211?

johannes