* Re: [PATCH] Input: yealink - stop URB resubmission on completion error
2026-06-16 11:04 ` [PATCH] Input: yealink - stop URB resubmission on completion error Jie Wang
@ 2026-06-16 3:31 ` sashiko-bot
0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-06-16 3:31 UTC (permalink / raw)
To: Jie Wang; +Cc: linux-input, dmitry.torokhov
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] The URB completion callbacks return early on any non-zero URB status, preventing resubmission on transient errors (like -EPROTO or -EILSEQ) and permanently breaking the device's polling loop.
Pre-existing issues:
- [High] Concurrent accesses to `yld->ctl_data` from sysfs and the asynchronous URB polling loop cause DMA buffer corruption, and passing a coherent DMA buffer to a synchronous USB API violates DMA mappings.
- [High] Ping-pong URBs can escape `usb_kill_urb()` during `input_close()`, leading to continuous background execution and use-after-free.
--
commit 02a6ac0b5f7fd710af199370b924a27f34ca574c
Author: Jie Wang <jie.wang@intel.com>
Input: yealink - stop URB resubmission on completion error
urb_irq_callback() and urb_ctl_callback() resubmit each other's URBs
regardless of completion status. When the device returns a persistent
error (-EPROTO), this creates an unthrottled resubmission loop that
starves the CPU and triggers RCU stalls.
Stop resubmitting on any non-zero URB status, following the standard
completion pattern used by other USB input drivers.
Reported-by: syzbot+039eab266c6321a174bd@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=039eab266c6321a174bd
Signed-off-by: Jie Wang <jie.wang@intel.com>
> diff --git a/drivers/input/misc/yealink.c b/drivers/input/misc/yealink.c
> index 560f895a00cda..fcf66912f86b6 100644
> --- a/drivers/input/misc/yealink.c
> +++ b/drivers/input/misc/yealink.c
> @@ -405,9 +405,20 @@ static void urb_irq_callback(struct urb *urb)
> struct yealink_dev *yld = urb->context;
> int ret, status = urb->status;
>
> - if (status)
> + switch (status) {
> + case 0:
> + break;
> + case -ECONNRESET:
> + case -ENOENT:
> + case -ESHUTDOWN:
> + dev_dbg(&yld->intf->dev, "%s - urb shutting down with status %d\n",
> + __func__, status);
> + return;
> + default:
> dev_err(&yld->intf->dev, "%s - urb status %d\n",
> __func__, status);
> + return;
> + }
[Severity: High]
Does returning early on any non-zero status prevent resubmission on transient
errors?
If an error like -EPROTO or -EILSEQ is encountered due to electrical noise,
this early return completely drops the URB. Because the driver has no backoff
timer or error retry mechanism, a single transient error will permanently stop
the polling state machine. Sysfs updates and key presses will silently be
ignored until the device is physically replugged.
[ ... ]
> @@ -443,9 +454,20 @@ static void urb_ctl_callback(struct urb *urb)
> struct yealink_dev *yld = urb->context;
> int ret = 0, status = urb->status;
>
> - if (status)
> + switch (status) {
> + case 0:
> + break;
> + case -ECONNRESET:
> + case -ENOENT:
> + case -ESHUTDOWN:
> + dev_dbg(&yld->intf->dev, "%s - urb shutting down with status %d\n",
> + __func__, status);
> + return;
> + default:
> dev_err(&yld->intf->dev, "%s - urb status %d\n",
> __func__, status);
> + return;
> + }
[Severity: High]
Will the same issue happen here, breaking the ping-pong URB loop on transient
USB errors and permanently freezing the device state machine?
[Severity: High]
This is a pre-existing issue, but does concurrent access to the shared control
buffer from sysfs cause DMA corruption?
Looking at yealink_set_ringtone(), it modifies the shared control buffer
without locking:
drivers/input/misc/yealink.c:yealink_set_ringtone() {
...
memset(yld->ctl_data, 0, sizeof(*(yld->ctl_data)));
yld->ctl_data->cmd = CMD_RING_VOLUME;
...
}
Since yld->ctl_data is used as the dedicated DMA transfer buffer for the
asynchronous control URB which runs continuously in a background loop, modifying
it from sysfs without locking against the async loop could corrupt an active DMA
transfer.
Additionally, yealink_set_ringtone() synchronously sends this buffer:
drivers/input/misc/yealink.c:yealink_cmd() {
...
return usb_control_msg(yld->udev,
usb_sndctrlpipe(yld->udev, 0),
USB_REQ_SET_CONFIGURATION,
USB_TYPE_CLASS | USB_RECIP_INTERFACE | USB_DIR_OUT,
0x200, 3,
p, sizeof(*p),
USB_CTRL_SET_TIMEOUT);
}
Because yld->ctl_data is allocated with usb_alloc_coherent(), does this violate
the DMA API by attempting to dma_map_single() already-coherent memory?
[Severity: High]
This is a pre-existing issue, but can ping-pong URBs escape the shutdown
sequence and cause a use-after-free?
The driver attempts to stop the ping-pong URBs in input_close() by setting a
flag and killing them:
drivers/input/misc/yealink.c:input_close() {
...
yld->shutdown = 1;
usb_kill_urb(yld->urb_ctl);
usb_kill_urb(yld->urb_irq);
...
}
If urb_irq_callback() evaluates !yld->shutdown as true but is preempted:
drivers/input/misc/yealink.c:urb_irq_callback() {
...
if (!yld->shutdown) {
ret = usb_submit_urb(yld->urb_ctl, GFP_ATOMIC);
...
}
And input_close() then sets shutdown = 1, kills urb_ctl (which isn't in flight
yet), and blocks killing urb_irq. When urb_irq_callback() resumes, it submits
urb_ctl. Because usb_kill_urb(urb_ctl) already returned and cleared the reject
count, the submission succeeds.
If the device is disconnected, usb_cleanup() frees the yealink_dev structure,
but the runaway URB will dereference it upon completion. Should this use
usb_poison_urb() instead to reliably stop the loop?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260616110440.1073059-1-jie.wang@intel.com?part=1
^ permalink raw reply [flat|nested] 2+ messages in thread