Bug 9595
| Summary: | Another potential NULL ptr dereference, drivers/usb/gadget/fsl_usb2_udc.c | ||
|---|---|---|---|
| Product: | Drivers | Reporter: | Marcio Buss (marciobuss) |
| Component: | USB | Assignee: | David Brownell (dbrownell) |
| Status: | CLOSED CODE_FIX | ||
| Severity: | normal | ||
| Priority: | P1 | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Kernel Version: | 2.6.23 | Subsystem: | |
| Regression: | --- | Bisected commit-id: | |
Reply-To: david-b@pacbell.net On Monday 17 December 2007, bugme-daemon@bugzilla.kernel.org wrote: > http://bugzilla.kernel.org/show_bug.cgi?id=9595 The maintainers of this driver are in a better position to handle this than I am ... I think this is a bogus bug report, since the claus in question is just rejecting endpoints other than ep0 that don't have descriptors. (If it's coded like other drivers...) Reply-To: leoli@freescale.com On Wed, 2008-01-02 at 01:31 -0800, David Brownell wrote: > On Monday 17 December 2007, bugme-daemon@bugzilla.kernel.org wrote: > > http://bugzilla.kernel.org/show_bug.cgi?id=9595 > > The maintainers of this driver are in a better position to handle > this than I am ... > > I think this is a bogus bug report, since the claus in question is > just rejecting endpoints other than ep0 that don't have descriptors. > (If it's coded like other drivers...) For fsl_usb2_udc driver, ep0 also has a descriptor. Current code is misleading and contains a logical mistake. Here is the patch to cleanup it. Thanks. --- diff --git a/drivers/usb/gadget/fsl_usb2_udc.c b/drivers/usb/gadget/fsl_usb2_udc.c index 038e7d7..08cb673 100644 --- a/drivers/usb/gadget/fsl_usb2_udc.c +++ b/drivers/usb/gadget/fsl_usb2_udc.c @@ -776,7 +776,7 @@ fsl_ep_queue(struct usb_ep *_ep, struct usb_request *_req, gfp_t gfp_flags) VDBG("%s, bad params\n", __FUNCTION__); return -EINVAL; } - if (!_ep || (!ep->desc && ep_index(ep))) { + if (unlikely(!_ep || !ep->desc)) { VDBG("%s, bad ep\n", __FUNCTION__); return -EINVAL; } Upstream merge checked |
The following code fragment occurs at line 773: if (!_ep || (!ep->desc && ep_index(ep))) { VDBG("%s, bad ep\n", __FUNCTION__); return -EINVAL; } Shouldn't the first conjunct of "&&" test be "ep->desc" ? if (!_ep || (ep->desc && ep_index(ep))) { VDBG("%s, bad ep\n", __FUNCTION__); return -EINVAL; }