* [PATCH 1/3] usb: chipidea: udc: _ep_queue and _hw_queue cleanup @ 2015-09-18 17:30 eu 2015-09-18 17:30 ` [PATCH 2/3] usb: chipidea: udc: improve error handling on ep_queue eu 2015-09-18 17:30 ` [PATCH 3/3] usb: gadget: f_midi: check for error on usb_ep_queue eu 0 siblings, 2 replies; 7+ messages in thread From: eu @ 2015-09-18 17:30 UTC (permalink / raw) To: linux-usb Cc: linux-kernel, Peter Chen, Greg Kroah-Hartman, Felipe Balbi, Andrzej Pietrasiewicz, Felipe F. Tonello From: "Felipe F. Tonello" <eu@felipetonello.com> Update comments to reflect current state of functions. Signed-off-by: Felipe F. Tonello <eu@felipetonello.com> --- Changes for v2: - Introduced this patch. drivers/usb/chipidea/udc.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c index 764f668..c936c72 100644 --- a/drivers/usb/chipidea/udc.c +++ b/drivers/usb/chipidea/udc.c @@ -404,9 +404,9 @@ static inline u8 _usb_addr(struct ci_hw_ep *ep) } /** - * _hardware_queue: configures a request at hardware level - * @gadget: gadget + * _hardware_enqueue: configures a request at hardware level * @hwep: endpoint + * @hwreq: request * * This function returns an error code */ @@ -750,8 +750,12 @@ static void isr_get_status_complete(struct usb_ep *ep, struct usb_request *req) /** * _ep_queue: queues (submits) an I/O request to an endpoint + * @ep: endpoint + * @req: request + * @gfp_flags: GFP flags (not used) * * Caller must hold lock + * This function returns an error code */ static int _ep_queue(struct usb_ep *ep, struct usb_request *req, gfp_t __maybe_unused gfp_flags) -- 2.1.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/3] usb: chipidea: udc: improve error handling on ep_queue 2015-09-18 17:30 [PATCH 1/3] usb: chipidea: udc: _ep_queue and _hw_queue cleanup eu @ 2015-09-18 17:30 ` eu 2015-10-22 1:41 ` Peter Chen 2015-10-27 9:27 ` Felipe Ferreri Tonello 2015-09-18 17:30 ` [PATCH 3/3] usb: gadget: f_midi: check for error on usb_ep_queue eu 1 sibling, 2 replies; 7+ messages in thread From: eu @ 2015-09-18 17:30 UTC (permalink / raw) To: linux-usb Cc: linux-kernel, Peter Chen, Greg Kroah-Hartman, Felipe Balbi, Andrzej Pietrasiewicz, Felipe F. Tonello From: "Felipe F. Tonello" <eu@felipetonello.com> _ep_queue() didn't check for errors when using add_td_to_list() which can fail if dma_pool_alloc fails, thus causing a kernel panic when lastnode->ptr is NULL. Signed-off-by: Felipe F. Tonello <eu@felipetonello.com> --- Changes for v2: - Use separate patch for cleanups. drivers/usb/chipidea/udc.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c index c936c72..7169113e 100644 --- a/drivers/usb/chipidea/udc.c +++ b/drivers/usb/chipidea/udc.c @@ -435,19 +435,27 @@ static int _hardware_enqueue(struct ci_hw_ep *hwep, struct ci_hw_req *hwreq) if (hwreq->req.dma % PAGE_SIZE) pages--; - if (rest == 0) - add_td_to_list(hwep, hwreq, 0); + if (rest == 0) { + ret = add_td_to_list(hwep, hwreq, 0); + if (ret < 0) + goto done; + } while (rest > 0) { unsigned count = min(hwreq->req.length - hwreq->req.actual, (unsigned)(pages * CI_HDRC_PAGE_SIZE)); - add_td_to_list(hwep, hwreq, count); + ret = add_td_to_list(hwep, hwreq, count); + if (ret < 0) + goto done; rest -= count; } if (hwreq->req.zero && hwreq->req.length - && (hwreq->req.length % hwep->ep.maxpacket == 0)) - add_td_to_list(hwep, hwreq, 0); + && (hwreq->req.length % hwep->ep.maxpacket == 0)) { + ret = add_td_to_list(hwep, hwreq, 0); + if (ret < 0) + goto done; + } firstnode = list_first_entry(&hwreq->tds, struct td_node, td); -- 2.1.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/3] usb: chipidea: udc: improve error handling on ep_queue 2015-09-18 17:30 ` [PATCH 2/3] usb: chipidea: udc: improve error handling on ep_queue eu @ 2015-10-22 1:41 ` Peter Chen 2015-10-27 9:27 ` Felipe Ferreri Tonello 1 sibling, 0 replies; 7+ messages in thread From: Peter Chen @ 2015-10-22 1:41 UTC (permalink / raw) To: eu Cc: linux-usb, linux-kernel, Greg Kroah-Hartman, Felipe Balbi, Andrzej Pietrasiewicz On Fri, Sep 18, 2015 at 06:30:20PM +0100, eu@felipetonello.com wrote: > From: "Felipe F. Tonello" <eu@felipetonello.com> > > _ep_queue() didn't check for errors when using add_td_to_list() > which can fail if dma_pool_alloc fails, thus causing a kernel > panic when lastnode->ptr is NULL. > > Signed-off-by: Felipe F. Tonello <eu@felipetonello.com> > --- > > Changes for v2: > - Use separate patch for cleanups. > > drivers/usb/chipidea/udc.c | 18 +++++++++++++----- > 1 file changed, 13 insertions(+), 5 deletions(-) > > diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c > index c936c72..7169113e 100644 > --- a/drivers/usb/chipidea/udc.c > +++ b/drivers/usb/chipidea/udc.c > @@ -435,19 +435,27 @@ static int _hardware_enqueue(struct ci_hw_ep *hwep, struct ci_hw_req *hwreq) > if (hwreq->req.dma % PAGE_SIZE) > pages--; > > - if (rest == 0) > - add_td_to_list(hwep, hwreq, 0); > + if (rest == 0) { > + ret = add_td_to_list(hwep, hwreq, 0); > + if (ret < 0) > + goto done; > + } > > while (rest > 0) { > unsigned count = min(hwreq->req.length - hwreq->req.actual, > (unsigned)(pages * CI_HDRC_PAGE_SIZE)); > - add_td_to_list(hwep, hwreq, count); > + ret = add_td_to_list(hwep, hwreq, count); > + if (ret < 0) > + goto done; > rest -= count; > } > > if (hwreq->req.zero && hwreq->req.length > - && (hwreq->req.length % hwep->ep.maxpacket == 0)) > - add_td_to_list(hwep, hwreq, 0); > + && (hwreq->req.length % hwep->ep.maxpacket == 0)) { > + ret = add_td_to_list(hwep, hwreq, 0); > + if (ret < 0) > + goto done; > + } > > firstnode = list_first_entry(&hwreq->tds, struct td_node, td); > > -- > 2.1.4 > Hi Felipe, I can't apply it in my tree, would you please create it based on chipdea tree? https://git.kernel.org/pub/scm/linux/kernel/git/peter.chen/usb.git branch: ci-for-usb-next -- Best Regards, Peter Chen ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/3] usb: chipidea: udc: improve error handling on ep_queue 2015-09-18 17:30 ` [PATCH 2/3] usb: chipidea: udc: improve error handling on ep_queue eu 2015-10-22 1:41 ` Peter Chen @ 2015-10-27 9:27 ` Felipe Ferreri Tonello 2015-10-27 9:29 ` Peter Chen 1 sibling, 1 reply; 7+ messages in thread From: Felipe Ferreri Tonello @ 2015-10-27 9:27 UTC (permalink / raw) To: linux-usb Cc: linux-kernel, Peter Chen, Greg Kroah-Hartman, Felipe Balbi, Andrzej Pietrasiewicz Hi Peter, Have you seen this patch? I saw that you didn't apply it to your tree, so I wonder if it is good or do I have to change anything. This patch is a bug fix for a memory leak, so it is quite important. -- Felipe On 18/09/15 18:30, eu@felipetonello.com wrote: > From: "Felipe F. Tonello" <eu@felipetonello.com> > > _ep_queue() didn't check for errors when using add_td_to_list() > which can fail if dma_pool_alloc fails, thus causing a kernel > panic when lastnode->ptr is NULL. > > Signed-off-by: Felipe F. Tonello <eu@felipetonello.com> > --- > > Changes for v2: > - Use separate patch for cleanups. > > drivers/usb/chipidea/udc.c | 18 +++++++++++++----- > 1 file changed, 13 insertions(+), 5 deletions(-) > > diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c > index c936c72..7169113e 100644 > --- a/drivers/usb/chipidea/udc.c > +++ b/drivers/usb/chipidea/udc.c > @@ -435,19 +435,27 @@ static int _hardware_enqueue(struct ci_hw_ep *hwep, struct ci_hw_req *hwreq) > if (hwreq->req.dma % PAGE_SIZE) > pages--; > > - if (rest == 0) > - add_td_to_list(hwep, hwreq, 0); > + if (rest == 0) { > + ret = add_td_to_list(hwep, hwreq, 0); > + if (ret < 0) > + goto done; > + } > > while (rest > 0) { > unsigned count = min(hwreq->req.length - hwreq->req.actual, > (unsigned)(pages * CI_HDRC_PAGE_SIZE)); > - add_td_to_list(hwep, hwreq, count); > + ret = add_td_to_list(hwep, hwreq, count); > + if (ret < 0) > + goto done; > rest -= count; > } > > if (hwreq->req.zero && hwreq->req.length > - && (hwreq->req.length % hwep->ep.maxpacket == 0)) > - add_td_to_list(hwep, hwreq, 0); > + && (hwreq->req.length % hwep->ep.maxpacket == 0)) { > + ret = add_td_to_list(hwep, hwreq, 0); > + if (ret < 0) > + goto done; > + } > > firstnode = list_first_entry(&hwreq->tds, struct td_node, td); > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH 2/3] usb: chipidea: udc: improve error handling on ep_queue 2015-10-27 9:27 ` Felipe Ferreri Tonello @ 2015-10-27 9:29 ` Peter Chen 0 siblings, 0 replies; 7+ messages in thread From: Peter Chen @ 2015-10-27 9:29 UTC (permalink / raw) To: Felipe Ferreri Tonello, linux-usb Cc: linux-kernel, Greg Kroah-Hartman, Felipe Balbi, Andrzej Pietrasiewicz > Hi Peter, > > Have you seen this patch? I saw that you didn't apply it to your tree, so I > wonder if it is good or do I have to change anything. > > This patch is a bug fix for a memory leak, so it is quite important. > Would you please create it based on my tree, branch ci-for-usb-next? I can't apply it. Peter > -- > Felipe > > On 18/09/15 18:30, eu@felipetonello.com wrote: > > From: "Felipe F. Tonello" <eu@felipetonello.com> > > > > _ep_queue() didn't check for errors when using add_td_to_list() which > > can fail if dma_pool_alloc fails, thus causing a kernel panic when > > lastnode->ptr is NULL. > > > > Signed-off-by: Felipe F. Tonello <eu@felipetonello.com> > > --- > > > > Changes for v2: > > - Use separate patch for cleanups. > > > > drivers/usb/chipidea/udc.c | 18 +++++++++++++----- > > 1 file changed, 13 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c > > index c936c72..7169113e 100644 > > --- a/drivers/usb/chipidea/udc.c > > +++ b/drivers/usb/chipidea/udc.c > > @@ -435,19 +435,27 @@ static int _hardware_enqueue(struct ci_hw_ep > *hwep, struct ci_hw_req *hwreq) > > if (hwreq->req.dma % PAGE_SIZE) > > pages--; > > > > - if (rest == 0) > > - add_td_to_list(hwep, hwreq, 0); > > + if (rest == 0) { > > + ret = add_td_to_list(hwep, hwreq, 0); > > + if (ret < 0) > > + goto done; > > + } > > > > while (rest > 0) { > > unsigned count = min(hwreq->req.length - hwreq->req.actual, > > (unsigned)(pages * > CI_HDRC_PAGE_SIZE)); > > - add_td_to_list(hwep, hwreq, count); > > + ret = add_td_to_list(hwep, hwreq, count); > > + if (ret < 0) > > + goto done; > > rest -= count; > > } > > > > if (hwreq->req.zero && hwreq->req.length > > - && (hwreq->req.length % hwep->ep.maxpacket == 0)) > > - add_td_to_list(hwep, hwreq, 0); > > + && (hwreq->req.length % hwep->ep.maxpacket == 0)) { > > + ret = add_td_to_list(hwep, hwreq, 0); > > + if (ret < 0) > > + goto done; > > + } > > > > firstnode = list_first_entry(&hwreq->tds, struct td_node, td); > > > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 3/3] usb: gadget: f_midi: check for error on usb_ep_queue 2015-09-18 17:30 [PATCH 1/3] usb: chipidea: udc: _ep_queue and _hw_queue cleanup eu 2015-09-18 17:30 ` [PATCH 2/3] usb: chipidea: udc: improve error handling on ep_queue eu @ 2015-09-18 17:30 ` eu 2015-09-18 17:32 ` Felipe Tonello 1 sibling, 1 reply; 7+ messages in thread From: eu @ 2015-09-18 17:30 UTC (permalink / raw) To: linux-usb Cc: linux-kernel, Peter Chen, Greg Kroah-Hartman, Felipe Balbi, Andrzej Pietrasiewicz, Felipe F. Tonello From: "Felipe F. Tonello" <eu@felipetonello.com> f_midi is not checking weather the is an error on usb_ep_queue request, ignoring potential problems, such as memory leaks. Signed-off-by: Felipe F. Tonello <eu@felipetonello.com> --- Changes for v2: - Update code style. drivers/usb/gadget/function/f_midi.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c index ad50a67..c04133422 100644 --- a/drivers/usb/gadget/function/f_midi.c +++ b/drivers/usb/gadget/function/f_midi.c @@ -543,10 +543,16 @@ static void f_midi_transmit(struct f_midi *midi, struct usb_request *req) } } - if (req->length > 0) - usb_ep_queue(ep, req, GFP_ATOMIC); - else + if (req->length > 0) { + int err; + + err = usb_ep_queue(ep, req, GFP_ATOMIC); + if (err < 0) + ERROR(midi, "%s queue req: %d\n", + midi->out_ep->name, err); + } else { free_ep_req(ep, req); + } } static void f_midi_in_tasklet(unsigned long data) -- 2.1.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] usb: gadget: f_midi: check for error on usb_ep_queue 2015-09-18 17:30 ` [PATCH 3/3] usb: gadget: f_midi: check for error on usb_ep_queue eu @ 2015-09-18 17:32 ` Felipe Tonello 0 siblings, 0 replies; 7+ messages in thread From: Felipe Tonello @ 2015-09-18 17:32 UTC (permalink / raw) To: USB list Cc: linux-kernel, Peter Chen, Greg Kroah-Hartman, Felipe Balbi, Andrzej Pietrasiewicz, Felipe F. Tonello On Fri, Sep 18, 2015 at 6:30 PM, <eu@felipetonello.com> wrote: > From: "Felipe F. Tonello" <eu@felipetonello.com> > > f_midi is not checking weather the is an error on usb_ep_queue > request, ignoring potential problems, such as memory leaks. > > Signed-off-by: Felipe F. Tonello <eu@felipetonello.com> > --- > > Changes for v2: > - Update code style. > > drivers/usb/gadget/function/f_midi.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c > index ad50a67..c04133422 100644 > --- a/drivers/usb/gadget/function/f_midi.c > +++ b/drivers/usb/gadget/function/f_midi.c > @@ -543,10 +543,16 @@ static void f_midi_transmit(struct f_midi *midi, struct usb_request *req) > } > } > > - if (req->length > 0) > - usb_ep_queue(ep, req, GFP_ATOMIC); > - else > + if (req->length > 0) { > + int err; > + > + err = usb_ep_queue(ep, req, GFP_ATOMIC); > + if (err < 0) > + ERROR(midi, "%s queue req: %d\n", > + midi->out_ep->name, err); I just realised there is a problem here. It is in_ep in this case. I will fix it in v3. > + } else { > free_ep_req(ep, req); > + } > } > > static void f_midi_in_tasklet(unsigned long data) > -- > 2.1.4 > ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-10-27 9:30 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-09-18 17:30 [PATCH 1/3] usb: chipidea: udc: _ep_queue and _hw_queue cleanup eu 2015-09-18 17:30 ` [PATCH 2/3] usb: chipidea: udc: improve error handling on ep_queue eu 2015-10-22 1:41 ` Peter Chen 2015-10-27 9:27 ` Felipe Ferreri Tonello 2015-10-27 9:29 ` Peter Chen 2015-09-18 17:30 ` [PATCH 3/3] usb: gadget: f_midi: check for error on usb_ep_queue eu 2015-09-18 17:32 ` Felipe Tonello
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).