lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <alpine.DEB.2.10.1512181851100.2499@hadrien>
Date:	Fri, 18 Dec 2015 18:53:19 +0100 (CET)
From:	Julia Lawall <julia.lawall@...6.fr>
To:	Geliang Tang <geliangtang@....com>
cc:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Sergei Shtylyov <sergei.shtylyov@...entembedded.com>,
	linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/9] usb: host: max3421-hcd: use list_for_each_entry*

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@...org
> In-Reply-To: <45e8397e370ed99ceb8aa1719c2b56a7ce741eb3.1450455485.git.geliangtang@....com>
> TO: Geliang Tang <geliangtang@....com>
> CC: Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Sergei Shtylyov <sergei.shtylyov@...entembedded.com>
> CC: Geliang Tang <geliangtang@....com>, linux-usb@...r.kernel.org, linux-kernel@...r.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@...uge.net>
> :::::: CC: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
>
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