Skip to content

Commit

Permalink
gadgetfs: get rid of flipping ->f_op in ep_config()
Browse files Browse the repository at this point in the history 
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>
  • Loading branch information
Al Viro committed Mar 8, 2015
1 parent 7fe3976 commit d4461a6
Showing 1 changed file with 37 additions and 53 deletions.
90 changes: 37 additions & 53 deletions drivers/usb/gadget/legacy/inode.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ MODULE_DESCRIPTION (DRIVER_DESC);
MODULE_AUTHOR ("David Brownell");
MODULE_LICENSE ("GPL");

static int ep_open(struct inode *, struct file *);


/*----------------------------------------------------------------------*/

Expand Down Expand Up @@ -283,14 +285,15 @@ static void epio_complete (struct usb_ep *ep, struct usb_request *req)
* still need dev->lock to use epdata->ep.
*/
static int
get_ready_ep (unsigned f_flags, struct ep_data *epdata)
get_ready_ep (unsigned f_flags, struct ep_data *epdata, bool is_write)
{
int val;

if (f_flags & O_NONBLOCK) {
if (!mutex_trylock(&epdata->lock))
goto nonblock;
if (epdata->state != STATE_EP_ENABLED) {
if (epdata->state != STATE_EP_ENABLED &&
(!is_write || epdata->state != STATE_EP_READY)) {
mutex_unlock(&epdata->lock);
nonblock:
val = -EAGAIN;
Expand All @@ -305,18 +308,20 @@ get_ready_ep (unsigned f_flags, struct ep_data *epdata)

switch (epdata->state) {
case STATE_EP_ENABLED:
return 0;
case STATE_EP_READY: /* not configured yet */
if (is_write)
return 0;
// FALLTHRU
case STATE_EP_UNBOUND: /* clean disconnect */
break;
// case STATE_EP_DISABLED: /* "can't happen" */
// case STATE_EP_READY: /* "can't happen" */
default: /* error! */
pr_debug ("%s: ep %p not available, state %d\n",
shortname, epdata, epdata->state);
// FALLTHROUGH
case STATE_EP_UNBOUND: /* clean disconnect */
val = -ENODEV;
mutex_unlock(&epdata->lock);
}
return val;
mutex_unlock(&epdata->lock);
return -ENODEV;
}

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

if ((status = get_ready_ep (fd->f_flags, data)) < 0)
if ((status = get_ready_ep (fd->f_flags, data, false)) < 0)
return status;

spin_lock_irq (&data->dev->lock);
Expand Down Expand Up @@ -572,7 +577,7 @@ ep_read_iter(struct kiocb *iocb, struct iov_iter *to)
ssize_t value;
char *buf;

if ((value = get_ready_ep(file->f_flags, epdata)) < 0)
if ((value = get_ready_ep(file->f_flags, epdata, false)) < 0)
return value;

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

static ssize_t ep_config(struct ep_data *, const char *, size_t);

static ssize_t
ep_write_iter(struct kiocb *iocb, struct iov_iter *from)
{
struct file *file = iocb->ki_filp;
struct ep_data *epdata = file->private_data;
size_t len = iov_iter_count(from);
bool configured;
ssize_t value;
char *buf;

if ((value = get_ready_ep(file->f_flags, epdata)) < 0)
if ((value = get_ready_ep(file->f_flags, epdata, true)) < 0)
return value;

configured = epdata->state == STATE_EP_ENABLED;

/* halt any endpoint by doing a "wrong direction" i/o call */
if (!usb_endpoint_dir_in(&epdata->desc)) {
if (configured && !usb_endpoint_dir_in(&epdata->desc)) {
if (usb_endpoint_xfer_isoc(&epdata->desc) ||
!is_sync_kiocb(iocb)) {
mutex_unlock(&epdata->lock);
Expand All @@ -659,7 +669,9 @@ ep_write_iter(struct kiocb *iocb, struct iov_iter *from)
goto out;
}

if (is_sync_kiocb(iocb)) {
if (unlikely(!configured)) {
value = ep_config(epdata, buf, len);
} else if (is_sync_kiocb(iocb)) {
value = ep_io(epdata, buf, len);
} else {
struct kiocb_priv *priv = kzalloc(sizeof *priv, GFP_KERNEL);
Expand All @@ -681,13 +693,13 @@ ep_write_iter(struct kiocb *iocb, struct iov_iter *from)
/* used after endpoint configuration */
static const struct file_operations ep_io_operations = {
.owner = THIS_MODULE,
.llseek = no_llseek,

.open = ep_open,
.release = ep_release,
.llseek = no_llseek,
.read = new_sync_read,
.write = new_sync_write,
.unlocked_ioctl = ep_ioctl,
.release = ep_release,

.read_iter = ep_read_iter,
.write_iter = ep_write_iter,
};
Expand All @@ -706,17 +718,12 @@ static const struct file_operations ep_io_operations = {
* speed descriptor, then optional high speed descriptor.
*/
static ssize_t
ep_config (struct file *fd, const char __user *buf, size_t len, loff_t *ptr)
ep_config (struct ep_data *data, const char *buf, size_t len)
{
struct ep_data *data = fd->private_data;
struct usb_ep *ep;
u32 tag;
int value, length = len;

value = mutex_lock_interruptible(&data->lock);
if (value < 0)
return value;

if (data->state != STATE_EP_READY) {
value = -EL2HLT;
goto fail;
Expand All @@ -727,9 +734,7 @@ ep_config (struct file *fd, const char __user *buf, size_t len, loff_t *ptr)
goto fail0;

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

/* full/low speed descriptor, then high speed */
if (copy_from_user (&data->desc, buf, USB_DT_ENDPOINT_SIZE)) {
goto fail1;
}
memcpy(&data->desc, buf, USB_DT_ENDPOINT_SIZE);
if (data->desc.bLength != USB_DT_ENDPOINT_SIZE
|| data->desc.bDescriptorType != USB_DT_ENDPOINT)
goto fail0;
if (len != USB_DT_ENDPOINT_SIZE) {
if (len != 2 * USB_DT_ENDPOINT_SIZE)
goto fail0;
if (copy_from_user (&data->hs_desc, buf + USB_DT_ENDPOINT_SIZE,
USB_DT_ENDPOINT_SIZE)) {
goto fail1;
}
memcpy(&data->hs_desc, buf + USB_DT_ENDPOINT_SIZE,
USB_DT_ENDPOINT_SIZE);
if (data->hs_desc.bLength != USB_DT_ENDPOINT_SIZE
|| data->hs_desc.bDescriptorType
!= USB_DT_ENDPOINT) {
Expand All @@ -776,24 +777,20 @@ ep_config (struct file *fd, const char __user *buf, size_t len, loff_t *ptr)
case USB_SPEED_LOW:
case USB_SPEED_FULL:
ep->desc = &data->desc;
value = usb_ep_enable(ep);
if (value == 0)
data->state = STATE_EP_ENABLED;
break;
case USB_SPEED_HIGH:
/* fails if caller didn't provide that descriptor... */
ep->desc = &data->hs_desc;
value = usb_ep_enable(ep);
if (value == 0)
data->state = STATE_EP_ENABLED;
break;
default:
DBG(data->dev, "unconnected, %s init abandoned\n",
data->name);
value = -EINVAL;
goto gone;
}
value = usb_ep_enable(ep);
if (value == 0) {
fd->f_op = &ep_io_operations;
data->state = STATE_EP_ENABLED;
value = length;
}
gone:
Expand All @@ -803,14 +800,10 @@ ep_config (struct file *fd, const char __user *buf, size_t len, loff_t *ptr)
data->desc.bDescriptorType = 0;
data->hs_desc.bDescriptorType = 0;
}
mutex_unlock(&data->lock);
return value;
fail0:
value = -EINVAL;
goto fail;
fail1:
value = -EFAULT;
goto fail;
}

static int
Expand Down Expand Up @@ -838,15 +831,6 @@ ep_open (struct inode *inode, struct file *fd)
return value;
}

/* used before endpoint configuration */
static const struct file_operations ep_config_operations = {
.llseek = no_llseek,

.open = ep_open,
.write = ep_config,
.release = ep_release,
};

/*----------------------------------------------------------------------*/

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

data->dentry = gadgetfs_create_file (dev->sb, data->name,
data, &ep_config_operations);
data, &ep_io_operations);
if (!data->dentry)
goto enomem2;
list_add_tail (&data->epfiles, &dev->epfiles);
Expand Down

0 comments on commit d4461a6

Please sign in to comment.