Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <f663ce29-72e7-7592-d4ef-d79fda1de102@quicinc.com>
Date:   Wed, 21 Sep 2022 08:03:04 -0700
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.2.2
Subject: Re: [PATCH 1/2] cfg80211: fix dead lock for nl80211_new_interface()
Content-Language: en-US
To:     Aran Dalton <arda@allwinnertech.com>, <johannes@sipsolutions.net>,
        <davem@davemloft.net>, <edumazet@google.com>, <kuba@kernel.org>,
        <pabeni@redhat.com>
CC:     <johannes.berg@intel.com>, <linux-wireless@vger.kernel.org>,
        <netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>
References: <20220921091913.110749-1-arda@allwinnertech.com>
From:   Jeff Johnson <quic_jjohnson@quicinc.com>
In-Reply-To: <20220921091913.110749-1-arda@allwinnertech.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 9/21/2022 2:19 AM, Aran Dalton wrote:
> Both nl80211_new_interface and cfg80211_netdev_notifier_call hold the
> same wiphy_lock, then cause deadlock.
> 
> The main call stack as bellow:
> 
> nl80211_new_interface() takes wiphy_lock
>   -> _nl80211_new_interface:
>    -> rdev_add_virtual_intf
>     -> rdev->ops->add_virtual_intf
>      -> register_netdevice
>       -> call_netdevice_notifiers(NETDEV_REGISTER, dev);
>        -> call_netdevice_notifiers_extack
>         -> call_netdevice_notifiers_info
>          -> raw_notifier_call_chain
>           -> cfg80211_netdev_notifier_call
>            -> wiphy_lock(&rdev->wiphy), cfg80211_register_wdev

In both of your patches please describe what you are doing in the patch 
to fix the problem, and in particular describe why your fix is safe.

> 
> Fixes: ea6b2098dd02 ("cfg80211: fix locking in netlink owner interface destruction")
> Signed-off-by: Aran Dalton <arda@allwinnertech.com>
> ---
>   net/wireless/nl80211.c | 2 --
>   1 file changed, 2 deletions(-)
> 
> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> index 2705e3ee8fc4..bdacddc3ffa3 100644
> --- a/net/wireless/nl80211.c
> +++ b/net/wireless/nl80211.c
> @@ -4260,9 +4260,7 @@ static int nl80211_new_interface(struct sk_buff *skb, struct genl_info *info)
>   	/* to avoid failing a new interface creation due to pending removal */
>   	cfg80211_destroy_ifaces(rdev);
>   
> -	wiphy_lock(&rdev->wiphy);
>   	ret = _nl80211_new_interface(skb, info);
> -	wiphy_unlock(&rdev->wiphy);
>   
>   	return ret;
>   }