Checking more, I think the 3 "do {...} while(--count)" loops in f_fs.c
are wrong: they should be "while(count--){...}" ("git format-patch"
attached, sorry, I'm on a dev-hostile mail client at the moment):
- count covers function-specific endpoint, so it is 0 when no endpoint
is declared, which leads to an integer underflow and exceeding eps
array end.
- a function enabling/disabling endpoint 0 does not make sense to me
(they are only supposed to care about their own descriptor, don't they
?)
On Tue, Jan 3, 2017 at 3:49 PM, Vincent Pelletier <plr.vincent@xxxxxxxxx> wrote:
> The functionfs side of things is a bit complicated, as I'm writing a
> python module[2] for it, and I am still very new to functionfs. It
> does not crash on 4.9-rc7 though.
As per the above, this was not the whole truth: a device declared with
2 endpoints does not crash on 4.9-rc7, crashes on 4.10-rc2, including
with the upcomming rc3 fixes. A zero endpoint version crashes *and*
made me realise the loop error.
Sadly, this fix alone is not enough to get a functional device. I did
not investigate much yet, and need to get some sleep.
--
Vincent Pelletier
From 85815ce5c82022ccfbb2c0184589c4766325b517 Mon Sep 17 00:00:00 2001
From: Vincent Pelletier <vincent@xxxxxxxxxx>
Date: Tue, 3 Jan 2017 16:47:23 +0100
Subject: [RFC] usb: gadget: f_fs: Fix iterations on endpoints.
When zero endpoints are declared for a function, there is no endpoint
to disable, enable or free, so replace do...while loops with while loops.
Change pre-decrement to post-decrement to iterate the same number of times
when there are endpoints to process.
Signed-off-by: Vincent Pelletier <vincent@xxxxxxxxxx>
---
drivers/usb/gadget/function/f_fs.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
index 5e746adc8a2d..5490fc51638e 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -1806,7 +1806,7 @@ static void ffs_func_eps_disable(struct ffs_function *func)
unsigned long flags;
spin_lock_irqsave(&func->ffs->eps_lock, flags);
- do {
+ while (count--) {
/* pending requests get nuked */
if (likely(ep->ep))
usb_ep_disable(ep->ep);
@@ -1817,7 +1817,7 @@ static void ffs_func_eps_disable(struct ffs_function *func)
__ffs_epfile_read_buffer_free(epfile);
++epfile;
}
- } while (--count);
+ }
spin_unlock_irqrestore(&func->ffs->eps_lock, flags);
}
@@ -1831,7 +1831,7 @@ static int ffs_func_eps_enable(struct ffs_function *func)
int ret = 0;
spin_lock_irqsave(&func->ffs->eps_lock, flags);
- do {
+ while(count--) {
struct usb_endpoint_descriptor *ds;
int desc_idx;
@@ -1867,7 +1867,7 @@ static int ffs_func_eps_enable(struct ffs_function *func)
++ep;
++epfile;
- } while (--count);
+ }
spin_unlock_irqrestore(&func->ffs->eps_lock, flags);
return ret;
@@ -3448,12 +3448,12 @@ static void ffs_func_unbind(struct usb_configuration *c,
/* cleanup after autoconfig */
spin_lock_irqsave(&func->ffs->eps_lock, flags);
- do {
+ while (count--) {
if (ep->ep && ep->req)
usb_ep_free_request(ep->ep, ep->req);
ep->req = NULL;
++ep;
- } while (--count);
+ }
spin_unlock_irqrestore(&func->ffs->eps_lock, flags);
kfree(func->eps);
func->eps = NULL;
--
2.11.0
