On Fri, Mar 23, 2012 at 3:50 AM, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote: > This patch (as1534) updates the documentation for usb_unlink_urb and > related functions. It explains that the caller must prevent the URB > being unlinked from getting deallocated while the unlink is taking > place. > > Signed-off-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> > CC: Ming Lei <tom.leiming@xxxxxxxxx> > > --- > > Documentation/usb/URB.txt | 13 +++++++++++++ > drivers/usb/core/urb.c | 12 ++++++++++++ > 2 files changed, 25 insertions(+) > > Index: usb-3.3/Documentation/usb/URB.txt > =================================================================== > --- usb-3.3.orig/Documentation/usb/URB.txt > +++ usb-3.3/Documentation/usb/URB.txt > @@ -168,6 +168,19 @@ that if the completion handler or anyone > they will get a -EPERM error. Thus you can be sure that when > usb_kill_urb() returns, the URB is totally idle. > > +There is a lifetime issue to consider. An URB may complete at any > +time, and the completion callback may free the URB. If this happens > +while usb_unlink_urb or usb_kill_urb is running, it will cause a > +memory-access violation. To prevent an URB from being deallocated at > +the wrong time, you can increment its reference count temporarily by > +calling > + > + struct urb *usb_get_urb(struct urb *urb) > + > +(ignore the return value; it is the same as the argument) before > +usb_unlink_urb and then decrement the reference count afterward by Calling usb_get_urb simply before usb_unlink_urb is not enough, the driver should make sure that the URB can't be freed during usb_get_urb. URB may be freed in .complete or other context triggered by .complete. So looks the point should be added. > +calling usb_free_urb. > + > > 1.7. What about the completion handler? > > Index: usb-3.3/drivers/usb/core/urb.c > =================================================================== > --- usb-3.3.orig/drivers/usb/core/urb.c > +++ usb-3.3/drivers/usb/core/urb.c > @@ -536,6 +536,10 @@ EXPORT_SYMBOL_GPL(usb_submit_urb); > * never submitted, or it was unlinked before, or the hardware is already > * finished with it), even if the completion handler has not yet run. > * > + * The URB must not be deallocated while this routine is running. If the > + * completion handler might free the URB, the driver should avoid problems by > + * calling usb_get_urb() before this routine and usb_free_urb() afterwards. Same with above. > + * > * Unlinking and Endpoint Queues: > * > * [The behaviors and guarantees described below do not apply to virtual > @@ -600,6 +604,10 @@ EXPORT_SYMBOL_GPL(usb_unlink_urb); > * with error -EPERM. Thus even if the URB's completion handler always > * tries to resubmit, it will not succeed and the URB will become idle. > * > + * The URB must not be deallocated while this routine is running. If the > + * completion handler might free the URB, the driver should avoid problems by > + * calling usb_get_urb() before this routine and usb_free_urb() afterwards. Same with above. > + * > * This routine may not be used in an interrupt context (such as a bottom > * half or a completion handler), or when holding a spinlock, or in other > * situations where the caller can't schedule(). > @@ -637,6 +645,10 @@ EXPORT_SYMBOL_GPL(usb_kill_urb); > * with error -EPERM. Thus even if the URB's completion handler always > * tries to resubmit, it will not succeed and the URB will become idle. > * > + * The URB must not be deallocated while this routine is running. If the > + * completion handler might free the URB, the driver should avoid problems by > + * calling usb_get_urb() before this routine and usb_free_urb() afterwards. Same with above. > + * > * This routine may not be used in an interrupt context (such as a bottom > * half or a completion handler), or when holding a spinlock, or in other > * situations where the caller can't schedule(). > -- Ming Lei -- 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
