Commit 4527025d authored by Jakub Kicinski's avatar Jakub Kicinski
Browse files

nfc: nci: fix circular locking dependency in nci_close_device



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: 6a2968aa ("NFC: basic NCI protocol implementation")
Reviewed-by: default avatarIan Ray <ian.ray@gehealthcare.com>
Link: https://patch.msgid.link/20260317193334.988609-1-kuba@kernel.org


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent 57ce3b2e
Loading
Loading
Loading
Loading
+6 −4
Original line number Diff line number Diff line
@@ -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;
}