On Fri, 15 Jun 2012, Lan Tianyu wrote:
> >>>> +struct usb_device *usb_get_hub_child(struct usb_device *hdev,
> >>>> + int port1)
> >>>> +{
> >>>> + struct usb_hub *hub = hdev_to_hub(hdev);
> >>>> +
> >>>> + if (port1< 1 || port1> hdev->maxchild)
> >>>> + return NULL;
> >>>> + return usb_get_dev(hub->port_data[port1 - 1].child);
> >>>
> >>> What is the reason for calling usb_get_dev() here? If the old code
> >>> didn't need it, why does the new code?
> >>
> >> Just for more safe, the function return the struct usb_device and the point
> >> may be invalid caused by device being released(memory being freed). But caller
> >> may still use it. This will cause oops. Last time, we discussed this may happen
> >> in the r8a66597-hcd.c and cc the maintainer but have not got response.
> >
> > What happens if the child device is released after your code evaluates
> >
> > hub->port_data[port1 - 1].child
> >
> > but before usb_get_dev() is called? There will still be an oops.
> >
> > No, the caller of usb_get_hub_child() has to be entirely responsible
> > for making certain that children don't get released unexpectedly.
> > Calling usb_get_dev() like this won't help.
> >
> Ok. One more question, what the caller should do to avoid oops?
It depends on the context. Usually the easiest thing is to hold the
hub's device lock. Probably most of the callers already _do_ hold this
lock.
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
