linux-kernel - RE: [PATCH] drivers: usb: dwc2: remove 'force' parameter from kill_all

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-prev] [day] [month] [year] [list]

Date:	Mon, 5 Jan 2015 19:41:09 +0000
From:	Paul Zimmerman <Paul.Zimmerman@...opsys.com>
To:	Robert Baldyga <r.baldyga@...sung.com>,
	"balbi@...com" <balbi@...com>
CC:	"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
	"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"peter.chen@...escale.com" <peter.chen@...escale.com>,
	"stern@...land.harvard.edu" <stern@...land.harvard.edu>,
	"k.opasiak@...sung.com" <k.opasiak@...sung.com>,
	"m.szyprowski@...sung.com" <m.szyprowski@...sung.com>
Subject: RE: [PATCH] drivers: usb: dwc2: remove 'force' parameter from
 kill_all_requests()

> From: Robert Baldyga [mailto:r.baldyga@...sung.com]
> Sent: Tuesday, December 16, 2014 2:52 AM
> 
> This patch fixes in simpler way the bug described in [1] and [2]. It
> looks like DWC2 is the only UDC driver that doesn't force usb requests
> to complete in ep_disable() function. This causes described problem,
> because we have no guarantee that all requests will be completed before
> unbind of usb function.
> 
> To fix this problem we force all requests of disabled endpoint to complete.
> Also currently running request is not handled. This allowed to simplify
> code of kill_all_requests() function, because 'force' parameter is always
> set to true, so we don't need it anymore.
> 
> In s3c_hsotg_rx_data() we change function used to print message when active
> request is NULL from dev_warn() to dev_dbg(), because such situation is
> harmless for driver and now it can take place during normal endpoint
> disabling.
> 
> [1] https://lkml.org/lkml/2014/12/9/283
> [2] https://lkml.org/lkml/2014/12/12/360
> 
> Signed-off-by: Robert Baldyga <r.baldyga@...sung.com>
> ---
>  drivers/usb/dwc2/gadget.c | 23 ++++++++---------------
>  1 file changed, 8 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index 200168e..fee0ad5 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -1305,7 +1305,7 @@ static void s3c_hsotg_rx_data(struct dwc2_hsotg *hsotg, int ep_idx, int size)
>  		u32 epctl = readl(hsotg->regs + DOEPCTL(ep_idx));
>  		int ptr;
> 
> -		dev_warn(hsotg->dev,
> +		dev_dbg(hsotg->dev,
>  			 "%s: FIFO %d bytes on ep%d but no req (DXEPCTl=0x%08x)\n",
>  			 __func__, size, ep_idx, epctl);
> 
> @@ -1988,30 +1988,23 @@ static void s3c_hsotg_irq_enumdone(struct dwc2_hsotg *hsotg)
>   * @hsotg: The device state.
>   * @ep: The endpoint the requests may be on.
>   * @result: The result code to use.
> - * @force: Force removal of any current requests
>   *
>   * Go through the requests on the given endpoint and mark them
>   * completed with the given result code.
>   */
>  static void kill_all_requests(struct dwc2_hsotg *hsotg,
>  			      struct s3c_hsotg_ep *ep,
> -			      int result, bool force)
> +			      int result)
>  {
>  	struct s3c_hsotg_req *req, *treq;
>  	unsigned size;
> 
> -	list_for_each_entry_safe(req, treq, &ep->queue, queue) {
> -		/*
> -		 * currently, we can't do much about an already
> -		 * running request on an in endpoint
> -		 */
> -
> -		if (ep->req == req && ep->dir_in && !force)
> -			continue;
> +	ep->req = NULL;
> 
> +	list_for_each_entry_safe(req, treq, &ep->queue, queue)
>  		s3c_hsotg_complete_request(hsotg, ep, req,
>  					   result);
> -	}
> +
>  	if (!hsotg->dedicated_fifos)
>  		return;
>  	size = (readl(hsotg->regs + DTXFSTS(ep->index)) & 0xffff) * 4;
> @@ -2036,7 +2029,7 @@ void s3c_hsotg_disconnect(struct dwc2_hsotg *hsotg)
> 
>  	hsotg->connected = 0;
>  	for (ep = 0; ep < hsotg->num_of_eps; ep++)
> -		kill_all_requests(hsotg, &hsotg->eps[ep], -ESHUTDOWN, true);
> +		kill_all_requests(hsotg, &hsotg->eps[ep], -ESHUTDOWN);
> 
>  	call_gadget(hsotg, disconnect);
>  }
> @@ -2334,7 +2327,7 @@ irq_retry:
>  				       msecs_to_jiffies(200))) {
> 
>  				kill_all_requests(hsotg, &hsotg->eps[0],
> -							  -ECONNRESET, true);
> +							  -ECONNRESET);
> 
>  				s3c_hsotg_core_init_disconnected(hsotg);
>  				s3c_hsotg_core_connect(hsotg);
> @@ -2588,7 +2581,7 @@ static int s3c_hsotg_ep_disable(struct usb_ep *ep)
> 
>  	spin_lock_irqsave(&hsotg->lock, flags);
>  	/* terminate all requests with shutdown */
> -	kill_all_requests(hsotg, hs_ep, -ESHUTDOWN, false);
> +	kill_all_requests(hsotg, hs_ep, -ESHUTDOWN);
> 
>  	hsotg->fifo_map &= ~(1<<hs_ep->fifo_index);
>  	hs_ep->fifo_index = 0;
> --
> 1.9.1

Acked-by: Paul Zimmerman <paulz@...opsys.com>

--
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/