On Wed, 1 Jun 2005, Dmitry Torokhov wrote:

> On 6/1/05, Alan Stern <[EMAIL PROTECTED]> wrote:
> > On Wed, 1 Jun 2005, Dmitry Torokhov wrote:
> > 
> > > > +static ssize_t show_tabletSize(struct device *dev, char *buf)
> > > > +{
> > > > +       struct usb_acecad *acecad = dev_get_drvdata(dev);
> > > > +
> > > > +       if (acecad == NULL)
> > > > +               return 0;
> > >
> > > This check - is it really needed? Attributes are deleted in
> > > disconnect... Wait, this needs locking because if someone happens to
> > > read one of the attributes while usb_acecad_disconnect is running you
> > > may end up accessing just freed memory, no?
> > 
> > No.
> > 
> > Attributes are complex things.  When an attribute is opened the sysfs code
> > does kobject_get on the underlying object.  The release method won't get
> > called, and hence the memory won't get freed, until the attribute is
> > closed.
> > 
> > On the other hand, remove methods typically do call dev_set_drvdata(dev,
> > NULL).  Thus a read-during-disconnect might very well end up with acecad
> > == NULL, and so the check above is necessary.
> > 
> 
> OK, consider this:
> 
> CPU1:                                CPU2
> 
> if (acecad == NULL) // fails
>                                               usb_set_intfdata(intf, NULL);
>                                               ...
>                                               kfree(acecad);
> 
> sprintf(... acecad->blah) -> oops
> 
> Unprotected check does not help and might as well be removed, with
> proper locking in place it is not needed either.

You are right and I was wrong.

It's not so easy to synchronize operations on an attribute with driver 
operations if the driver doesn't own the attribute's kobject.  In this 
case there would have to be a single driver-wide semaphore protecting 
intf->dev.driver_data for all interfaces controlled by the driver.  And 
even that could theoretically fail if more than one driver was capable of 
binding to the interface.

The check _is_ still needed, however, since it is always possible for the
attribute's read method to be called at an arbitrary time following the
return of the disconnect routine.

Alan Stern



-------------------------------------------------------
This SF.Net email is sponsored by Yahoo.
Introducing Yahoo! Search Developer Network - Create apps using Yahoo!
Search APIs Find out how you can build Yahoo! directly into your own
Applications - visit http://developer.yahoo.net/?fr=offad-ysdn-ostg-q22005
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to