On Wed, 6 Mar 2013, Paul Zimmerman wrote:
> > BTW, you _do_ have a race because you don't check ep->hcpriv when
> > ->urb_enqueue() is called. This means that you could queue to an
> > endpoint which is in the process of getting disabled. The urb would
> > either be lost or completed before being started (if I read the code
> > correctly).
>
> Good spotting. Yes, I need to recheck ep->hcpriv after reacquiring
> the spinlock, thanks. Interestingly, ehci-hcd and ohci-hcd do that,
> but uhci-hcd does not. I wonder if that's a bug?
I'm not sure what you mean. uhci-hcd does this:
static int uhci_urb_enqueue(struct usb_hcd *hcd,
struct urb *urb, gfp_t mem_flags)
{
int ret;
struct uhci_hcd *uhci = hcd_to_uhci(hcd);
unsigned long flags;
struct urb_priv *urbp;
struct uhci_qh *qh;
spin_lock_irqsave(&uhci->lock, flags);
========^
ret = usb_hcd_link_urb_to_ep(hcd, urb);
if (ret)
goto done_not_linked;
ret = -ENOMEM;
urbp = uhci_alloc_urb_priv(uhci, urb);
if (!urbp)
goto done;
if (urb->ep->hcpriv)
========^
qh = urb->ep->hcpriv;
else {
qh = uhci_alloc_qh(uhci, urb->dev, urb->ep);
if (!qh)
goto err_no_qh;
}
Did you mean that ohci-hcd doesn't check? Yes, that's a mistake, maybe
even a bug.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
