[PATCH] nfc: nci: drain rx_wq before cmd_wq on unregister

[PATCH] nfc: nci: drain rx_wq before cmd_wq on unregister

[Yogesh Gaur]

nci_unregister_device() destroys cmd_wq first and rx_wq second. Any
rx_work still running at that point can handle a response and queue
cmd_work from nci_rsp_packet(), which is refused because cmd_wq is
already draining:

  workqueue: cannot queue nci_cmd_work on wq nfc2_nci_cmd_wq
  WARNING: kernel/workqueue.c:2352 at __queue_work+0xdb7/0x1370 kernel/workqueue.c:2351, CPU#3: kworker/u32:0/12
  Workqueue: nfc2_nci_rx_wq nci_rx_work
  Call Trace:
   queue_work_on+0x180/0x1e0 kernel/workqueue.c:2501
   queue_work include/linux/workqueue.h:700 [inline]
   nci_rsp_packet+0x297/0x3420 net/nfc/nci/rsp.c:463
   nci_rx_work+0x29c/0x430 net/nfc/nci/core.c:1579
   process_one_work+0xac7/0x1b10 kernel/workqueue.c:3396

nci_close_device() does not stop rx_work from running again
afterwards: when the device was never brought up it does not flush
rx_wq at all, and the driver can still deliver frames through
nci_recv_frame() until it has stopped calling it.

Destroy the queues in dependency order instead. rx_work queues
cmd_work and tx_work, so rx_wq goes first. The cmd and data timers
queue cmd_work and rx_work, and cmd_work and tx_work re-arm them, so
shut both timers down before any of the queues go away; after
timer_shutdown_sync() the re-arming is a no-op.

Fixes: 6a2968aaf50c ("NFC: basic NCI protocol implementation")
Reported-by: syzbot+ebbbf06f152da8ea4716@syzkaller.appspotmail.com
Assisted-by: LLM
Signed-off-by: Yogesh Gaur <yogeshgaur.83@gmail.com>
---
Built with W=1 only. syzbot has no reproducer for this report, so the
fix has not been runtime-tested

 net/nfc/nci/core.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/net/nfc/nci/core.c b/net/nfc/nci/core.c
index 73e3a96470ac..aa417f863f96 100644
--- a/net/nfc/nci/core.c
+++ b/net/nfc/nci/core.c
@@ -1329,8 +1329,13 @@ void nci_unregister_device(struct nci_dev *ndev)
 
 	nci_close_device(ndev);
 
-	destroy_workqueue(ndev->cmd_wq);
+	/* cmd_work and tx_work re-arm these, and they queue cmd/rx work */
+	timer_shutdown_sync(&ndev->cmd_timer);
+	timer_shutdown_sync(&ndev->data_timer);
+
+	/* rx_work queues cmd_work and tx_work, so drain rx_wq first */
 	destroy_workqueue(ndev->rx_wq);
+	destroy_workqueue(ndev->cmd_wq);
 	destroy_workqueue(ndev->tx_wq);
 
 	list_for_each_entry_safe(conn_info, n, &ndev->conn_info_list, list) {
-- 
2.55.0.windows.5