On Sat, 8 May 2021 at 16:29, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: > > On Sat, May 08, 2021 at 10:14:41AM +0200, dave penkler wrote: > > On Thu, 6 May 2021 at 22:31, Guido Kiener > > <Guido.Kiener@xxxxxxxxxxxxxxxxx> wrote: > > > > > > > -----Original Message----- > > > > From: Alan Stern > > > > Sent: Thursday, May 6, 2021 8:32 PM > > > > To: Kiener Guido 14DS1 > > > > > > > > On Thu, May 06, 2021 at 05:44:55PM +0000, Guido Kiener wrote: > > > > > > -----Original Message----- > > > > > > From: Alan Stern > > > > > > Sent: Thursday, May 6, 2021 3:49 PM > > > > > > To: Kiener Guido 14DS1 <Guido.Kiener@xxxxxxxxxxxxxxxxx> > > > > > > > > > > > > > > Thanks for your assessment. I agree with the general feeling. I > > > > > > > counted about hundred specific usb drivers, so wouldn't it be > > > > > > > better to fix the > > > > > > problem in some of the host drivers (e.g. urb.c)? > > > > > > > We could return an error when calling usb_submit_urb() on an erroneous > > > > pipe. > > > > > > > I cannot estimate the side effects and we need to check all > > > > > > > drivers again how they deal with the error situation. Maybe there > > > > > > > are some special driver > > > > > > that need a specialized error handling. > > > > > > > In this case these drivers could reset the (new?) error flag to > > > > > > > allow calling usb_submit_urb() again without error. This could work, isn't it? > > > > > > > > > > > > That is feasible, although it would be an awkward approach. As you > > > > > > said, the side effects aren't clear. But it might work. > > > > > > > > > > Otherwise I see only the other approach to change hundred drivers and > > > > > add the cases EPROTO, EILSEQ and ETIME in each callback handler. The > > > > > usbtmc driver already respects the EILSEQ and ETIME, and only EPROTO is > > > > missing. > > > > > The rest should be more a management task. > > > > > BTW do you assume it is only a problem for INT pipes or is it also a > > > > > problem for isochronous and bulk transfers? > > > > > > > > All of them. Control too. > > > > > > > > > > Will you be able to test patches? > > > > > > > > > > I only can test the USBTMC function in some different PCs. I do not > > > > > have automated regression tests for USB drivers or Linux kernels. > > > > > Maybe there is company who could do that. > > > > > > > > Well then, if I do find time to write a patch, I'll ask you to try it out with the usbtmc > > > > driver. > > > > > > You mean that you will do a patch in urb.c or a host driver? Or just add a line in usbtmc.c? > > > Anyhow there is no hurry. On May 20 I will send you a mail if I'm able to > > > provoke one of these hardware errors EPROTO, EILSQ, or ETIME. Otherwise > > > it doesn't make sense to test it. > > > > > > -Guido > > > > EPROTO is a link level issue and needs to be handled by the host driver. > > Are you referring to the host controller driver, or to the class device > driver running on the host? The host controller driver is responsible > for creating the -EPROTO error code in the first place. The class > device driver is responsible for taking an appropriate action in > response. host controller driver > > > When the host driver detects a protocol error while processing an URB > > it completes the URB with EPROTO status and marks the endpoint as > > halted. > > Not true. It does not mark the endpoint as halted, not unless it > receives a STALL handshake from the device. A STALL is not a protocol > error. > > > When the class driver resubmits the URB and the if the host driver > > finds the endpoint still marked as halted it should return EPIPE > > status on the resubmitted URB > > Irrelevant. Not at all. The point is that when an application is talking to an instrument over the usbtmc driver, the underlying host controller and its driver will detect and silence a babbling endpoint. Hence no EPROTO loop will ensue in this case and therefore no changes are needed in usbtmc. > > > When the class driver and usbtmc in particular receives an URB with > > EPIPE status it cleans up and does not resubmit. > > Can someone from syzbot land please confirm whether usbtmc running on > > the xhci host driver causes an RCU stall to be detected ? > > That is not an easy thing to test, and syzbot is not capable of testing > it. You would need a USB device which could deliberately be set to > create a protocol error; I don't know of any devices like that. > > Alan Stern
