Skip to content

Commit d4461a6

Browse files
author
Al Viro
committed
gadgetfs: get rid of flipping ->f_op in ep_config()
Final 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: * get_ready_ep() gets a new argument - true when called from ep_write_iter(), false otherwise. * make it quiet when it finds STATE_EP_READY (no printk, that is; the case won't be impossible after that change). * when that new argument is true, treat STATE_EP_READY the same way as STATE_EP_ENABLED (i.e. return zero and do not unlock). * in ep_write_iter(), after success of get_ready_ep() turn if (!usb_endpoint_dir_in(&epdata->desc)) { into if (epdata->state == STATE_EP_ENABLED && !usb_endpoint_dir_in(&epdata->desc)) { - that logics only applies after config. * have ep_config() take kernel-side buffer (i.e. use memcpy() instead of copy_from_user() in there) and in the "let's call ep_io or ep_aio" (again, in ep_write_iter()) add "... or ep_config() in case it's not configured yet" Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
1 parent 7fe3976 commit d4461a6

File tree

1 file changed

+37
-53
lines changed

1 file changed

+37
-53
lines changed

‎drivers/usb/gadget/legacy/inode.c

Lines changed: 37 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,8 @@ MODULE_DESCRIPTION (DRIVER_DESC);
7474
MODULE_AUTHOR ("David Brownell");
7575
MODULE_LICENSE ("GPL");
7676

77+
static int ep_open(struct inode *, struct file *);
78+
7779

7880
/*----------------------------------------------------------------------*/
7981

@@ -283,14 +285,15 @@ static void epio_complete (struct usb_ep *ep, struct usb_request *req)
283285
* still need dev->lock to use epdata->ep.
284286
*/
285287
static int
286-
get_ready_ep (unsigned f_flags, struct ep_data *epdata)
288+
get_ready_ep (unsigned f_flags, struct ep_data *epdata, bool is_write)
287289
{
288290
int val;
289291

290292
if (f_flags & O_NONBLOCK) {
291293
if (!mutex_trylock(&epdata->lock))
292294
goto nonblock;
293-
if (epdata->state != STATE_EP_ENABLED) {
295+
if (epdata->state != STATE_EP_ENABLED &&
296+
(!is_write || epdata->state != STATE_EP_READY)) {
294297
mutex_unlock(&epdata->lock);
295298
nonblock:
296299
val = -EAGAIN;
@@ -305,18 +308,20 @@ get_ready_ep (unsigned f_flags, struct ep_data *epdata)
305308

306309
switch (epdata->state) {
307310
case STATE_EP_ENABLED:
311+
return 0;
312+
case STATE_EP_READY: /* not configured yet */
313+
if (is_write)
314+
return 0;
315+
// FALLTHRU
316+
case STATE_EP_UNBOUND: /* clean disconnect */
308317
break;
309318
// case STATE_EP_DISABLED: /* "can't happen" */
310-
// case STATE_EP_READY: /* "can't happen" */
311319
default: /* error! */
312320
pr_debug ("%s: ep %p not available, state %d\n",
313321
shortname, epdata, epdata->state);
314-
// FALLTHROUGH
315-
case STATE_EP_UNBOUND: /* clean disconnect */
316-
val = -ENODEV;
317-
mutex_unlock(&epdata->lock);
318322
}
319-
return val;
323+
mutex_unlock(&epdata->lock);
324+
return -ENODEV;
320325
}
321326

322327
static ssize_t
@@ -390,7 +395,7 @@ static long ep_ioctl(struct file *fd, unsigned code, unsigned long value)
390395
struct ep_data *data = fd->private_data;
391396
int status;
392397

393-
if ((status = get_ready_ep (fd->f_flags, data)) < 0)
398+
if ((status = get_ready_ep (fd->f_flags, data, false)) < 0)
394399
return status;
395400

396401
spin_lock_irq (&data->dev->lock);
@@ -572,7 +577,7 @@ ep_read_iter(struct kiocb *iocb, struct iov_iter *to)
572577
ssize_t value;
573578
char *buf;
574579

575-
if ((value = get_ready_ep(file->f_flags, epdata)) < 0)
580+
if ((value = get_ready_ep(file->f_flags, epdata, false)) < 0)
576581
return value;
577582

578583
/* halt any endpoint by doing a "wrong direction" i/o call */
@@ -620,20 +625,25 @@ ep_read_iter(struct kiocb *iocb, struct iov_iter *to)
620625
return value;
621626
}
622627

628+
static ssize_t ep_config(struct ep_data *, const char *, size_t);
629+
623630
static ssize_t
624631
ep_write_iter(struct kiocb *iocb, struct iov_iter *from)
625632
{
626633
struct file *file = iocb->ki_filp;
627634
struct ep_data *epdata = file->private_data;
628635
size_t len = iov_iter_count(from);
636+
bool configured;
629637
ssize_t value;
630638
char *buf;
631639

632-
if ((value = get_ready_ep(file->f_flags, epdata)) < 0)
640+
if ((value = get_ready_ep(file->f_flags, epdata, true)) < 0)
633641
return value;
634642

643+
configured = epdata->state == STATE_EP_ENABLED;
644+
635645
/* halt any endpoint by doing a "wrong direction" i/o call */
636-
if (!usb_endpoint_dir_in(&epdata->desc)) {
646+
if (configured && !usb_endpoint_dir_in(&epdata->desc)) {
637647
if (usb_endpoint_xfer_isoc(&epdata->desc) ||
638648
!is_sync_kiocb(iocb)) {
639649
mutex_unlock(&epdata->lock);
@@ -659,7 +669,9 @@ ep_write_iter(struct kiocb *iocb, struct iov_iter *from)
659669
goto out;
660670
}
661671

662-
if (is_sync_kiocb(iocb)) {
672+
if (unlikely(!configured)) {
673+
value = ep_config(epdata, buf, len);
674+
} else if (is_sync_kiocb(iocb)) {
663675
value = ep_io(epdata, buf, len);
664676
} else {
665677
struct kiocb_priv *priv = kzalloc(sizeof *priv, GFP_KERNEL);
@@ -681,13 +693,13 @@ ep_write_iter(struct kiocb *iocb, struct iov_iter *from)
681693
/* used after endpoint configuration */
682694
static const struct file_operations ep_io_operations = {
683695
.owner = THIS_MODULE,
684-
.llseek = no_llseek,
685696

697+
.open = ep_open,
698+
.release = ep_release,
699+
.llseek = no_llseek,
686700
.read = new_sync_read,
687701
.write = new_sync_write,
688702
.unlocked_ioctl = ep_ioctl,
689-
.release = ep_release,
690-
691703
.read_iter = ep_read_iter,
692704
.write_iter = ep_write_iter,
693705
};
@@ -706,17 +718,12 @@ static const struct file_operations ep_io_operations = {
706718
* speed descriptor, then optional high speed descriptor.
707719
*/
708720
static ssize_t
709-
ep_config (struct file *fd, const char __user *buf, size_t len, loff_t *ptr)
721+
ep_config (struct ep_data *data, const char *buf, size_t len)
710722
{
711-
struct ep_data *data = fd->private_data;
712723
struct usb_ep *ep;
713724
u32 tag;
714725
int value, length = len;
715726

716-
value = mutex_lock_interruptible(&data->lock);
717-
if (value < 0)
718-
return value;
719-
720727
if (data->state != STATE_EP_READY) {
721728
value = -EL2HLT;
722729
goto fail;
@@ -727,9 +734,7 @@ ep_config (struct file *fd, const char __user *buf, size_t len, loff_t *ptr)
727734
goto fail0;
728735

729736
/* we might need to change message format someday */
730-
if (copy_from_user (&tag, buf, 4)) {
731-
goto fail1;
732-
}
737+
memcpy(&tag, buf, 4);
733738
if (tag != 1) {
734739
DBG(data->dev, "config %s, bad tag %d\n", data->name, tag);
735740
goto fail0;
@@ -742,19 +747,15 @@ ep_config (struct file *fd, const char __user *buf, size_t len, loff_t *ptr)
742747
*/
743748

744749
/* full/low speed descriptor, then high speed */
745-
if (copy_from_user (&data->desc, buf, USB_DT_ENDPOINT_SIZE)) {
746-
goto fail1;
747-
}
750+
memcpy(&data->desc, buf, USB_DT_ENDPOINT_SIZE);
748751
if (data->desc.bLength != USB_DT_ENDPOINT_SIZE
749752
|| data->desc.bDescriptorType != USB_DT_ENDPOINT)
750753
goto fail0;
751754
if (len != USB_DT_ENDPOINT_SIZE) {
752755
if (len != 2 * USB_DT_ENDPOINT_SIZE)
753756
goto fail0;
754-
if (copy_from_user (&data->hs_desc, buf + USB_DT_ENDPOINT_SIZE,
755-
USB_DT_ENDPOINT_SIZE)) {
756-
goto fail1;
757-
}
757+
memcpy(&data->hs_desc, buf + USB_DT_ENDPOINT_SIZE,
758+
USB_DT_ENDPOINT_SIZE);
758759
if (data->hs_desc.bLength != USB_DT_ENDPOINT_SIZE
759760
|| data->hs_desc.bDescriptorType
760761
!= USB_DT_ENDPOINT) {
@@ -776,24 +777,20 @@ ep_config (struct file *fd, const char __user *buf, size_t len, loff_t *ptr)
776777
case USB_SPEED_LOW:
777778
case USB_SPEED_FULL:
778779
ep->desc = &data->desc;
779-
value = usb_ep_enable(ep);
780-
if (value == 0)
781-
data->state = STATE_EP_ENABLED;
782780
break;
783781
case USB_SPEED_HIGH:
784782
/* fails if caller didn't provide that descriptor... */
785783
ep->desc = &data->hs_desc;
786-
value = usb_ep_enable(ep);
787-
if (value == 0)
788-
data->state = STATE_EP_ENABLED;
789784
break;
790785
default:
791786
DBG(data->dev, "unconnected, %s init abandoned\n",
792787
data->name);
793788
value = -EINVAL;
789+
goto gone;
794790
}
791+
value = usb_ep_enable(ep);
795792
if (value == 0) {
796-
fd->f_op = &ep_io_operations;
793+
data->state = STATE_EP_ENABLED;
797794
value = length;
798795
}
799796
gone:
@@ -803,14 +800,10 @@ ep_config (struct file *fd, const char __user *buf, size_t len, loff_t *ptr)
803800
data->desc.bDescriptorType = 0;
804801
data->hs_desc.bDescriptorType = 0;
805802
}
806-
mutex_unlock(&data->lock);
807803
return value;
808804
fail0:
809805
value = -EINVAL;
810806
goto fail;
811-
fail1:
812-
value = -EFAULT;
813-
goto fail;
814807
}
815808

816809
static int
@@ -838,15 +831,6 @@ ep_open (struct inode *inode, struct file *fd)
838831
return value;
839832
}
840833

841-
/* used before endpoint configuration */
842-
static const struct file_operations ep_config_operations = {
843-
.llseek = no_llseek,
844-
845-
.open = ep_open,
846-
.write = ep_config,
847-
.release = ep_release,
848-
};
849-
850834
/*----------------------------------------------------------------------*/
851835

852836
/* EP0 IMPLEMENTATION can be partly in userspace.
@@ -1586,7 +1570,7 @@ static int activate_ep_files (struct dev_data *dev)
15861570
goto enomem1;
15871571

15881572
data->dentry = gadgetfs_create_file (dev->sb, data->name,
1589-
data, &ep_config_operations);
1573+
data, &ep_io_operations);
15901574
if (!data->dentry)
15911575
goto enomem2;
15921576
list_add_tail (&data->epfiles, &dev->epfiles);

0 commit comments

Comments
 (0)