Am Mittwoch, den 06.06.2012, 22:28 +0800 schrieb Ming Lei:
> On Wed, Jun 6, 2012 at 9:23 PM, <stefani@xxxxxxxxxxx> wrote:
> > From: Stefani Seibold <stefani@xxxxxxxxxxx>
> >
> > This is a fix for the USB skeleton driver to bring it in shape.
> >
> > - The usb_interface structure pointer will be no longer stored
> > - Every access to the USB will be handled trought the usb_interface pointer
> > - Add a new bool 'connected' for signaling a disconnect (== false)
> > - Handle a non blocking read without blocking
> > - Code cleanup
> > - Synchronize disconnect() handler with open() and release(), to fix races
> > - Introduced fsync
> > - Single user mode
> > - More code cleanup :-)
> > - Save some bytes in the dev structure
>
> So many purposes, you need to split your patches for review easily, :-)
>
This is the next step :-)
> > static void skel_delete(struct kref *kref)
> > {
> > @@ -86,15 +90,13 @@ static int skel_open(struct inode *inode, struct file *file)
> > {
> > struct usb_skel *dev;
> > struct usb_interface *interface;
> > - int subminor;
> > - int retval = 0;
> > + int retval;
> >
> > - subminor = iminor(inode);
> > + /* lock against skel_disconnect() */
> > + mutex_lock(&sync_mutex);
>
> This one is not needed since we have minor_rwsem(drivers/usb/core/file.c)
> to avoid the race.
>
The mutex is not for the minor handling, it is for the disconnect(). As
mentioned in the previous posting, there is a race betwenn open() and
connect().
Oliver told me that a interface pointer can be already used by an other
driver when the disconnect() was called.
So the interface will be used to determinate the dev pointer, which can
at this time also owned by an other driver.
And at last the dev pointer could be point to an already released
memory.
So it is IMHO needed.
> >
> > static int skel_release(struct inode *inode, struct file *file)
> > {
> > - struct usb_skel *dev;
> > -
> > - dev = file->private_data;
> > - if (dev == NULL)
> > - return -ENODEV;
> > + struct usb_skel *dev = file->private_data;
> >
> > + /* lock against skel_disconnect() */
> > + mutex_lock(&sync_mutex);
>
> Since the reference count is held now, so is there any race between
> release and disconnect?
>
Same as above, the interface could be owned by an other driver.
Thanks,
Stefani
--
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
