All of lore.kernel.org
 help / color / mirror / Atom feed
From: Felipe Balbi <balbi@ti.com>
To: lkp@lists.01.org
Subject: Re: [PATCH v3] usb: gadget: forbid queuing request to a disabled ep
Date: Thu, 17 Dec 2015 09:26:29 -0600	[thread overview]
Message-ID: <87bn9pdrkq.fsf@saruman.tx.rr.com> (raw)
In-Reply-To: <1450346431-8064-1-git-send-email-changbin.du@intel.com>

[-- Attachment #1: Type: text/plain, Size: 1473 bytes --]


Hi,

changbin.du(a)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>
> ---
> change from v2: igonre ep0 as it always enabled during usb session.
> change from v1: add WARN_ON_ONCE message.
> ---
>  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..0c5d9ea 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 (WARN_ON_ONCE(!ep->enabled && !ep->address))

this will only trigger for a disabled ep0. Are you testing any of your
patches at all ?

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: Felipe Balbi <balbi@ti.com>
To: <changbin.du@intel.com>
Cc: <gregkh@linuxfoundation.org>, <John.Youn@synopsys.com>,
	<linux-usb@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<lkp@01.org>, <fengguang.wu@intel.com>,
	"Du, Changbin" <changbin.du@intel.com>
Subject: Re: [PATCH v3] usb: gadget: forbid queuing request to a disabled ep
Date: Thu, 17 Dec 2015 09:26:29 -0600	[thread overview]
Message-ID: <87bn9pdrkq.fsf@saruman.tx.rr.com> (raw)
In-Reply-To: <1450346431-8064-1-git-send-email-changbin.du@intel.com>

[-- Attachment #1: Type: text/plain, Size: 1471 bytes --]


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>
> ---
> change from v2: igonre ep0 as it always enabled during usb session.
> change from v1: add WARN_ON_ONCE message.
> ---
>  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..0c5d9ea 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 (WARN_ON_ONCE(!ep->enabled && !ep->address))

this will only trigger for a disabled ep0. Are you testing any of your
patches at all ?

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]

  reply	other threads:[~2015-12-17 15:26 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-30  5:21 [PATCH 0/2] Two fix for dwc2 gadget driver changbin.du
2015-11-30  5:21 ` [PATCH 1/2] usb: dwc2: add ep enabled flag to avoid double enable/disable changbin.du
2015-12-10 17:26   ` Felipe Balbi
2015-12-14  3:23     ` Du, Changbin
2015-11-30  5:21 ` [PATCH 2/2] usb: dwc2: forbid queuing request to a disabled ep changbin.du
2015-12-10 17:27   ` Felipe Balbi
2015-12-03  1:20 ` [PATCH 0/2] Two fix for dwc2 gadget driver John Youn
2015-12-03  4:23   ` Du, Changbin
2015-12-04  7:21     ` [PATCH] usb: gadget: forbid queuing request to a disabled ep changbin.du
2015-12-10 17:28       ` Felipe Balbi
2015-12-14  3:48         ` [PATCH v2] " changbin.du
2015-12-14 10:20           ` Du, Changbin
2015-12-14 10:20             ` Du, Changbin
2015-12-16 16:52             ` Felipe Balbi
2015-12-16 16:52               ` Felipe Balbi
2015-12-17  9:35               ` Du, Changbin
2015-12-17  9:35                 ` Du, Changbin
2015-12-17 10:00               ` [PATCH v3] " changbin.du
2015-12-17 10:00                 ` changbin.du
2015-12-17 15:26                 ` Felipe Balbi [this message]
2015-12-17 15:26                   ` Felipe Balbi
2015-12-18  7:34                   ` Du, Changbin
2015-12-18  7:34                     ` Du, Changbin
2015-12-18  7:36                   ` [PATCH v4] " changbin.du
2015-12-18  7:36                     ` changbin.du

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87bn9pdrkq.fsf@saruman.tx.rr.com \
    --to=balbi@ti.com \
    --cc=lkp@lists.01.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.