Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210723050919.1910964-1-mudongliangabcd@gmail.com>
 <d2b0f847dbf6b6d1e585ef8de1d9d367f8d9fd3b.camel@sipsolutions.net>
 <CAD-N9QWDNvo_3bdB=8edyYWvEV=b-66Tx-P6_7JGgrSYshDh0A@mail.gmail.com>
 <11ba299b812212a07fe3631b7be0e8b8fd5fb569.camel@sipsolutions.net>
 <CAD-N9QWRNyZnnDQ3XTQ_SAWNEgiMCJV+5Z69eHtRVcxYtXcM+A@mail.gmail.com> <e549fbb09d7c618762996aca4242c2ae50f85a5c.camel@sipsolutions.net>
In-Reply-To: <e549fbb09d7c618762996aca4242c2ae50f85a5c.camel@sipsolutions.net>
From:   Dongliang Mu <mudongliangabcd@gmail.com>
Date:   Fri, 23 Jul 2021 17:59:39 +0800
Message-ID: <CAD-N9QV02fnr8LLSwxTuyevYgkL_2aicO2b_7uZ46s6BGKaTmw@mail.gmail.com>
Subject: Re: [PATCH] cfg80211: free the object allocated in wiphy_apply_custom_regulatory
To:     Johannes Berg <johannes@sipsolutions.net>
Cc:     Kalle Valo <kvalo@codeaurora.org>,
        "David S. Miller" <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>,
        Luca Coelho <luciano.coelho@intel.com>,
        Ilan Peer <ilan.peer@intel.com>,
        syzbot+1638e7c770eef6b6c0d0@syzkaller.appspotmail.com,
        linux-wireless@vger.kernel.org,
        "open list:NETWORKING [GENERAL]" <netdev@vger.kernel.org>,
        linux-kernel <linux-kernel@vger.kernel.org>,
        Dan Carpenter <dan.carpenter@oracle.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, Jul 23, 2021 at 5:42 PM Johannes Berg <johannes@sipsolutions.net> 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
>