Re: [PATCH] Input: yealink - stop URB resubmission on completion error

Re: [PATCH] Input: yealink - stop URB resubmission on completion error

[sashiko-bot]

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

[PATCH] Input: yealink - stop URB resubmission on completion error

[Jie Wang]

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>
---
 drivers/input/misc/yealink.c | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/drivers/input/misc/yealink.c b/drivers/input/misc/yealink.c
index 8786ed8b3565..bbfa1a9c23d1 100644
--- a/drivers/input/misc/yealink.c
+++ b/drivers/input/misc/yealink.c
@@ -414,9 +414,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;
+	}

 	switch (yld->irq_data->cmd) {
 	case CMD_KEYPRESS:
@@ -452,9 +463,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;
+	}

 	switch (yld->ctl_data->cmd) {
 	case CMD_KEYPRESS:
--
2.34.1

Re: [PATCH] Input: yealink - stop URB resubmission on completion error

[Dmitry Torokhov]

Hi Jie,

On Tue, Jun 16, 2026 at 11:04:40AM +0000, Jie Wang wrote:
> 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>

It would be good to have a helper like

bool usb_is_permanent_error(struct urb *urb, struct device *dev);

instead of having this "switch" in all the drivers. It also unclear
whether we should not attempt to re-submit the URB on errors other than
ECONNRESET, ENOENT, or ESHUTDOWN.

> ---
>  drivers/input/misc/yealink.c | 26 ++++++++++++++++++++++----
>  1 file changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/input/misc/yealink.c b/drivers/input/misc/yealink.c
> index 8786ed8b3565..bbfa1a9c23d1 100644
> --- a/drivers/input/misc/yealink.c
> +++ b/drivers/input/misc/yealink.c
> @@ -414,9 +414,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;
> +	}
> 
>  	switch (yld->irq_data->cmd) {
>  	case CMD_KEYPRESS:
> @@ -452,9 +463,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;
> +	}
> 
>  	switch (yld->ctl_data->cmd) {
>  	case CMD_KEYPRESS:
> --
> 2.34.1

Thanks.

-- 
Dmitry

RE: [PATCH] Input: yealink - stop URB resubmission on completion error

[Wang, Jie]

Hi Dmitry,

Thank you for the review.

> -----Original Message-----
> From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Sent: Wednesday, June 17, 2026 3:17 AM
> To: Wang, Jie <jie.wang@intel.com>
> Cc: Henk Vergonet <Henk.Vergonet@gmail.com>; linux-input@vger.kernel.org;
> linux-kernel@vger.kernel.org;
> syzbot+039eab266c6321a174bd@syzkaller.appspotmail.com; syzkaller-
> bugs@googlegroups.com
> Subject: Re: [PATCH] Input: yealink - stop URB resubmission on completion error
> 
> Hi Jie,
> 
> On Tue, Jun 16, 2026 at 11:04:40AM +0000, Jie Wang wrote:
> > 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>
> 
> It would be good to have a helper like
> 
> bool usb_is_permanent_error(struct urb *urb, struct device *dev);
> 
> instead of having this "switch" in all the drivers. It also unclear whether we should
> not attempt to re-submit the URB on errors other than ECONNRESET, ENOENT, or
> ESHUTDOWN.
> 

For the helper - how about a local yealink_urb_check_status() in v2?
A usb_is_permanent_error() in usb.h would benefit more drivers
across the tree, but that's a larger effort best done separately.

For resubmission - I'm thinking stop immediately on -ECONNRESET,
-ENOENT, -ESHUTDOWN, and for 
everything else retry up to 50 consecutive errors before giving up.
Counter resets on any successful completion, so transient bus glitches
recover but a dead device won't spin the CPU forever.

Let me know if that sounds reasonable and I'll send v2.

> > ---
> >  drivers/input/misc/yealink.c | 26 ++++++++++++++++++++++----
> >  1 file changed, 22 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/input/misc/yealink.c
> > b/drivers/input/misc/yealink.c index 8786ed8b3565..bbfa1a9c23d1 100644
> > --- a/drivers/input/misc/yealink.c
> > +++ b/drivers/input/misc/yealink.c
> > @@ -414,9 +414,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;
> > +	}
> >
> >  	switch (yld->irq_data->cmd) {
> >  	case CMD_KEYPRESS:
> > @@ -452,9 +463,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;
> > +	}
> >
> >  	switch (yld->ctl_data->cmd) {
> >  	case CMD_KEYPRESS:
> > --
> > 2.34.1
> 
> Thanks.
> 
> --
> Dmitry

Re: [PATCH] Input: yealink - stop URB resubmission on completion error

[Dmitry Torokhov]

On Wed, Jun 17, 2026 at 05:49:07AM +0000, Wang, Jie wrote:
> Hi Dmitry,
> 
> Thank you for the review.
> 
> > -----Original Message-----
> > From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > Sent: Wednesday, June 17, 2026 3:17 AM
> > To: Wang, Jie <jie.wang@intel.com>
> > Cc: Henk Vergonet <Henk.Vergonet@gmail.com>; linux-input@vger.kernel.org;
> > linux-kernel@vger.kernel.org;
> > syzbot+039eab266c6321a174bd@syzkaller.appspotmail.com; syzkaller-
> > bugs@googlegroups.com
> > Subject: Re: [PATCH] Input: yealink - stop URB resubmission on completion error
> > 
> > Hi Jie,
> > 
> > On Tue, Jun 16, 2026 at 11:04:40AM +0000, Jie Wang wrote:
> > > 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>
> > 
> > It would be good to have a helper like
> > 
> > bool usb_is_permanent_error(struct urb *urb, struct device *dev);
> > 
> > instead of having this "switch" in all the drivers. It also unclear whether we should
> > not attempt to re-submit the URB on errors other than ECONNRESET, ENOENT, or
> > ESHUTDOWN.
> > 
> 
> For the helper - how about a local yealink_urb_check_status() in v2?
> A usb_is_permanent_error() in usb.h would benefit more drivers
> across the tree, but that's a larger effort best done separately.

I do not think we are in a rush to fix this: the driver is old and I
have no idea how many users actually have the device.

> 
> For resubmission - I'm thinking stop immediately on -ECONNRESET,
> -ENOENT, -ESHUTDOWN, and for 
> everything else retry up to 50 consecutive errors before giving up.
> Counter resets on any successful completion, so transient bus glitches
> recover but a dead device won't spin thThat is probably reasonable.e CPU forever.

That is sounds reasonable.


Thanks.

-- 
Dmitry