Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1282634 > unrolled thread

Re: [PATCH 0/2] Two fix for dwc2 gadget driver

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 0/2] Two fix for dwc2 gadget driver John Youn <John.Youn@synopsys.com> - 2015-12-03 02:30 +0100
    RE: [PATCH 0/2] Two fix for dwc2 gadget driver "Du, Changbin" <changbin.du@intel.com> - 2015-12-03 05:30 +0100
      [PATCH] usb: gadget: forbid queuing request to a disabled ep changbin.du@intel.com - 2015-12-04 08:30 +0100
        Re: [PATCH] usb: gadget: forbid queuing request to a disabled ep Felipe Balbi <balbi@ti.com> - 2015-12-10 18:30 +0100

#1282634 — Re: [PATCH 0/2] Two fix for dwc2 gadget driver

On 11/29/2015 9:29 PM, changbin.du@intel.com wrote:
> From: "Du, Changbin" <changbin.du@intel.com>
> 
> With the first patch, enable a enabled ep will return -EBUSY.
> The second patch forbid queuing on disabled ep to avoid panic.


The usb_ep->enabled flag was added in 4.4.

It looks like these same checks are also added at the API level in the
usb_ep_enable() and usb_ep_disable().

In case this is bypassed we should probably add them in the gadget
anyways but using the existing flag.

Regards,
John



> 
> Du, Changbin (2):
>   usb: dwc2: add ep enabled flag to avoid double enable/disable
>   usb: dwc2: forbid queuing request to a disabled ep
> 
>  drivers/usb/dwc2/core.h   |  1 +
>  drivers/usb/dwc2/gadget.c | 26 +++++++++++++++++++++++++-
>  2 files changed, 26 insertions(+), 1 deletion(-)
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1282694

> On 11/29/2015 9:29 PM, changbin.du@intel.com wrote:
> > From: "Du, Changbin" <changbin.du@intel.com>
> >
> > With the first patch, enable a enabled ep will return -EBUSY.
> > The second patch forbid queuing on disabled ep to avoid panic.
> 
> 
> The usb_ep->enabled flag was added in 4.4.
> 
> It looks like these same checks are also added at the API level in the
> usb_ep_enable() and usb_ep_disable().
> 
> In case this is bypassed we should probably add them in the gadget
> anyways but using the existing flag.
> 
> Regards,
> John
> 
Hmm, just learnt the flag on gadget API layer. And I just see usb_ep_enable return success if it is already enabled.
But I think it should return an error to inform the caller. Because the ep configuration may probably be changed.
In this case, usb_ep_enable will do different behavior.

Hmm, the usb_ep_queue doesn't check the enabled flag. Should be added. Let me have a try.

Best Regards,
Changbin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1283605 — [PATCH] usb: gadget: forbid queuing request to a disabled ep

From: "Du, Changbin" <changbin.du@intel.com>

Queue a request to disabled ep  doesn't make sense, and induce caller
make mistakes.

Here is a example for the android mtp gadget function driver. A mem
corruption can happen on below senario.
1) On disconnect, mtp driver disable its EPs,
2) During send_file_work and receive_file_work, mtp queues a request
   to ep. (The mtp driver need improve its synchronization logic!)
3) mtp_function_unbind is invoked and all mtp requests are freed.
4) when udc process the request queued on step 2, will cause kernel
   NULL pointer dereference exception.

Signed-off-by: Du, Changbin <changbin.du@intel.com>
---
This patch is seprated from below patches because gadget layer has
added the 'enabled' flag in v4.4. so abandon it and submit new one.
[PATCH 0/2] Two fix for dwc2 gadget driver
  usb: dwc2: add ep enabled flag to avoid double enable/disable
  usb: dwc2: forbid queuing request to a disabled ep

---
 include/linux/usb/gadget.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index 3d583a1..d813bd2 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -402,6 +402,9 @@ static inline void usb_ep_free_request(struct usb_ep *ep,
 static inline int usb_ep_queue(struct usb_ep *ep,
 			       struct usb_request *req, gfp_t gfp_flags)
 {
+	if (!ep->enabled)
+		return -ESHUTDOWN;
+
 	return ep->ops->queue(ep, req, gfp_flags);
 }
 
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1288700 — Re: [PATCH] usb: gadget: forbid queuing request to a disabled ep

[Multipart message — attachments visible in raw view] — view raw

Hi,

changbin.du@intel.com writes:
> From: "Du, Changbin" <changbin.du@intel.com>
>
> Queue a request to disabled ep  doesn't make sense, and induce caller
> make mistakes.
>
> Here is a example for the android mtp gadget function driver. A mem
> corruption can happen on below senario.
> 1) On disconnect, mtp driver disable its EPs,
> 2) During send_file_work and receive_file_work, mtp queues a request
>    to ep. (The mtp driver need improve its synchronization logic!)
> 3) mtp_function_unbind is invoked and all mtp requests are freed.
> 4) when udc process the request queued on step 2, will cause kernel
>    NULL pointer dereference exception.
>
> Signed-off-by: Du, Changbin <changbin.du@intel.com>
> ---
> This patch is seprated from below patches because gadget layer has
> added the 'enabled' flag in v4.4. so abandon it and submit new one.
> [PATCH 0/2] Two fix for dwc2 gadget driver
>   usb: dwc2: add ep enabled flag to avoid double enable/disable
>   usb: dwc2: forbid queuing request to a disabled ep
>
> ---
>  include/linux/usb/gadget.h | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
> index 3d583a1..d813bd2 100644
> --- a/include/linux/usb/gadget.h
> +++ b/include/linux/usb/gadget.h
> @@ -402,6 +402,9 @@ static inline void usb_ep_free_request(struct usb_ep *ep,
>  static inline int usb_ep_queue(struct usb_ep *ep,
>  			       struct usb_request *req, gfp_t gfp_flags)
>  {
> +	if (!ep->enabled)
> +		return -ESHUTDOWN;

same warn here:

	if (WARN_ON_ONCE(!ep->enabled))
        	return -ESHUTDOWN;

> +
>  	return ep->ops->queue(ep, req, gfp_flags);
>  }
>  
> -- 
> 2.5.0
>

-- 
balbi

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web