[PATCH v2 0/2] nfc: pn533: fix hung task on stalled USB send

[PATCH v2 0/2] nfc: pn533: fix hung task on stalled USB send

[Zhao Feng]

The out URB wait in pn533_usb_send_frame() is unbounded, so a device
that never completes the transfer hangs the task holding the command
lock (syzbot report).  Simply adding a timeout is not enough, because
the error path of pn533_wq_cmd() cannot deal with a send failure: it
drops the queued command without completing it, leaving synchronous
callers asleep forever and the command queue frozen.

Patch 1 fixes that pre-existing error path; patch 2 then adds the
actual send timeout, handling the race where the transfer completes
right between the timeout expiring and the URB being killed.

Issues pointed out by the Sashiko AI review of v1.

Link to v1: https://lore.kernel.org/netdev/20261023025602.1746-1-zhaofeng2@cmss.chinamobile.com/

Zhao Feng (2):
  nfc: pn533: complete failed queued commands and keep queue moving
  nfc: pn533: add timeout to USB frame send completion wait

 drivers/nfc/pn533/pn533.c |  8 +++++++-
 drivers/nfc/pn533/usb.c   | 20 ++++++++++++++++----
 2 files changed, 23 insertions(+), 5 deletions(-)

-- 
2.39.2

[PATCH v2 1/2] nfc: pn533: complete failed queued commands and keep queue moving

[Zhao Feng]

When pn533_wq_cmd() fails to send a queued command it silently drops
it: the completion callback is never called, cmd_pending stays set and
cmd_work is never rescheduled.  As a result:

- a synchronous caller sleeps forever in pn533_send_cmd_sync()'s
  wait_for_completion();
- an asynchronous caller is never completed, leaking its callback
  context (e.g. struct pn533_data_exchange_arg, and the sock_hold()
  reference for rawsock);
- cmd_pending stuck at 1 makes every later command queue up without
  ever being sent, freezing the device until unbind.

The broken error path is as old as the command queue, but it used to
require an URB submission error to trigger; a device that stalls the
OUT endpoint reaches it via the send timeout added by the next patch.

Call the command's completion callback with the error and reschedule
cmd_work (unless the callback chained a direct command by returning
-EINPROGRESS) so that every queued command completes with an error and
the queue drains instead of freezing.

Signed-off-by: Zhao Feng <zhaofeng2@cmss.chinamobile.com>
Cc: <stable@vger.kernel.org>
---
 drivers/nfc/pn533/pn533.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/nfc/pn533/pn533.c b/drivers/nfc/pn533/pn533.c
index e2bc67300a..3087f4f347 100644
--- a/drivers/nfc/pn533/pn533.c
+++ b/drivers/nfc/pn533/pn533.c
@@ -572,9 +572,15 @@ static void pn533_wq_cmd(struct work_struct *work)
 	dev->cmd = cmd;
 	rc = dev->phy_ops->send_frame(dev, cmd->req);
 	if (rc < 0) {
+		int cb_rc;
+
 		dev->cmd = NULL;
 		dev_kfree_skb(cmd->req);
+		cb_rc = cmd->complete_cb(dev, cmd->complete_cb_context,
+					 ERR_PTR(rc));
 		kfree(cmd);
+		if (cb_rc != -EINPROGRESS)
+			queue_work(dev->wq, &dev->cmd_work);
 		return;
 	}
 
-- 
2.39.2

[PATCH v2 2/2] nfc: pn533: add timeout to USB frame send completion wait

[Zhao Feng]

pn533_usb_send_frame() waits unboundedly for the out URB to complete.
A device that never completes the transfer makes the task sleep
forever in TASK_UNINTERRUPTIBLE while holding the command mutex, as
reported by syzbot (hung task in pn533_usb_send_frame).

Use a 1 second timeout instead.  On timeout, kill the URB and return
-ETIMEDOUT so the command in progress fails gracefully instead of
hanging the task.

The transfer may complete between the timeout expiring and
usb_kill_urb(); in that case urb->status is 0 after the kill and the
send proceeds normally instead of reporting a spurious failure,
mirroring what usb_start_wait_urb() does.

Fixes: 9dab880d675b ("nfc: pn533: Wait for out_urb's completion in pn533_usb_send_frame()")
Reported-by: syzbot+1c5368988052dc2d8201@syzkaller.appspotmail.com
Closes: https://syzbot.org/bug?extid=1c5368988052dc2d8201
Signed-off-by: Zhao Feng <zhaofeng2@cmss.chinamobile.com>
Cc: <stable@vger.kernel.org>
---
v2: check urb->status after killing the URB on timeout and carry on if
    the transfer completed in the race window; add Fixes tag and Cc
    stable; the pn533_wq_cmd() error handling fix is now patch 1/2.
    Issues reported by the Sashiko AI review.

 drivers/nfc/pn533/usb.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/nfc/pn533/usb.c b/drivers/nfc/pn533/usb.c
