* 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 1/9] usb: host: fotg210: use list_for_each_entry_safe @ 2015-12-18 16:34 Geliang Tang 2015-12-18 16:34 ` [PATCH 2/9] usb: host: max3421-hcd: use list_for_each_entry* Geliang Tang 0 siblings, 1 reply; 3+ messages in thread From: Geliang Tang @ 2015-12-18 16:34 UTC (permalink / raw) To: Greg Kroah-Hartman, Peter Senna Tschudin, Sergei Shtylyov Cc: Geliang Tang, linux-usb, linux-kernel Use list_for_each_entry_safe() instead of list_for_each_safe() to simplify the code. Signed-off-by: Geliang Tang <geliangtang@163.com> --- drivers/usb/host/fotg210-hcd.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/drivers/usb/host/fotg210-hcd.c b/drivers/usb/host/fotg210-hcd.c index 2341af4..360a5e9 100644 --- a/drivers/usb/host/fotg210-hcd.c +++ b/drivers/usb/host/fotg210-hcd.c @@ -2267,7 +2267,7 @@ static unsigned qh_completions(struct fotg210_hcd *fotg210, struct fotg210_qh *qh) { struct fotg210_qtd *last, *end = qh->dummy; - struct list_head *entry, *tmp; + struct fotg210_qtd *qtd, *tmp; int last_status; int stopped; unsigned count = 0; @@ -2301,12 +2301,10 @@ rescan: * then let the queue advance. * if queue is stopped, handles unlinks. */ - list_for_each_safe(entry, tmp, &qh->qtd_list) { - struct fotg210_qtd *qtd; + list_for_each_entry_safe(qtd, tmp, &qh->qtd_list, qtd_list) { struct urb *urb; u32 token = 0; - qtd = list_entry(entry, struct fotg210_qtd, qtd_list); urb = qtd->urb; /* clean up any state from previous QTD ...*/ @@ -2544,14 +2542,11 @@ retry_xacterr: * used for cleanup after errors, before HC sees an URB's TDs. */ static void qtd_list_free(struct fotg210_hcd *fotg210, struct urb *urb, - struct list_head *qtd_list) + struct list_head *head) { - struct list_head *entry, *temp; - - list_for_each_safe(entry, temp, qtd_list) { - struct fotg210_qtd *qtd; + struct fotg210_qtd *qtd, *temp; - qtd = list_entry(entry, struct fotg210_qtd, qtd_list); + list_for_each_entry_safe(qtd, temp, head, qtd_list) { list_del(&qtd->qtd_list); fotg210_qtd_free(fotg210, qtd); } -- 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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox