[PATCH] nfc: port100: cancel cmd_complete_work before freeing on disconnect

[Maoyi Xie]

port100_disconnect() kills the URBs and frees dev->cmd. dev itself is devm
allocated and released once disconnect() returns. It never cancels
dev->cmd_complete_work.

A response or ack URB completion schedules cmd_complete_work from
port100_recv_response() or port100_recv_ack(). usb_kill_urb() waits for a
completion handler that is still running. A work item it already queued is
not cancelled. After disconnect() frees dev->cmd and returns, the worker
runs port100_send_async_complete(). Its first accesses are

	struct port100_cmd *cmd = dev->cmd;
	int status = cmd->status;

so it reads the freed command. That is a use after free.

Cancel the work synchronously after killing the URBs. Nothing can
re-schedule it then. Do this before freeing dev->cmd.

This was reproduced under KASAN on 7.1-rc7. With the current code the work
runs after dev->cmd is freed. KASAN reports a slab use after free read of
cmd->status from the workqueue. With cancel_work_sync() the work is
cancelled and the report is gone.

  BUG: KASAN: slab-use-after-free in the cmd_complete_work worker
  Read of size 4 (cmd->status) by task kworker
  Freed by the kfree(dev->cmd) in port100_disconnect()

thunderbolt XDomain fixed the same race recently. A delayed work ran
after disconnect had freed its state. port100 has the same shape and is
still unfixed.

Sven Van Asbroeck proposed this fix in a 2019 RFC [1]. It was never merged
and the disconnect path is still unaddressed. The earlier fix, commit
f80cfe2f2658 ("NFC: port100: fix use-after-free in port100_send_complete"),
only added usb_kill_urb() to the probe error path.

Link: https://lore.kernel.org/all/20190205160118.27491-1-TheSven73@xxxxxxxxxxxxxx/ [1]
Fixes: 0347a6ab300a ("NFC: port100: Commands mechanism implementation")
Suggested-by: Sven Van Asbroeck <thesven73@xxxxxxxxx>
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Maoyi Xie <maoyixie.tju@xxxxxxxxx>
---
 drivers/nfc/port100.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/nfc/port100.c b/drivers/nfc/port100.c
index 5ae61d7ebcfe..2483864806de 100644
--- a/drivers/nfc/port100.c
+++ b/drivers/nfc/port100.c
@@ -1618,6 +1618,8 @@ static void port100_disconnect(struct usb_interface *interface)
 	usb_kill_urb(dev->in_urb);
 	usb_kill_urb(dev->out_urb);
 
+	cancel_work_sync(&dev->cmd_complete_work);
+
 	usb_free_urb(dev->in_urb);
 	usb_free_urb(dev->out_urb);
 
-- 
2.34.1