index 0b7e3b118d..265baf2213 100644
--- a/drivers/nfc/pn533/usb.c
+++ b/drivers/nfc/pn533/usb.c
@@ -30,6 +30,9 @@
 #define ACS_VENDOR_ID 0x072f
 #define ACR122U_PRODUCT_ID 0x2200
 
+/* How long to wait for the out URB to complete */
+#define PN533_USB_SEND_TIMEOUT_MS	1000
+
 static const struct usb_device_id pn533_usb_table[] = {
 	{ USB_DEVICE(PN533_VENDOR_ID, PN533_PRODUCT_ID),
 	  .driver_info = PN533_DEVICE_STD },
@@ -184,8 +187,20 @@ static int pn533_usb_send_frame(struct pn533 *dev,
 	if (rc)
 		return rc;
 
-	wait_for_completion(&arg.done);
-	phy->out_urb->context = cntx;
+	if (!wait_for_completion_timeout(&arg.done,
+					 msecs_to_jiffies(PN533_USB_SEND_TIMEOUT_MS))) {
+		usb_kill_urb(phy->out_urb);
+		phy->out_urb->context = cntx;
+		/*
+		 * The transfer may have completed between the timeout
+		 * expiring and the urb being killed; if so, carry on as
+		 * usual instead of reporting a bogus failure.
+		 */
+		if (phy->out_urb->status)
+			return -ETIMEDOUT;
+	} else {
+		phy->out_urb->context = cntx;
+	}
 
 	if (dev->protocol_type == PN533_PROTO_REQ_RESP) {
 		/* request for response for sent packet directly */
-- 
2.39.2

Re: [PATCH v2 1/2] nfc: pn533: complete failed queued commands and keep queue moving

[Simon Horman]

On Tue, Oct 27, 2026 at 03:01:34PM +0800, Zhao Feng wrote:
> When pn533_wq_cmd() fails to send a queued command it silently drops
> it: the completion callback is never called, cmd_pending stays set and
> cmd_work is never rescheduled.  As a result:
> 
> - a synchronous caller sleeps forever in pn533_send_cmd_sync()'s
>   wait_for_completion();
> - an asynchronous caller is never completed, leaking its callback
>   context (e.g. struct pn533_data_exchange_arg, and the sock_hold()
>   reference for rawsock);
> - cmd_pending stuck at 1 makes every later command queue up without
>   ever being sent, freezing the device until unbind.
> 
> The broken error path is as old as the command queue, but it used to
> require an URB submission error to trigger; a device that stalls the
> OUT endpoint reaches it via the send timeout added by the next patch.
> 
> Call the command's completion callback with the error and reschedule
> cmd_work (unless the callback chained a direct command by returning
> -EINPROGRESS) so that every queued command completes with an error and
> the queue drains instead of freezing.
> 
> Signed-off-by: Zhao Feng <zhaofeng2@cmss.chinamobile.com>
> Cc: <stable@vger.kernel.org>

I think this patch also warrants a Fixes tag.

I don't think it's necessary to repost just to address that,
rather you can just respond to this email with the appropriate tag.

Also, I note that this patch did not apply cleanly (using git am)
and thus the CI did not run. I think this is due to minor fuzz -
git am is very fussy about that. I am unsure if there is value in
reposting to address that.

I've looked over the review of v1 and the updates made in v2.
And overall this looks good to me.

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH v2 2/2] nfc: pn533: add timeout to USB frame send completion wait

[Simon Horman]

On Tue, Oct 27, 2026 at 03:01:35PM +0800, Zhao Feng wrote:
> pn533_usb_send_frame() waits unboundedly for the out URB to complete.
> A device that never completes the transfer makes the task sleep
> forever in TASK_UNINTERRUPTIBLE while holding the command mutex, as
> reported by syzbot (hung task in pn533_usb_send_frame).
> 
> Use a 1 second timeout instead.  On timeout, kill the URB and return
> -ETIMEDOUT so the command in progress fails gracefully instead of
> hanging the task.
> 
> The transfer may complete between the timeout expiring and
> usb_kill_urb(); in that case urb->status is 0 after the kill and the
> send proceeds normally instead of reporting a spurious failure,
> mirroring what usb_start_wait_urb() does.
> 
> Fixes: 9dab880d675b ("nfc: pn533: Wait for out_urb's completion in pn533_usb_send_frame()")
> Reported-by: syzbot+1c5368988052dc2d8201@syzkaller.appspotmail.com
> Closes: https://syzbot.org/bug?extid=1c5368988052dc2d8201
> Signed-off-by: Zhao Feng <zhaofeng2@cmss.chinamobile.com>
> Cc: <stable@vger.kernel.org>
> ---
> v2: check urb->status after killing the URB on timeout and carry on if
>     the transfer completed in the race window; add Fixes tag and Cc
>     stable; the pn533_wq_cmd() error handling fix is now patch 1/2.
>     Issues reported by the Sashiko AI review.

Reviewed-by: Simon Horman <horms@kernel.org>