On Tue, 3 Mar 2015, Al Viro wrote:

> Looking at that thing again...  why do they need to be dummy?  After all,
> those methods start with get_ready_ep(), which will fail unless we have
> ->state == STATE_EP_ENABLED.  So they'd be failing just fine until that
> first write() anyway.  Let's do the following:

In addition to the changes you made, it looks like you will need the 
following or something similar (also untested).  I'm not sure if this 
is race-free, but it's better than before.

Alan Stern



Index: usb-3.19/drivers/usb/gadget/legacy/inode.c
===================================================================
--- usb-3.19.orig/drivers/usb/gadget/legacy/inode.c
+++ usb-3.19/drivers/usb/gadget/legacy/inode.c
@@ -987,6 +987,10 @@ ep0_read (struct file *fd, char __user *
        enum ep0_state                  state;
 
        spin_lock_irq (&dev->lock);
+       if (dev->state <= STATE_DEV_OPENED) {
+               retval = -ENODEV;
+               goto done;
+       }
 
        /* report fd mode change before acting on it */
        if (dev->setup_abort) {
@@ -1185,8 +1189,6 @@ ep0_write (struct file *fd, const char _
        struct dev_data         *dev = fd->private_data;
        ssize_t                 retval = -ESRCH;
 
-       spin_lock_irq (&dev->lock);
-
        /* report fd mode change before acting on it */
        if (dev->setup_abort) {
                dev->setup_abort = 0;
@@ -1232,7 +1234,6 @@ ep0_write (struct file *fd, const char _
        } else
                DBG (dev, "fail %s, state %d\n", __func__, dev->state);
 
-       spin_unlock_irq (&dev->lock);
        return retval;
 }
 
@@ -1240,6 +1241,10 @@ static int
 ep0_fasync (int f, struct file *fd, int on)
 {
        struct dev_data         *dev = fd->private_data;
+
+       if (dev->state <= STATE_DEV_OPENED)
+               return -ENODEV;
+
        // caller must F_SETOWN before signal delivery happens
        VDEBUG (dev, "%s %s\n", __func__, on ? "on" : "off");
        return fasync_helper (f, fd, on, &dev->fasync);
@@ -1279,6 +1284,9 @@ ep0_poll (struct file *fd, poll_table *w
        struct dev_data         *dev = fd->private_data;
        int                     mask = 0;
 
+       if (dev->state <= STATE_DEV_OPENED)
+               return -ENODEV;
+
        poll_wait(fd, &dev->wait, wait);
 
        spin_lock_irq (&dev->lock);
@@ -1314,19 +1322,6 @@ static long dev_ioctl (struct file *fd,
        return ret;
 }
 
-/* used after device configuration */
-static const struct file_operations ep0_io_operations = {
-       .owner =        THIS_MODULE,
-       .llseek =       no_llseek,
-
-       .read =         ep0_read,
-       .write =        ep0_write,
-       .fasync =       ep0_fasync,
-       .poll =         ep0_poll,
-       .unlocked_ioctl =       dev_ioctl,
-       .release =      dev_release,
-};
-
 /*----------------------------------------------------------------------*/
 
 /* The in-kernel gadget driver handles most ep0 issues, in particular
@@ -1850,6 +1845,14 @@ dev_config (struct file *fd, const char
        u32                     tag;
        char                    *kbuf;
 
+       spin_lock_irq(&dev->lock);
+       if (dev->state > STATE_DEV_OPENED) {
+               value = ep0_write(fd, buf, len, ptr);
+               spin_unlock_irq(&dev->lock);
+               return value;
+       }
+       spin_unlock_irq(&dev->lock);
+
        if (len < (USB_DT_CONFIG_SIZE + USB_DT_DEVICE_SIZE + 4))
                return -EINVAL;
 
@@ -1923,7 +1926,6 @@ dev_config (struct file *fd, const char
                 * on, they can work ... except in cleanup paths that
                 * kick in after the ep0 descriptor is closed.
                 */
-               fd->f_op = &ep0_io_operations;
                value = len;
        }
        return value;
@@ -1954,12 +1956,14 @@ dev_open (struct inode *inode, struct fi
        return value;
 }
 
-static const struct file_operations dev_init_operations = {
+static const struct file_operations ep0_operations = {
        .llseek =       no_llseek,
 
        .open =         dev_open,
+       .read =         ep0_read,
        .write =        dev_config,
        .fasync =       ep0_fasync,
+       .poll =         ep0_poll,
        .unlocked_ioctl = dev_ioctl,
        .release =      dev_release,
 };
@@ -2075,7 +2079,7 @@ gadgetfs_fill_super (struct super_block
                goto Enomem;
 
        dev->sb = sb;
-       dev->dentry = gadgetfs_create_file(sb, CHIP, dev, &dev_init_operations);
+       dev->dentry = gadgetfs_create_file(sb, CHIP, dev, &ep0_operations);
        if (!dev->dentry) {
                put_dev(dev);
                goto Enomem;

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to