[PATCH net] nfc: nci: fix circular locking dependency in nci_close_device

[PATCH net] nfc: nci: fix circular locking dependency in nci_close_device

[Jakub Kicinski]

nci_close_device() flushes rx_wq and tx_wq while holding req_lock.
This causes a circular locking dependency because nci_rx_work()
running on rx_wq can end up taking req_lock too:

  nci_rx_work -> nci_rx_data_packet -> nci_data_exchange_complete
    -> __sk_destruct -> rawsock_destruct -> nfc_deactivate_target
    -> nci_deactivate_target -> nci_request -> mutex_lock(&ndev->req_lock)

Move the flush of rx_wq after req_lock has been released.
This should safe (I think) because NCI_UP has already been cleared
and the transport is closed, so the work will see it and return
-ENETDOWN.

NIPA has been hitting this running the nci selftest with a debug
kernel on roughly 4% of the runs.

Fixes: 6a2968aaf50c ("NFC: basic NCI protocol implementation")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
CC: ian.ray@gehealthcare.com
CC: ilane@ti.com
CC: linville@tuxdriver.com
---
 net/nfc/nci/core.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/net/nfc/nci/core.c b/net/nfc/nci/core.c
index 43d871525dbc..5f46c4b5720f 100644
--- a/net/nfc/nci/core.c
+++ b/net/nfc/nci/core.c
@@ -579,8 +579,7 @@ static int nci_close_device(struct nci_dev *ndev)
 	skb_queue_purge(&ndev->rx_q);
 	skb_queue_purge(&ndev->tx_q);
 
-	/* Flush RX and TX wq */
-	flush_workqueue(ndev->rx_wq);
+	/* Flush TX wq, RX wq flush can't be under the lock */
 	flush_workqueue(ndev->tx_wq);
 
 	/* Reset device */
@@ -592,13 +591,13 @@ static int nci_close_device(struct nci_dev *ndev)
 		      msecs_to_jiffies(NCI_RESET_TIMEOUT));
 
 	/* After this point our queues are empty
-	 * and no works are scheduled.
+	 * rx work may be running but will see that NCI_UP was cleared
 	 */
 	ndev->ops->close(ndev);
 
 	clear_bit(NCI_INIT, &ndev->flags);
 
-	/* Flush cmd wq */
+	/* Flush cmd and tx wq */
 	flush_workqueue(ndev->cmd_wq);
 
 	timer_delete_sync(&ndev->cmd_timer);
@@ -613,6 +612,9 @@ static int nci_close_device(struct nci_dev *ndev)
 
 	mutex_unlock(&ndev->req_lock);
 
+	/* rx_work may take req_lock via nci_deactivate_target */
+	flush_workqueue(ndev->rx_wq);
+
 	return 0;
 }
 
-- 
2.53.0

Re: [PATCH net] nfc: nci: fix circular locking dependency in nci_close_device

[Ian Ray]

On Tue, Mar 17, 2026 at 12:33:34PM -0700, Jakub Kicinski wrote:
> nci_close_device() flushes rx_wq and tx_wq while holding req_lock.
> This causes a circular locking dependency because nci_rx_work()
> running on rx_wq can end up taking req_lock too:
> 
>   nci_rx_work -> nci_rx_data_packet -> nci_data_exchange_complete
>     -> __sk_destruct -> rawsock_destruct -> nfc_deactivate_target
>     -> nci_deactivate_target -> nci_request -> mutex_lock(&ndev->req_lock)
> 
> Move the flush of rx_wq after req_lock has been released.
> This should safe (I think) because NCI_UP has already been cleared
> and the transport is closed, so the work will see it and return
> -ENETDOWN.
> 
> NIPA has been hitting this running the nci selftest with a debug
> kernel on roughly 4% of the runs.
> 
> Fixes: 6a2968aaf50c ("NFC: basic NCI protocol implementation")
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>

Reviewed-by: Ian Ray <ian.ray@gehealthcare.com>

> ---
> CC: ian.ray@gehealthcare.com
> CC: ilane@ti.com
> CC: linville@tuxdriver.com
> ---
>  net/nfc/nci/core.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/net/nfc/nci/core.c b/net/nfc/nci/core.c
> index 43d871525dbc..5f46c4b5720f 100644
> --- a/net/nfc/nci/core.c
> +++ b/net/nfc/nci/core.c
> @@ -579,8 +579,7 @@ static int nci_close_device(struct nci_dev *ndev)
>         skb_queue_purge(&ndev->rx_q);
>         skb_queue_purge(&ndev->tx_q);
> 
> -       /* Flush RX and TX wq */
> -       flush_workqueue(ndev->rx_wq);
> +       /* Flush TX wq, RX wq flush can't be under the lock */
>         flush_workqueue(ndev->tx_wq);
> 
>         /* Reset device */
> @@ -592,13 +591,13 @@ static int nci_close_device(struct nci_dev *ndev)
>                       msecs_to_jiffies(NCI_RESET_TIMEOUT));
> 
>         /* After this point our queues are empty
> -        * and no works are scheduled.
> +        * rx work may be running but will see that NCI_UP was cleared
>          */
>         ndev->ops->close(ndev);
> 
>         clear_bit(NCI_INIT, &ndev->flags);
> 
> -       /* Flush cmd wq */
> +       /* Flush cmd and tx wq */
>         flush_workqueue(ndev->cmd_wq);
> 
>         timer_delete_sync(&ndev->cmd_timer);
> @@ -613,6 +612,9 @@ static int nci_close_device(struct nci_dev *ndev)
> 
>         mutex_unlock(&ndev->req_lock);
> 
> +       /* rx_work may take req_lock via nci_deactivate_target */
> +       flush_workqueue(ndev->rx_wq);
> +
>         return 0;
>  }
> 
> --
> 2.53.0
> 

Re: [PATCH net] nfc: nci: fix circular locking dependency in nci_close_device

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Tue, 17 Mar 2026 12:33:34 -0700 you wrote:
> nci_close_device() flushes rx_wq and tx_wq while holding req_lock.
> This causes a circular locking dependency because nci_rx_work()
> running on rx_wq can end up taking req_lock too:
> 
>   nci_rx_work -> nci_rx_data_packet -> nci_data_exchange_complete
>     -> __sk_destruct -> rawsock_destruct -> nfc_deactivate_target
>     -> nci_deactivate_target -> nci_request -> mutex_lock(&ndev->req_lock)
> 
> [...]

Here is the summary with links:
  - [net] nfc: nci: fix circular locking dependency in nci_close_device
    https://git.kernel.org/netdev/net/c/4527025d440c

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html