RE: [PATCH v3] usb: gadget: forbid queuing request to a disabled ep
> > > > 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 Oops, I sent a wrong patch. I will send right patch again as v4, very sorry for this. The right patch has been verified on 3.14 by back-porting related 1 patch. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v3] usb: gadget: forbid queuing request to a disabled ep
Hi, changbin...@intel.com writes: > From: "Du, Changbin" > > 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 > --- > 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 signature.asc Description: PGP signature
Re: [PATCH v3] usb: gadget: forbid queuing request to a disabled ep
Hi, changbin...@intel.com writes: > From: "Du, Changbin"> > 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 > --- > 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 signature.asc Description: PGP signature
RE: [PATCH v3] usb: gadget: forbid queuing request to a disabled ep
> > > > 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 Oops, I sent a wrong patch. I will send right patch again as v4, very sorry for this. The right patch has been verified on 3.14 by back-porting related 1 patch. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/