Re: [PATCH 4.19.y 2/3] usb: dwc3: gadget: prevent dwc3_request from being queued twice

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Jul 29, 2019 at 05:06:13PM +0000, Gopal, Saranya wrote:
> > On Mon, Jul 29, 2019 at 07:13:38PM +0530, Saranya Gopal wrote:
> > > From: Felipe Balbi <felipe.balbi@xxxxxxxxxxxxxxx>
> > >
> > > [Upstream commit b2b6d601365a1acb90b87c85197d79]
> > >
> > > Queueing the same request twice can introduce hard-to-debug
> > > problems. At least one function driver - Android's f_mtp.c - is known
> > > to cause this problem.
> > >
> > > While that function is out-of-tree, this is a problem that's easy
> > > enough to avoid.
> > >
> > > Signed-off-by: Felipe Balbi <felipe.balbi@xxxxxxxxxxxxxxx>
> > > Signed-off-by: Saranya Gopal <saranya.gopal@xxxxxxxxx>
> > > ---
> > >  drivers/usb/dwc3/gadget.c | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > >
> > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> > > index 3f337a0..a56a92a 100644
> > > --- a/drivers/usb/dwc3/gadget.c
> > > +++ b/drivers/usb/dwc3/gadget.c
> > > @@ -1291,6 +1291,11 @@ static int __dwc3_gadget_ep_queue(struct
> > dwc3_ep *dep, struct dwc3_request *req)
> > >  				&req->request, req->dep->name))
> > >  		return -EINVAL;
> > >
> > > +	if (WARN(req->status < DWC3_REQUEST_STATUS_COMPLETED,
> > > +				"%s: request %pK already in flight\n",
> > > +				dep->name, &req->request))
> > > +		return -EINVAL;
> > 
> > So we are going to trip syzbot up on this out-of-tree driver?  Brave...
> 
> I had retained the commit message from the upstream commit.
> However, without this patch, I see issues with adb as well.
> Adb will hang after adb root/unroot command and needs a reboot to recover.

So you see huge WARN dumps in the log?

That's fine, but be aware, if userspace can trigger this, then syzbot
will trigger it, and any system running 'panic on warn' will crash.

If this is something that we normally have to catch and handle, WARN()
is not how to do it.  But we should fix that upstream, not here.

thanks,

greg k-h



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux