On Tue, Jun 11, 2024 at 03:09:32PM -0400, Alan Stern wrote:
> On Tue, Jun 11, 2024 at 10:35:12AM -0400, Alan Stern wrote:
> > Greg, Oliver, or anyone else:
> >
> > Questions:
> >
> > If a broken or malicious device causes a USB class driver to add a
> > thousand (or more) error messages per second to the kernel log,
> > indefinitely, would that be considered a form of DOS?
> >
> > Should the driver be fixed?
> >
> > What is an acceptable rate for an unending stream of error messages?
> > Once a second? Once a minute?
> >
> > At what point should the driver give up and stop trying to communicate
> > with the device?
> >
> > (These are not moot questions. There are indeed drivers, and probably
> > not just in the USB subsystem, subject to this sort of behavior.)
>
> Along those lines, what do you think of the following patch for handling
> -EPROTO, -EILSEQ, or -ETIME status values for the interrupt URB in the
> cdc-wdm driver? After one of those errors, the URB is immediately
> resubmitted, so the error is likely to occur again no more than a
> millisecond later. Changing dev_err() to dev_dbg() prevents log
> spamming.
>
> Alternatively, the driver could avoid resubmitting the URB when one of
> those errors occurs. This is perhaps less appropriate, because these
> kinds of errors can be transient (although that is normally rare).
>
> Alan Stern
>
>
>
> Index: usb-devel/drivers/usb/class/cdc-wdm.c
> ===================================================================
> --- usb-devel.orig/drivers/usb/class/cdc-wdm.c
> +++ usb-devel/drivers/usb/class/cdc-wdm.c
> @@ -266,14 +266,14 @@ static void wdm_int_callback(struct urb
> dev_err(&desc->intf->dev, "Stall on int endpoint\n");
> goto sw; /* halt is cleared in work */
> default:
> - dev_err(&desc->intf->dev,
> + dev_dbg(&desc->intf->dev,
> "nonzero urb status received: %d\n", status);
dev_err_ratelimited() maybe instead?
> break;
> }
> }
>
> if (urb->actual_length < sizeof(struct usb_cdc_notification)) {
> - dev_err(&desc->intf->dev, "wdm_int_callback - %d bytes\n",
> + dev_dbg(&desc->intf->dev, "wdm_int_callback - %d bytes\n",
Same here?
thanks,
greg k-h
