From: Michal Nazarewicz <mina86@mina86.com>
To: Felipe Balbi <balbi@ti.com>
Cc: Rohith Seelaboyina <rseelaboyina@nvidia.com>,
balbi@ti.com, gregkh@linuxfoundation.org, r.baldyga@samsung.com,
andrzej.p@samsung.com, joe@perches.com,
linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH] usb: f_fs: refactor and document __ffs_ep0_read_events better
Date: Wed, 10 Sep 2014 17:50:24 +0200 [thread overview]
Message-ID: <xa1tfvbl6t5w.fsf@mina86.com> (raw)
In-Reply-To: <20150108170236.GI31926@saruman>
Instead of using variable length array, use a static length equal to
the size of the ffs->ev.types array. This gets rid of a sparse warning:
drivers/usb/gadget/function/f_fs.c:401:44: warning:
Variable length array is used.
and makes it more explicit that the array has a very tight upper size
limit. Also add some more documentation about the ev.types array and
how its size is limited and affects the rest of the code.
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Reported-by: Rohith Seelaboyina <rseelaboyina@nvidia.com>
Signed-off-by: Michal Nazarewicz <mina86@mina86.com>
---
drivers/usb/gadget/function/f_fs.c | 28 +++++++++++++++++-----------
1 file changed, 17 insertions(+), 11 deletions(-)
diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
index 63314ed..a00ee97 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -390,17 +390,20 @@ done_spin:
return ret;
}
+/* Called with ffs->ev.waitq.lock and ffs->mutex held, both released on exit. */
static ssize_t __ffs_ep0_read_events(struct ffs_data *ffs, char __user *buf,
size_t n)
{
/*
- * We are holding ffs->ev.waitq.lock and ffs->mutex and we need
- * to release them.
+ * n cannot be bigger than ffs->ev.count, which cannot be bigger than
+ * size of ffs->ev.types array (which is four) so that's how much space
+ * we reserve.
*/
- struct usb_functionfs_event events[n];
+ struct usb_functionfs_event events[ARRAY_SIZE(ffs->ev.types)];
+ const size_t size = n * sizeof *events;
unsigned i = 0;
- memset(events, 0, sizeof events);
+ memset(events, 0, size);
do {
events[i].type = ffs->ev.types[i];
@@ -410,19 +413,15 @@ static ssize_t __ffs_ep0_read_events(struct ffs_data *ffs, char __user *buf,
}
} while (++i < n);
- if (n < ffs->ev.count) {
- ffs->ev.count -= n;
+ ffs->ev.count -= n;
+ if (ffs->ev.count)
memmove(ffs->ev.types, ffs->ev.types + n,
ffs->ev.count * sizeof *ffs->ev.types);
- } else {
- ffs->ev.count = 0;
- }
spin_unlock_irq(&ffs->ev.waitq.lock);
mutex_unlock(&ffs->mutex);
- return unlikely(__copy_to_user(buf, events, sizeof events))
- ? -EFAULT : sizeof events;
+ return unlikely(__copy_to_user(buf, events, size)) ? -EFAULT : size;
}
static ssize_t ffs_ep0_read(struct file *file, char __user *buf,
@@ -2377,6 +2376,13 @@ static void __ffs_event_add(struct ffs_data *ffs,
if (ffs->setup_state == FFS_SETUP_PENDING)
ffs->setup_state = FFS_SETUP_CANCELLED;
+ /*
+ * Logic of this function guarantees that there are at most four pending
+ * evens on ffs->ev.types queue. This is important because the queue
+ * has space for four elements only and __ffs_ep0_read_events function
+ * depends on that limit as well. If more event types are added, those
+ * limits have to be revisited or guaranteed to still hold.
+ */
switch (type) {
case FUNCTIONFS_RESUME:
rem_type2 = FUNCTIONFS_SUSPEND;
--
2.2.0.rc0.207.ga3a616c
prev parent reply other threads:[~2015-01-08 18:09 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-12-24 12:00 [PATCH v3] usb: gadget: ffs: Fix sparse error Rohith Seelaboyina
2014-12-24 14:23 ` Michal Nazarewicz
2015-01-08 17:02 ` Felipe Balbi
2014-09-10 15:50 ` Michal Nazarewicz [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xa1tfvbl6t5w.fsf@mina86.com \
--to=mina86@mina86.com \
--cc=andrzej.p@samsung.com \
--cc=balbi@ti.com \
--cc=gregkh@linuxfoundation.org \
--cc=joe@perches.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=r.baldyga@samsung.com \
--cc=rseelaboyina@nvidia.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.