* Re: [PATCH 2/9] usb: host: max3421-hcd: use list_for_each_entry*
[not found] <201512190149.tp8A4V4C%fengguang.wu@intel.com>
@ 2015-12-18 17:53 ` Julia Lawall
2015-12-19 16:11 ` [PATCH 2/9 v2] " Geliang Tang
0 siblings, 1 reply; 3+ messages in thread
From: Julia Lawall @ 2015-12-18 17:53 UTC (permalink / raw)
To: Geliang Tang; +Cc: Greg Kroah-Hartman, Sergei Shtylyov, linux-usb, linux-kernel
Geliang,
Please check whether line 762 can be reached in the case where the
list_for_each_entry reaches the end of the list. If that can happen,
max3421_ep should not be dereferenced.
julia
On Sat, 19 Dec 2015, kbuild test robot wrote:
> CC: kbuild-all@01.org
> In-Reply-To: <45e8397e370ed99ceb8aa1719c2b56a7ce741eb3.1450455485.git.geliangtang@163.com>
> TO: Geliang Tang <geliangtang@163.com>
> CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>, Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> CC: Geliang Tang <geliangtang@163.com>, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org
>
> Hi Geliang,
>
> [auto build test WARNING on usb/usb-testing]
> [also build test WARNING on v4.4-rc5 next-20151218]
>
> url: https://github.com/0day-ci/linux/commits/Geliang-Tang/usb-host-fotg210-use-list_for_each_entry_safe/20151219-003955
> base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
> :::::: branch date: 66 minutes ago
> :::::: commit date: 66 minutes ago
>
> >> drivers/usb/host/max3421-hcd.c:762:5-15: ERROR: invalid reference to the index variable of the iterator on line 675
>
> git remote add linux-review https://github.com/0day-ci/linux
> git remote update linux-review
> git checkout 610e92adf2a45ef78039582494d8023f385d12ec
> vim +762 drivers/usb/host/max3421-hcd.c
>
> 2d53139f David Mosberger 2014-04-28 669
> 2d53139f David Mosberger 2014-04-28 670 spin_lock_irqsave(&max3421_hcd->lock, flags);
> 2d53139f David Mosberger 2014-04-28 671
> 2d53139f David Mosberger 2014-04-28 672 for (;
> 2d53139f David Mosberger 2014-04-28 673 max3421_hcd->sched_pass < SCHED_PASS_DONE;
> 2d53139f David Mosberger 2014-04-28 674 ++max3421_hcd->sched_pass)
> 610e92ad Geliang Tang 2015-12-19 @675 list_for_each_entry(max3421_ep, &max3421_hcd->ep_list,
> 610e92ad Geliang Tang 2015-12-19 676 ep_list) {
> 2d53139f David Mosberger 2014-04-28 677 urb = NULL;
> 2d53139f David Mosberger 2014-04-28 678 ep = max3421_ep->ep;
> 2d53139f David Mosberger 2014-04-28 679
> 2d53139f David Mosberger 2014-04-28 680 switch (usb_endpoint_type(&ep->desc)) {
> 2d53139f David Mosberger 2014-04-28 681 case USB_ENDPOINT_XFER_ISOC:
> 2d53139f David Mosberger 2014-04-28 682 case USB_ENDPOINT_XFER_INT:
> 2d53139f David Mosberger 2014-04-28 683 if (max3421_hcd->sched_pass !=
> 2d53139f David Mosberger 2014-04-28 684 SCHED_PASS_PERIODIC)
> 2d53139f David Mosberger 2014-04-28 685 continue;
> 2d53139f David Mosberger 2014-04-28 686 break;
> 2d53139f David Mosberger 2014-04-28 687
> 2d53139f David Mosberger 2014-04-28 688 case USB_ENDPOINT_XFER_CONTROL:
> 2d53139f David Mosberger 2014-04-28 689 case USB_ENDPOINT_XFER_BULK:
> 2d53139f David Mosberger 2014-04-28 690 if (max3421_hcd->sched_pass !=
> 2d53139f David Mosberger 2014-04-28 691 SCHED_PASS_NON_PERIODIC)
> 2d53139f David Mosberger 2014-04-28 692 continue;
> 2d53139f David Mosberger 2014-04-28 693 break;
> 2d53139f David Mosberger 2014-04-28 694 }
> 2d53139f David Mosberger 2014-04-28 695
> 2d53139f David Mosberger 2014-04-28 696 if (list_empty(&ep->urb_list))
> 2d53139f David Mosberger 2014-04-28 697 continue; /* nothing to do */
> 2d53139f David Mosberger 2014-04-28 698 urb = list_first_entry(&ep->urb_list, struct urb,
> 2d53139f David Mosberger 2014-04-28 699 urb_list);
> 2d53139f David Mosberger 2014-04-28 700 if (urb->unlinked) {
> 2d53139f David Mosberger 2014-04-28 701 dev_dbg(&spi->dev, "%s: URB %p unlinked=%d",
> 2d53139f David Mosberger 2014-04-28 702 __func__, urb, urb->unlinked);
> 2d53139f David Mosberger 2014-04-28 703 max3421_hcd->curr_urb = urb;
> 2d53139f David Mosberger 2014-04-28 704 max3421_hcd->urb_done = 1;
> 2d53139f David Mosberger 2014-04-28 705 spin_unlock_irqrestore(&max3421_hcd->lock,
> 2d53139f David Mosberger 2014-04-28 706 flags);
> 2d53139f David Mosberger 2014-04-28 707 return 1;
> 2d53139f David Mosberger 2014-04-28 708 }
> 2d53139f David Mosberger 2014-04-28 709
> 2d53139f David Mosberger 2014-04-28 710 switch (usb_endpoint_type(&ep->desc)) {
> 2d53139f David Mosberger 2014-04-28 711 case USB_ENDPOINT_XFER_CONTROL:
> 2d53139f David Mosberger 2014-04-28 712 /*
> 2d53139f David Mosberger 2014-04-28 713 * Allow one control transaction per
> 2d53139f David Mosberger 2014-04-28 714 * frame per endpoint:
> 2d53139f David Mosberger 2014-04-28 715 */
> 2d53139f David Mosberger 2014-04-28 716 if (frame_diff(max3421_ep->last_active,
> 2d53139f David Mosberger 2014-04-28 717 max3421_hcd->frame_number) == 0)
> 2d53139f David Mosberger 2014-04-28 718 continue;
> 2d53139f David Mosberger 2014-04-28 719 break;
> 2d53139f David Mosberger 2014-04-28 720
> 2d53139f David Mosberger 2014-04-28 721 case USB_ENDPOINT_XFER_BULK:
> 2d53139f David Mosberger 2014-04-28 722 if (max3421_ep->retransmit
> 2d53139f David Mosberger 2014-04-28 723 && (frame_diff(max3421_ep->last_active,
> 2d53139f David Mosberger 2014-04-28 724 max3421_hcd->frame_number)
> 2d53139f David Mosberger 2014-04-28 725 == 0))
> 2d53139f David Mosberger 2014-04-28 726 /*
> 2d53139f David Mosberger 2014-04-28 727 * We already tried this EP
> 2d53139f David Mosberger 2014-04-28 728 * during this frame and got a
> 2d53139f David Mosberger 2014-04-28 729 * NAK or error; wait for next frame
> 2d53139f David Mosberger 2014-04-28 730 */
> 2d53139f David Mosberger 2014-04-28 731 continue;
> 2d53139f David Mosberger 2014-04-28 732 break;
> 2d53139f David Mosberger 2014-04-28 733
> 2d53139f David Mosberger 2014-04-28 734 case USB_ENDPOINT_XFER_ISOC:
> 2d53139f David Mosberger 2014-04-28 735 case USB_ENDPOINT_XFER_INT:
> 2d53139f David Mosberger 2014-04-28 736 if (frame_diff(max3421_hcd->frame_number,
> 2d53139f David Mosberger 2014-04-28 737 max3421_ep->last_active)
> 2d53139f David Mosberger 2014-04-28 738 < urb->interval)
> 2d53139f David Mosberger 2014-04-28 739 /*
> 2d53139f David Mosberger 2014-04-28 740 * We already processed this
> 2d53139f David Mosberger 2014-04-28 741 * end-point in the current
> 2d53139f David Mosberger 2014-04-28 742 * frame
> 2d53139f David Mosberger 2014-04-28 743 */
> 2d53139f David Mosberger 2014-04-28 744 continue;
> 2d53139f David Mosberger 2014-04-28 745 break;
> 2d53139f David Mosberger 2014-04-28 746 }
> 2d53139f David Mosberger 2014-04-28 747
> 2d53139f David Mosberger 2014-04-28 748 /* move current ep to tail: */
> 610e92ad Geliang Tang 2015-12-19 749 list_move_tail(&max3421_ep->ep_list,
> 610e92ad Geliang Tang 2015-12-19 750 &max3421_hcd->ep_list);
> 2d53139f David Mosberger 2014-04-28 751 curr_urb = urb;
> 2d53139f David Mosberger 2014-04-28 752 goto done;
> 2d53139f David Mosberger 2014-04-28 753 }
> 2d53139f David Mosberger 2014-04-28 754 done:
> 2d53139f David Mosberger 2014-04-28 755 if (!curr_urb) {
> 2d53139f David Mosberger 2014-04-28 756 spin_unlock_irqrestore(&max3421_hcd->lock, flags);
> 2d53139f David Mosberger 2014-04-28 757 return 0;
> 2d53139f David Mosberger 2014-04-28 758 }
> 2d53139f David Mosberger 2014-04-28 759
> 2d53139f David Mosberger 2014-04-28 760 urb = max3421_hcd->curr_urb = curr_urb;
> 2d53139f David Mosberger 2014-04-28 761 epnum = usb_endpoint_num(&urb->ep->desc);
> 2d53139f David Mosberger 2014-04-28 @762 if (max3421_ep->retransmit)
> 2d53139f David Mosberger 2014-04-28 763 /* restart (part of) a USB transaction: */
> 2d53139f David Mosberger 2014-04-28 764 max3421_ep->retransmit = 0;
> 2d53139f David Mosberger 2014-04-28 765 else {
>
> :::::: The code at line 762 was first introduced by commit
> :::::: 2d53139f31626bad6f8983d8e519ddde2cbba921 Add support for using a MAX3421E chip as a host driver.
>
> :::::: TO: David Mosberger <davidm@egauge.net>
> :::::: CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH 2/9 v2] usb: host: max3421-hcd: use list_for_each_entry*
2015-12-18 17:53 ` [PATCH 2/9] usb: host: max3421-hcd: use list_for_each_entry* Julia Lawall
@ 2015-12-19 16:11 ` Geliang Tang
0 siblings, 0 replies; 3+ messages in thread
From: Geliang Tang @ 2015-12-19 16:11 UTC (permalink / raw)
To: Julia Lawall, Greg Kroah-Hartman, Sergei Shtylyov
Cc: Geliang Tang, linux-usb, linux-kernel
Use list_for_each_entry*() instead of list_for_each*() to simplify
the code.
Signed-off-by: Geliang Tang <geliangtang@163.com>
---
Changes in v2:
- drop changes in max3421_select_and_start_urb().
---
drivers/usb/host/max3421-hcd.c | 16 +++++-----------
1 file changed, 5 insertions(+), 11 deletions(-)
diff --git a/drivers/usb/host/max3421-hcd.c b/drivers/usb/host/max3421-hcd.c
index bd98706..c369c29 100644
--- a/drivers/usb/host/max3421-hcd.c
+++ b/drivers/usb/host/max3421-hcd.c
@@ -797,19 +797,16 @@ max3421_check_unlink(struct usb_hcd *hcd)
{
struct spi_device *spi = to_spi_device(hcd->self.controller);
struct max3421_hcd *max3421_hcd = hcd_to_max3421(hcd);
- struct list_head *pos, *upos, *next_upos;
struct max3421_ep *max3421_ep;
struct usb_host_endpoint *ep;
- struct urb *urb;
+ struct urb *urb, *next;
unsigned long flags;
int retval = 0;
spin_lock_irqsave(&max3421_hcd->lock, flags);
- list_for_each(pos, &max3421_hcd->ep_list) {
- max3421_ep = container_of(pos, struct max3421_ep, ep_list);
+ list_for_each_entry(max3421_ep, &max3421_hcd->ep_list, ep_list) {
ep = max3421_ep->ep;
- list_for_each_safe(upos, next_upos, &ep->urb_list) {
- urb = container_of(upos, struct urb, urb_list);
+ list_for_each_entry_safe(urb, next, &ep->urb_list, urb_list) {
if (urb->unlinked) {
retval = 1;
dev_dbg(&spi->dev, "%s: URB %p unlinked=%d",
@@ -1184,22 +1181,19 @@ dump_eps(struct usb_hcd *hcd)
struct max3421_hcd *max3421_hcd = hcd_to_max3421(hcd);
struct max3421_ep *max3421_ep;
struct usb_host_endpoint *ep;
- struct list_head *pos, *upos;
char ubuf[512], *dp, *end;
unsigned long flags;
struct urb *urb;
int epnum, ret;
spin_lock_irqsave(&max3421_hcd->lock, flags);
- list_for_each(pos, &max3421_hcd->ep_list) {
- max3421_ep = container_of(pos, struct max3421_ep, ep_list);
+ list_for_each_entry(max3421_ep, &max3421_hcd->ep_list, ep_list) {
ep = max3421_ep->ep;
dp = ubuf;
end = dp + sizeof(ubuf);
*dp = '\0';
- list_for_each(upos, &ep->urb_list) {
- urb = container_of(upos, struct urb, urb_list);
+ list_for_each_entry(urb, &ep->urb_list, urb_list) {
ret = snprintf(dp, end - dp, " %p(%d.%s %d/%d)", urb,
usb_pipetype(urb->pipe),
usb_urb_dir_in(urb) ? "IN" : "OUT",
--
2.5.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH 2/9] usb: host: max3421-hcd: use list_for_each_entry*
2015-12-18 16:34 [PATCH 1/9] usb: host: fotg210: use list_for_each_entry_safe Geliang Tang
@ 2015-12-18 16:34 ` Geliang Tang
0 siblings, 0 replies; 3+ messages in thread
From: Geliang Tang @ 2015-12-18 16:34 UTC (permalink / raw)
To: Greg Kroah-Hartman, Sergei Shtylyov; +Cc: Geliang Tang, linux-usb, linux-kernel
Use list_for_each_entry*() instead of list_for_each*() to simplify
the code.
Signed-off-by: Geliang Tang <geliangtang@163.com>
---
drivers/usb/host/max3421-hcd.c | 25 +++++++++----------------
1 file changed, 9 insertions(+), 16 deletions(-)
diff --git a/drivers/usb/host/max3421-hcd.c b/drivers/usb/host/max3421-hcd.c
index bd98706..7257962 100644
--- a/drivers/usb/host/max3421-hcd.c
+++ b/drivers/usb/host/max3421-hcd.c
@@ -665,7 +665,6 @@ max3421_select_and_start_urb(struct usb_hcd *hcd)
struct max3421_ep *max3421_ep;
int epnum, force_toggles = 0;
struct usb_host_endpoint *ep;
- struct list_head *pos;
unsigned long flags;
spin_lock_irqsave(&max3421_hcd->lock, flags);
@@ -673,10 +672,9 @@ max3421_select_and_start_urb(struct usb_hcd *hcd)
for (;
max3421_hcd->sched_pass < SCHED_PASS_DONE;
++max3421_hcd->sched_pass)
- list_for_each(pos, &max3421_hcd->ep_list) {
+ list_for_each_entry(max3421_ep, &max3421_hcd->ep_list,
+ ep_list) {
urb = NULL;
- max3421_ep = container_of(pos, struct max3421_ep,
- ep_list);
ep = max3421_ep->ep;
switch (usb_endpoint_type(&ep->desc)) {
@@ -748,7 +746,8 @@ max3421_select_and_start_urb(struct usb_hcd *hcd)
}
/* move current ep to tail: */
- list_move_tail(pos, &max3421_hcd->ep_list);
+ list_move_tail(&max3421_ep->ep_list,
+ &max3421_hcd->ep_list);
curr_urb = urb;
goto done;
}
@@ -797,19 +796,16 @@ max3421_check_unlink(struct usb_hcd *hcd)
{
struct spi_device *spi = to_spi_device(hcd->self.controller);
struct max3421_hcd *max3421_hcd = hcd_to_max3421(hcd);
- struct list_head *pos, *upos, *next_upos;
struct max3421_ep *max3421_ep;
struct usb_host_endpoint *ep;
- struct urb *urb;
+ struct urb *urb, *next;
unsigned long flags;
int retval = 0;
spin_lock_irqsave(&max3421_hcd->lock, flags);
- list_for_each(pos, &max3421_hcd->ep_list) {
- max3421_ep = container_of(pos, struct max3421_ep, ep_list);
+ list_for_each_entry(max3421_ep, &max3421_hcd->ep_list, ep_list) {
ep = max3421_ep->ep;
- list_for_each_safe(upos, next_upos, &ep->urb_list) {
- urb = container_of(upos, struct urb, urb_list);
+ list_for_each_entry_safe(urb, next, &ep->urb_list, urb_list) {
if (urb->unlinked) {
retval = 1;
dev_dbg(&spi->dev, "%s: URB %p unlinked=%d",
@@ -1184,22 +1180,19 @@ dump_eps(struct usb_hcd *hcd)
struct max3421_hcd *max3421_hcd = hcd_to_max3421(hcd);
struct max3421_ep *max3421_ep;
struct usb_host_endpoint *ep;
- struct list_head *pos, *upos;
char ubuf[512], *dp, *end;
unsigned long flags;
struct urb *urb;
int epnum, ret;
spin_lock_irqsave(&max3421_hcd->lock, flags);
- list_for_each(pos, &max3421_hcd->ep_list) {
- max3421_ep = container_of(pos, struct max3421_ep, ep_list);
+ list_for_each_entry(max3421_ep, &max3421_hcd->ep_list, ep_list) {
ep = max3421_ep->ep;
dp = ubuf;
end = dp + sizeof(ubuf);
*dp = '\0';
- list_for_each(upos, &ep->urb_list) {
- urb = container_of(upos, struct urb, urb_list);
+ list_for_each_entry(urb, &ep->urb_list, urb_list) {
ret = snprintf(dp, end - dp, " %p(%d.%s %d/%d)", urb,
usb_pipetype(urb->pipe),
usb_urb_dir_in(urb) ? "IN" : "OUT",
--
2.5.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-12-19 16:12 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <201512190149.tp8A4V4C%fengguang.wu@intel.com>
2015-12-18 17:53 ` [PATCH 2/9] usb: host: max3421-hcd: use list_for_each_entry* Julia Lawall
2015-12-19 16:11 ` [PATCH 2/9 v2] " Geliang Tang
2015-12-18 16:34 [PATCH 1/9] usb: host: fotg210: use list_for_each_entry_safe Geliang Tang
2015-12-18 16:34 ` [PATCH 2/9] usb: host: max3421-hcd: use list_for_each_entry* Geliang Tang
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.