Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <feb446cd-e935-6fc4-7f3c-d7ab21d18d8e@gmail.com>
Date:   Mon, 7 Feb 2022 22:13:08 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.5.1
Subject: Re: [BUG] intersil: p54: possible deadlock in p54_remove_interface()
 and p54_stop()
Content-Language: en-US
To:     Jia-Ju Bai <baijiaju1990@gmail.com>, chunkeey@googlemail.com,
        kvalo@kernel.org, davem@davemloft.net, kuba@kernel.org
Cc:     linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <fb543659-f69d-242f-b18a-69dd8b8b5ca1@gmail.com>
From:   Christian Lamparter <chunkeey@gmail.com>
In-Reply-To: <fb543659-f69d-242f-b18a-69dd8b8b5ca1@gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

Hi,

On 07/02/2022 16:31, Jia-Ju Bai wrote:
> Hello,
> 
> My static analysis tool reports a possible deadlock in the p54 driver in Linux 5.16:
> 
> p54_remove_interface()
>    mutex_lock(&priv->conf_mutex); --> Line 262 (Lock A)
> wait_for_completion_interruptible_timeout(&priv->beacon_comp, HZ); --> Line 271 (Wait X)
> 
> p54_stop()
>    mutex_lock(&priv->conf_mutex); --> Line 208 (Lock A)
>    p54p_stop() (call via priv->stop)
>      p54_free_skb()
>        p54_tx_qos_accounting_free()
>          complete(&priv->beacon_comp); --> Line 230 (Wake X)
> 
> When p54_remove_interface() is executed, "Wait X" is performed by holding "Lock A". 
> If p54_stop() is executed at this time, "Wake X" cannot be performed to wake 
> up "Wait X" in p54_remove_interface(), because "Lock A" has been already hold by
> p54_remove_interface(), causing a possible deadlock.
>
> I find that "Wait X" is performed with a timeout, to relieve the possible deadlock;
> but I think this timeout can cause inefficient execution.
> 
> I am not quite sure whether this possible problem is real and how to fix it if it is real.
> Any feedback would be appreciated, thanks :)

This has been such a long long time ago. But I think I found
the right documentation entry for you:

<https://www.kernel.org/doc/html/v5.16/driver-api/80211/mac80211.html> (scroll down a bit)
| remove_interface
| 	Notifies a driver that an interface is going down.
| 	>>The stop callback is called after this if it is the last interface
|	and no monitor interfaces are present.<<
|	(it goes on a bit. But I don't think there's anything important left)

The documentation tells you not to worry. p54_stop() and p54_remove_interface()
are being serialized by mac80211.

Cheers,
Christian