于 2012年12月16日 01:04, Alan Stern 写道:
> On Sat, 15 Dec 2012, Chen Gang wrote:
> 
>> > 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.
> It isn't.  ohci_urb_dequeue calls usb_hcd_check_unlink_urb before 
> looking at urb->hcpriv.  If urb_free_priv has been called already then 
> usb_hcd_check_unlink_urb will return a non-zero value.  This is because 
> finish_urb calls usb_hcd_unlink_urb_from_ep.
> 

  for ohci_urb_dequeue, it is surely as what you said above.
  can you be sure that another (finish_unlinks, takeback_td...) also consider 
it ?
  it seems we have already considered, but I am not quite sure
    (since we already agree to sending a patch, it is not necessary to reply).

> Also, don't forget that the first think usb_hcd_giveback_urb does is 
> set urb->hcpriv to NULL.
> 

  in finish_urb:
    when call usb_hcd_giveback_urb, need unlock firstly (lock again, after 
finish).
    kfree urb->hcpriv in lock protected, but set it NULL out of lock protected.
      (at least, it seems not a good idea)
      (since we already agree to sending a patch, it is not necessary to reply).

>> >   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.
> If you want to send in a patch to do this, go ahead.
>

  ok, I will send (although it may be not a bug, better to do it)

  excuse me, please wait for some days (I should construct environments for 
test it).

  thanks.
 
> Alan Stern
> 
> 
> 


-- 
Chen Gang

Asianux Corporation
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to