ok, let's start here.
This patch set removes any use of the list iterator variable past
the list body. This will allow defining the list iterator variable
within the list_for_each_entry_*() macros to avoid any (invalid)
use after the loop. If no break/goto was hit during list traversal
the list iterator variable would otherwise be a bogus pointer
since it is computed on something that is not actually an element
of the list.
I've basically followed what we discussed in:
https://lore.kernel.org/all/[email protected]/
There are some cases where it might be possible to 'ditch' the tmp
variable and refactor the code past the loop into the loop body.
For the sake of keeping the *_dequeue() functions more similar, I've
decided against doing it for some and leaving it in others.
In general there are four use cases after the loop body here:
1) using &req->req in a comparision after the loop
2) using the iterator as a pointer in a comparision after the loop
3) use the &iterator->list to compare with the head to see if the
loop exits early
4) using the iterator past the loop but using the rc variable to see if the
loop exits early
Jakob Koschel (26):
usb: gadget: fsl: remove usage of list iterator past the loop body
usb: gadget: bdc: remove usage of list iterator past the loop body
usb: gadget: udc: atmel: remove usage of list iterator past the loop
body
usb: gadget: udc: pxa25x: remove usage of list iterator past the loop
body
usb: gadget: udc: at91: remove usage of list iterator past the loop
body
usb: gadget: goku_udc: remove usage of list iterator past the loop
body
usb: gadget: udc: gr_udc: remove usage of list iterator past the loop
body
usb: gadget: lpc32xx_udc: remove usage of list iterator past the loop
body
usb: gadget: mv_u3d: remove usage of list iterator past the loop body
usb: gadget: udc: mv_udc_core: remove usage of list iterator past the
loop body
usb: gadget: net2272: remove usage of list iterator past the loop body
usb: gadget: udc: net2280: remove usage of list iterator past the loop
body
usb: gadget: omap_udc: remove usage of list iterator past the loop
body
usb: gadget: s3c-hsudc: remove usage of list iterator past the loop
body
usb: gadget: udc-xilinx: remove usage of list iterator past the loop
body
usb: gadget: aspeed: remove usage of list iterator past the loop body
usb: gadget: configfs: remove using list iterator after loop body as a
ptr
usb: gadget: legacy: remove using list iterator after loop body as a
ptr
usb: gadget: udc: max3420_udc: remove using list iterator after loop
body as a ptr
usb: gadget: tegra-xudc: remove using list iterator after loop body as
a ptr
usb: gadget: composite: remove check of list iterator against head
past the loop body
usb: gadget: pxa27x_udc: replace usage of rc to check if a list
element was found
usb: gadget: udc: s3c2410: replace usage of rc to check if a list
element was found
usb: gadget: udc: core: replace usage of rc to check if a list element
was found
usb: gadget: dummy_hcd: replace usage of rc to check if a list element
was found
usb: gadget: udc: s3c2410: replace usage of rc to check if a list
element was found
drivers/usb/gadget/composite.c | 18 ++++++++++--------
drivers/usb/gadget/configfs.c | 20 ++++++++++++--------
drivers/usb/gadget/legacy/hid.c | 23 ++++++++++++-----------
drivers/usb/gadget/udc/aspeed-vhub/epn.c | 10 ++++++----
drivers/usb/gadget/udc/at91_udc.c | 10 ++++++----
drivers/usb/gadget/udc/atmel_usba_udc.c | 11 +++++++----
drivers/usb/gadget/udc/bdc/bdc_ep.c | 11 ++++++++---
drivers/usb/gadget/udc/core.c | 16 ++++++++++------
drivers/usb/gadget/udc/dummy_hcd.c | 11 ++++++-----
drivers/usb/gadget/udc/fsl_qe_udc.c | 11 +++++++----
drivers/usb/gadget/udc/fsl_udc_core.c | 11 +++++++----
drivers/usb/gadget/udc/goku_udc.c | 10 ++++++----
drivers/usb/gadget/udc/gr_udc.c | 10 ++++++----
drivers/usb/gadget/udc/lpc32xx_udc.c | 10 ++++++----
drivers/usb/gadget/udc/max3420_udc.c | 11 +++++++----
drivers/usb/gadget/udc/mv_u3d_core.c | 10 ++++++----
drivers/usb/gadget/udc/mv_udc_core.c | 10 ++++++----
drivers/usb/gadget/udc/net2272.c | 11 ++++++-----
drivers/usb/gadget/udc/net2280.c | 11 +++++++----
drivers/usb/gadget/udc/omap_udc.c | 10 ++++++----
drivers/usb/gadget/udc/pxa25x_udc.c | 11 +++++++----
drivers/usb/gadget/udc/pxa27x_udc.c | 9 +++++----
drivers/usb/gadget/udc/s3c-hsudc.c | 10 ++++++----
drivers/usb/gadget/udc/s3c2410_udc.c | 11 ++++++-----
drivers/usb/gadget/udc/tegra-xudc.c | 10 ++++++----
drivers/usb/gadget/udc/udc-xilinx.c | 11 +++++++----
26 files changed, 184 insertions(+), 123 deletions(-)
base-commit: 719fce7539cd3e186598e2aed36325fe892150cf
--
2.25.1
If the list representing the request queue does not contain the expected
request, the value of the list_for_each_entry() iterator will not point
to a valid structure. To avoid type confusion in such case, the list
iterator scope will be limited to the list_for_each_entry() loop.
In preparation to limiting scope of the list iterator to the list traversal
loop, use a dedicated pointer to point to the found request object [1].
Link: https://lore.kernel.org/all/[email protected]/
Signed-off-by: Jakob Koschel <[email protected]>
---
drivers/usb/gadget/udc/at91_udc.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/gadget/udc/at91_udc.c b/drivers/usb/gadget/udc/at91_udc.c
index 9040a0561466..3578b061a3ee 100644
--- a/drivers/usb/gadget/udc/at91_udc.c
+++ b/drivers/usb/gadget/udc/at91_udc.c
@@ -704,7 +704,7 @@ static int at91_ep_queue(struct usb_ep *_ep,
static int at91_ep_dequeue(struct usb_ep *_ep, struct usb_request *_req)
{
struct at91_ep *ep;
- struct at91_request *req;
+ struct at91_request *req = NULL, *tmp;
unsigned long flags;
struct at91_udc *udc;
@@ -717,11 +717,13 @@ static int at91_ep_dequeue(struct usb_ep *_ep, struct usb_request *_req)
spin_lock_irqsave(&udc->lock, flags);
/* make sure it's actually queued on this endpoint */
- list_for_each_entry (req, &ep->queue, queue) {
- if (&req->req == _req)
+ list_for_each_entry(tmp, &ep->queue, queue) {
+ if (&tmp->req == _req) {
+ req = tmp;
break;
+ }
}
- if (&req->req != _req) {
+ if (!req) {
spin_unlock_irqrestore(&udc->lock, flags);
return -EINVAL;
}
--
2.25.1
If the list representing the request queue does not contain the expected
request, the value of the list_for_each_entry() iterator will not point
to a valid structure. To avoid type confusion in such case, the list
iterator scope will be limited to the list_for_each_entry() loop.
In preparation to limiting scope of the list iterator to the list traversal
loop, use a dedicated pointer to point to the found request object [1].
Link: https://lore.kernel.org/all/[email protected]/
Signed-off-by: Jakob Koschel <[email protected]>
---
drivers/usb/gadget/udc/udc-xilinx.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/gadget/udc/udc-xilinx.c b/drivers/usb/gadget/udc/udc-xilinx.c
index 2907fad04e2c..34b7000f0602 100644
--- a/drivers/usb/gadget/udc/udc-xilinx.c
+++ b/drivers/usb/gadget/udc/udc-xilinx.c
@@ -1136,17 +1136,20 @@ static int xudc_ep_queue(struct usb_ep *_ep, struct usb_request *_req,
static int xudc_ep_dequeue(struct usb_ep *_ep, struct usb_request *_req)
{
struct xusb_ep *ep = to_xusb_ep(_ep);
- struct xusb_req *req = to_xusb_req(_req);
+ struct xusb_req *req = NULL;
+ struct xusb_req *tmp;
struct xusb_udc *udc = ep->udc;
unsigned long flags;
spin_lock_irqsave(&udc->lock, flags);
/* Make sure it's actually queued on this endpoint */
- list_for_each_entry(req, &ep->queue, queue) {
- if (&req->usb_req == _req)
+ list_for_each_entry(tmp, &ep->queue, queue) {
+ if (&tmp->usb_req == _req) {
+ req = tmp;
break;
+ }
}
- if (&req->usb_req != _req) {
+ if (!req) {
spin_unlock_irqrestore(&udc->lock, flags);
return -EINVAL;
}
--
2.25.1
If the list representing the request queue does not contain the expected
request, the value of the list_for_each_entry() iterator will not point
to a valid structure. To avoid type confusion in such case, the list
iterator scope will be limited to the list_for_each_entry() loop.
In preparation to limiting scope of the list iterator to the list traversal
loop, use a dedicated pointer to point to the found request object [1].
Link: https://lore.kernel.org/all/[email protected]/
Signed-off-by: Jakob Koschel <[email protected]>
---
drivers/usb/gadget/udc/bdc/bdc_ep.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/gadget/udc/bdc/bdc_ep.c b/drivers/usb/gadget/udc/bdc/bdc_ep.c
index 8e2f20b12519..829e96791d0a 100644
--- a/drivers/usb/gadget/udc/bdc/bdc_ep.c
+++ b/drivers/usb/gadget/udc/bdc/bdc_ep.c
@@ -1757,6 +1757,7 @@ static int bdc_gadget_ep_dequeue(struct usb_ep *_ep,
struct usb_request *_req)
{
struct bdc_req *req;
+ struct bdc_req *tmp;
unsigned long flags;
struct bdc_ep *ep;
struct bdc *bdc;
@@ -1771,12 +1772,16 @@ static int bdc_gadget_ep_dequeue(struct usb_ep *_ep,
dev_dbg(bdc->dev, "%s ep:%s req:%p\n", __func__, ep->name, req);
bdc_dbg_bd_list(bdc, ep);
spin_lock_irqsave(&bdc->lock, flags);
+
+ req = NULL;
/* make sure it's still queued on this endpoint */
- list_for_each_entry(req, &ep->queue, queue) {
- if (&req->usb_req == _req)
+ list_for_each_entry(tmp, &ep->queue, queue) {
+ if (&tmp->usb_req == _req) {
+ req = tmp;
break;
+ }
}
- if (&req->usb_req != _req) {
+ if (!req) {
spin_unlock_irqrestore(&bdc->lock, flags);
dev_err(bdc->dev, "usb_req !=req n");
return -EINVAL;
--
2.25.1
If the list representing the request queue does not contain the expected
request, the value of the list_for_each_entry() iterator will not point
to a valid structure. To avoid type confusion in such case, the list
iterator scope will be limited to the list_for_each_entry() loop.
In preparation to limiting scope of the list iterator to the list traversal
loop, use a dedicated pointer to point to the found request object [1].
Link: https://lore.kernel.org/all/[email protected]/
Signed-off-by: Jakob Koschel <[email protected]>
---
drivers/usb/gadget/udc/lpc32xx_udc.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/gadget/udc/lpc32xx_udc.c b/drivers/usb/gadget/udc/lpc32xx_udc.c
index a25d01c89564..fbb235b1f23f 100644
--- a/drivers/usb/gadget/udc/lpc32xx_udc.c
+++ b/drivers/usb/gadget/udc/lpc32xx_udc.c
@@ -1830,7 +1830,7 @@ static int lpc32xx_ep_queue(struct usb_ep *_ep,
static int lpc32xx_ep_dequeue(struct usb_ep *_ep, struct usb_request *_req)
{
struct lpc32xx_ep *ep;
- struct lpc32xx_request *req;
+ struct lpc32xx_request *req = NULL, *tmp;
unsigned long flags;
ep = container_of(_ep, struct lpc32xx_ep, ep);
@@ -1840,11 +1840,13 @@ static int lpc32xx_ep_dequeue(struct usb_ep *_ep, struct usb_request *_req)
spin_lock_irqsave(&ep->udc->lock, flags);
/* make sure it's actually queued on this endpoint */
- list_for_each_entry(req, &ep->queue, queue) {
- if (&req->req == _req)
+ list_for_each_entry(tmp, &ep->queue, queue) {
+ if (&tmp->req == _req) {
+ req = tmp;
break;
+ }
}
- if (&req->req != _req) {
+ if (!req) {
spin_unlock_irqrestore(&ep->udc->lock, flags);
return -EINVAL;
}
--
2.25.1