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@xxxxxxxxx>
> Sent: Wednesday, June 17, 2026 3:17 AM
> To: Wang, Jie <jie.wang@xxxxxxxxx>
> Cc: Henk Vergonet <Henk.Vergonet@xxxxxxxxx>; linux-input@xxxxxxxxxxxxxxx;
> linux-kernel@xxxxxxxxxxxxxxx;
> syzbot+039eab266c6321a174bd@xxxxxxxxxxxxxxxxxxxxxxxxx; syzkaller-
> bugs@xxxxxxxxxxxxxxxx
> 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@xxxxxxxxxxxxxxxxxxxxxxxxx
> > Closes: https://syzkaller.appspot.com/bug?extid=039eab266c6321a174bd
> > Signed-off-by: Jie Wang <jie.wang@xxxxxxxxx>
> 
> 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