Hello Alan Stern:
in drivers/usb/host/ohci-q.c, function finish_urb:
when we finish call urb_free_priv, we not set urb->hcpriv = NULL (line 46)
urb_free_priv call kfree for urb_priv (which is urb->hcpriv, line 29)
within finish_urb, we not set urb->hcpriv = NULL (line 43..81)
in drivers/usb/host/ohci-hdc.c, function ohci_urb_dequeue:
it judges urb->hcpriv == NULL to decide whether call finish_urb (line 317..322)
within ohci_urb_dequeue, we not set urb->hcpriv = NULL (line 321..326)
another functions (finish_unlinks, takeback_td...), can call finish_urb, too.
they do not set urb->hcpriv = NULL, either.
they do not judge urb->hcpriv == NULL before calling finish_urb.
although I can not say it is surely a bug.
I still suggest:
in finish_urb, set urb->hcpriv = NULL, after finish calling urb_free_priv.
in urb_free_priv, better to judge whether urb_priv == NULL, firstly.
thanks.
gchen.
drivers/usb/host/ohci-q.c:
13 static void urb_free_priv (struct ohci_hcd *hc, urb_priv_t *urb_priv)
14 {
15 int last = urb_priv->length - 1;
16
17 if (last >= 0) {
18 int i;
19 struct td *td;
20
21 for (i = 0; i <= last; i++) {
22 td = urb_priv->td [i];
23 if (td)
24 td_free (hc, td);
25 }
26 }
27
28 list_del (&urb_priv->pending);
29 kfree (urb_priv);
30 }
31
32 /*-------------------------------------------------------------------------*/
33
34 /*
35 * URB goes back to driver, and isn't reissued.
36 * It's completely gone from HC data structures.
37 * PRECONDITION: ohci lock held, irqs blocked.
38 */
39 static void
40 finish_urb(struct ohci_hcd *ohci, struct urb *urb, int status)
41 __releases(ohci->lock)
42 __acquires(ohci->lock)
43 {
44 // ASSERT (urb->hcpriv != 0);
45
46 urb_free_priv (ohci, urb->hcpriv);
47 if (likely(status == -EINPROGRESS))
48 status = 0;
49
50 switch (usb_pipetype (urb->pipe)) {
51 case PIPE_ISOCHRONOUS:
52 ohci_to_hcd(ohci)->self.bandwidth_isoc_reqs--;
53 if (ohci_to_hcd(ohci)->self.bandwidth_isoc_reqs == 0) {
54 if (quirk_amdiso(ohci))
55 usb_amd_quirk_pll_enable();
56 if (quirk_amdprefetch(ohci))
57 sb800_prefetch(ohci, 0);
58 }
59 break;
60 case PIPE_INTERRUPT:
61 ohci_to_hcd(ohci)->self.bandwidth_int_reqs--;
62 break;
63 }
64
65 #ifdef OHCI_VERBOSE_DEBUG
66 urb_print(urb, "RET", usb_pipeout (urb->pipe), status);
67 #endif
68
69 /* urb->complete() can reenter this HCD */
70 usb_hcd_unlink_urb_from_ep(ohci_to_hcd(ohci), urb);
71 spin_unlock (&ohci->lock);
72 usb_hcd_giveback_urb(ohci_to_hcd(ohci), urb, status);
73 spin_lock (&ohci->lock);
74
75 /* stop periodic dma if it's not needed */
76 if (ohci_to_hcd(ohci)->self.bandwidth_isoc_reqs == 0
77 && ohci_to_hcd(ohci)->self.bandwidth_int_reqs == 0) {
78 ohci->hc_control &= ~(OHCI_CTRL_PLE|OHCI_CTRL_IE);
79 ohci_writel (ohci, ohci->hc_control, &ohci->regs->control);
80 }
81 }
...
in drivers/usb/host/ohci-hcd.c
290 static int ohci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
291 {
292 struct ohci_hcd *ohci = hcd_to_ohci (hcd);
293 unsigned long flags;
294 int rc;
295
296 #ifdef OHCI_VERBOSE_DEBUG
297 urb_print(urb, "UNLINK", 1, status);
298 #endif
299
300 spin_lock_irqsave (&ohci->lock, flags);
301 rc = usb_hcd_check_unlink_urb(hcd, urb, status);
302 if (rc) {
303 ; /* Do nothing */
304 } else if (ohci->rh_state == OHCI_RH_RUNNING) {
305 urb_priv_t *urb_priv;
306
307 /* Unless an IRQ completed the unlink while it was being
308 * handed to us, flag it for unlink and giveback, and force
309 * some upcoming INTR_SF to call finish_unlinks()
310 */
311 urb_priv = urb->hcpriv;
312 if (urb_priv) {
313 if (urb_priv->ed->state == ED_OPER)
314 start_ed_unlink (ohci, urb_priv->ed);
315 }
316 } else {
317 /*
318 * with HC dead, we won't respect hc queue pointers
319 * any more ... just clean up every urb's memory.
320 */
321 if (urb->hcpriv)
322 finish_urb(ohci, urb, status);
323 }
324 spin_unlock_irqrestore (&ohci->lock, flags);
325 return rc;
326 }
327
--
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
