netdev - Re: [PATCH 1/2] cfg80211: fix dead lock for nl80211_new

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f663ce29-72e7-7592-d4ef-d79fda1de102@quicinc.com>
Date:   Wed, 21 Sep 2022 08:03:04 -0700
From:   Jeff Johnson <quic_jjohnson@...cinc.com>
To:     Aran Dalton <arda@...winnertech.com>, <johannes@...solutions.net>,
        <davem@...emloft.net>, <edumazet@...gle.com>, <kuba@...nel.org>,
        <pabeni@...hat.com>
CC:     <johannes.berg@...el.com>, <linux-wireless@...r.kernel.org>,
        <netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/2] cfg80211: fix dead lock for nl80211_new_interface()

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@...winnertech.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;
>   }