* [PATCH 0/2] improve error handling on chipidea/udc.c and f_midi.c
@ 2015-09-18 17:12 eu
2015-09-18 17:12 ` [PATCH 1/2] usb: chipidea: udc: improve error handling on ep_queue eu
` (2 more replies)
0 siblings, 3 replies; 15+ messages in thread
From: eu @ 2015-09-18 17:12 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>
Felipe F. Tonello (2):
usb: chipidea: udc: improve error handling on ep_queue
usb: gadget: f_midi: check for error on usb_ep_queue
drivers/usb/chipidea/udc.c | 26 +++++++++++++++++++-------
drivers/usb/gadget/function/f_midi.c | 10 ++++++++--
2 files changed, 27 insertions(+), 9 deletions(-)
--
2.1.4
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/2] usb: chipidea: udc: improve error handling on ep_queue
2015-09-18 17:12 [PATCH 0/2] improve error handling on chipidea/udc.c and f_midi.c eu
@ 2015-09-18 17:12 ` eu
2015-09-18 17:17 ` Felipe Balbi
2015-09-21 6:29 ` Peter Chen
2015-09-18 17:12 ` [PATCH 2/2] usb: gadget: f_midi: check for error on usb_ep_queue eu
2015-09-18 20:56 ` [PATCH 0/2] improve error handling on chipidea/udc.c and f_midi.c Albino B Neto
2 siblings, 2 replies; 15+ messages in thread
From: eu @ 2015-09-18 17:12 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>
---
drivers/usb/chipidea/udc.c | 26 +++++++++++++++++++-------
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index 764f668..7169113e 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
*/
@@ -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);
@@ -750,8 +758,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] 15+ messages in thread
* [PATCH 2/2] usb: gadget: f_midi: check for error on usb_ep_queue
2015-09-18 17:12 [PATCH 0/2] improve error handling on chipidea/udc.c and f_midi.c eu
2015-09-18 17:12 ` [PATCH 1/2] usb: chipidea: udc: improve error handling on ep_queue eu
@ 2015-09-18 17:12 ` eu
2015-09-18 17:19 ` Felipe Balbi
` (2 more replies)
2015-09-18 20:56 ` [PATCH 0/2] improve error handling on chipidea/udc.c and f_midi.c Albino B Neto
2 siblings, 3 replies; 15+ messages in thread
From: eu @ 2015-09-18 17:12 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>
---
drivers/usb/gadget/function/f_midi.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c
index ad50a67..a5e446d 100644
--- a/drivers/usb/gadget/function/f_midi.c
+++ b/drivers/usb/gadget/function/f_midi.c
@@ -543,8 +543,14 @@ static void f_midi_transmit(struct f_midi *midi, struct usb_request *req)
}
}
- if (req->length > 0)
- usb_ep_queue(ep, req, GFP_ATOMIC);
+ 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);
}
--
2.1.4
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] usb: chipidea: udc: improve error handling on ep_queue
2015-09-18 17:12 ` [PATCH 1/2] usb: chipidea: udc: improve error handling on ep_queue eu
@ 2015-09-18 17:17 ` Felipe Balbi
2015-09-18 17:30 ` Felipe Tonello
2015-09-21 6:29 ` Peter Chen
1 sibling, 1 reply; 15+ messages in thread
From: Felipe Balbi @ 2015-09-18 17:17 UTC (permalink / raw)
To: eu
Cc: linux-usb, linux-kernel, Peter Chen, Greg Kroah-Hartman,
Felipe Balbi, Andrzej Pietrasiewicz
[-- Attachment #1: Type: text/plain, Size: 2896 bytes --]
Hi,
On Fri, Sep 18, 2015 at 06:12:40PM +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>
this can still be split down further.
> ---
> drivers/usb/chipidea/udc.c | 26 +++++++++++++++++++-------
> 1 file changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> index 764f668..7169113e 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 is a cleanup and you shouldn't have a fix depending on a
cleanup. Fixes are merged during the -rc cycle, while cleanups will be
deferred to the following merge window.
> *
> * This function returns an error code
> */
> @@ -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;
> + }
this is your fix.
>
> 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;
and this
> 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;
> + }
>
and this.
> firstnode = list_first_entry(&hwreq->tds, struct td_node, td);
>
> @@ -750,8 +758,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)
cleanup
> *
> * Caller must hold lock
> + * This function returns an error code
somewhat pointless, but could come with the cleanup, no strong
feelings.
> */
> static int _ep_queue(struct usb_ep *ep, struct usb_request *req,
> gfp_t __maybe_unused gfp_flags)
> --
> 2.1.4
>
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] usb: gadget: f_midi: check for error on usb_ep_queue
2015-09-18 17:12 ` [PATCH 2/2] usb: gadget: f_midi: check for error on usb_ep_queue eu
@ 2015-09-18 17:19 ` Felipe Balbi
2015-09-19 16:09 ` Sergei Shtylyov
2015-09-21 6:30 ` Peter Chen
2 siblings, 0 replies; 15+ messages in thread
From: Felipe Balbi @ 2015-09-18 17:19 UTC (permalink / raw)
To: eu
Cc: linux-usb, linux-kernel, Peter Chen, Greg Kroah-Hartman,
Felipe Balbi, Andrzej Pietrasiewicz
[-- Attachment #1: Type: text/plain, Size: 1271 bytes --]
On Fri, Sep 18, 2015 at 06:12:41PM +0100, 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>
> ---
> drivers/usb/gadget/function/f_midi.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c
> index ad50a67..a5e446d 100644
> --- a/drivers/usb/gadget/function/f_midi.c
> +++ b/drivers/usb/gadget/function/f_midi.c
> @@ -543,8 +543,14 @@ static void f_midi_transmit(struct f_midi *midi, struct usb_request *req)
> }
> }
>
> - if (req->length > 0)
> - usb_ep_queue(ep, req, GFP_ATOMIC);
> + 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
yeah, cool, but you need to fix the style here. This else needs
to be after the curly brace and you need to curly brace to the
else branch too.
> free_ep_req(ep, req);
> }
> --
> 2.1.4
>
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] usb: chipidea: udc: improve error handling on ep_queue
2015-09-18 17:17 ` Felipe Balbi
@ 2015-09-18 17:30 ` Felipe Tonello
0 siblings, 0 replies; 15+ messages in thread
From: Felipe Tonello @ 2015-09-18 17:30 UTC (permalink / raw)
To: Felipe Balbi
Cc: USB list, linux-kernel, Peter Chen, Greg Kroah-Hartman,
Andrzej Pietrasiewicz
Hi Felipe,
On Fri, Sep 18, 2015 at 6:17 PM, Felipe Balbi <balbi@ti.com> wrote:
> Hi,
>
> On Fri, Sep 18, 2015 at 06:12:40PM +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>
>
> this can still be split down further.
>
>> ---
>> drivers/usb/chipidea/udc.c | 26 +++++++++++++++++++-------
>> 1 file changed, 19 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
>> index 764f668..7169113e 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 is a cleanup and you shouldn't have a fix depending on a
> cleanup. Fixes are merged during the -rc cycle, while cleanups will be
> deferred to the following merge window.
Got it.
>
>> *
>> * This function returns an error code
>> */
>> @@ -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;
>> + }
>
> this is your fix.
>
>>
>> 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;
>
> and this
>
>> 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;
>> + }
>>
>
>
> and this.
>
>> firstnode = list_first_entry(&hwreq->tds, struct td_node, td);
>>
>> @@ -750,8 +758,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)
>
> cleanup
>
>> *
>> * Caller must hold lock
>> + * This function returns an error code
>
> somewhat pointless, but could come with the cleanup, no strong
> feelings.
>
>> */
>> static int _ep_queue(struct usb_ep *ep, struct usb_request *req,
>> gfp_t __maybe_unused gfp_flags)
>> --
>> 2.1.4
>>
>
> --
> balbi
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/2] improve error handling on chipidea/udc.c and f_midi.c
2015-09-18 17:12 [PATCH 0/2] improve error handling on chipidea/udc.c and f_midi.c eu
2015-09-18 17:12 ` [PATCH 1/2] usb: chipidea: udc: improve error handling on ep_queue eu
2015-09-18 17:12 ` [PATCH 2/2] usb: gadget: f_midi: check for error on usb_ep_queue eu
@ 2015-09-18 20:56 ` Albino B Neto
2015-09-18 20:58 ` Felipe Balbi
2 siblings, 1 reply; 15+ messages in thread
From: Albino B Neto @ 2015-09-18 20:56 UTC (permalink / raw)
To: eu
Cc: linux-usb, LKML, Peter Chen, Greg Kroah-Hartman, Felipe Balbi,
Andrzej Pietrasiewicz
2015-09-18 14:12 GMT-03:00 <eu@felipetonello.com>:
> From: "Felipe F. Tonello" <eu@felipetonello.com>
Signed-off-by ?
Albino
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 0/2] improve error handling on chipidea/udc.c and f_midi.c
2015-09-18 20:56 ` [PATCH 0/2] improve error handling on chipidea/udc.c and f_midi.c Albino B Neto
@ 2015-09-18 20:58 ` Felipe Balbi
0 siblings, 0 replies; 15+ messages in thread
From: Felipe Balbi @ 2015-09-18 20:58 UTC (permalink / raw)
To: Albino B Neto
Cc: eu, linux-usb, LKML, Peter Chen, Greg Kroah-Hartman,
Felipe Balbi, Andrzej Pietrasiewicz
[-- Attachment #1: Type: text/plain, Size: 233 bytes --]
On Fri, Sep 18, 2015 at 05:56:48PM -0300, Albino B Neto wrote:
> 2015-09-18 14:12 GMT-03:00 <eu@felipetonello.com>:
> > From: "Felipe F. Tonello" <eu@felipetonello.com>
>
> Signed-off-by ?
this is not a patch
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] usb: gadget: f_midi: check for error on usb_ep_queue
2015-09-18 17:12 ` [PATCH 2/2] usb: gadget: f_midi: check for error on usb_ep_queue eu
2015-09-18 17:19 ` Felipe Balbi
@ 2015-09-19 16:09 ` Sergei Shtylyov
2015-09-21 6:30 ` Peter Chen
2 siblings, 0 replies; 15+ messages in thread
From: Sergei Shtylyov @ 2015-09-19 16:09 UTC (permalink / raw)
To: eu, linux-usb
Cc: linux-kernel, Peter Chen, Greg Kroah-Hartman, Felipe Balbi,
Andrzej Pietrasiewicz
Hello.
On 9/18/2015 8:12 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>
> ---
> drivers/usb/gadget/function/f_midi.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c
> index ad50a67..a5e446d 100644
> --- a/drivers/usb/gadget/function/f_midi.c
> +++ b/drivers/usb/gadget/function/f_midi.c
> @@ -543,8 +543,14 @@ static void f_midi_transmit(struct f_midi *midi, struct usb_request *req)
> }
> }
>
> - if (req->length > 0)
> - usb_ep_queue(ep, req, GFP_ATOMIC);
> + 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
} else {
> free_ep_req(ep, req);
}
> }
The lines added above show the proper kernel CodingStyle. } should be on
the same line as *else* and {} should be used in all branches of the *if*
statement if at least one branch has them.
MBR, Sergei
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] usb: chipidea: udc: improve error handling on ep_queue
2015-09-18 17:12 ` [PATCH 1/2] usb: chipidea: udc: improve error handling on ep_queue eu
2015-09-18 17:17 ` Felipe Balbi
@ 2015-09-21 6:29 ` Peter Chen
2015-09-21 9:17 ` Felipe Tonello
1 sibling, 1 reply; 15+ messages in thread
From: Peter Chen @ 2015-09-21 6:29 UTC (permalink / raw)
To: eu
Cc: linux-usb, linux-kernel, Greg Kroah-Hartman, Felipe Balbi,
Andrzej Pietrasiewicz
On Fri, Sep 18, 2015 at 06:12:40PM +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
Would you find the root cause why dma_pool_alloc fails?
> panic when lastnode->ptr is NULL.
>
> Signed-off-by: Felipe F. Tonello <eu@felipetonello.com>
> ---
> drivers/usb/chipidea/udc.c | 26 +++++++++++++++++++-------
> 1 file changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
> index 764f668..7169113e 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
> */
> @@ -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);
>
> @@ -750,8 +758,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
>
--
Best Regards,
Peter Chen
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] usb: gadget: f_midi: check for error on usb_ep_queue
2015-09-18 17:12 ` [PATCH 2/2] usb: gadget: f_midi: check for error on usb_ep_queue eu
2015-09-18 17:19 ` Felipe Balbi
2015-09-19 16:09 ` Sergei Shtylyov
@ 2015-09-21 6:30 ` Peter Chen
2015-09-21 8:16 ` Felipe Tonello
2 siblings, 1 reply; 15+ messages in thread
From: Peter Chen @ 2015-09-21 6:30 UTC (permalink / raw)
To: eu
Cc: linux-usb, linux-kernel, Greg Kroah-Hartman, Felipe Balbi,
Andrzej Pietrasiewicz
On Fri, Sep 18, 2015 at 06:12:41PM +0100, 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
%s/weather/whether
%s/the/there
> request, ignoring potential problems, such as memory leaks.
>
> Signed-off-by: Felipe F. Tonello <eu@felipetonello.com>
> ---
> drivers/usb/gadget/function/f_midi.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/f_midi.c b/drivers/usb/gadget/function/f_midi.c
> index ad50a67..a5e446d 100644
> --- a/drivers/usb/gadget/function/f_midi.c
> +++ b/drivers/usb/gadget/function/f_midi.c
> @@ -543,8 +543,14 @@ static void f_midi_transmit(struct f_midi *midi, struct usb_request *req)
> }
> }
>
> - if (req->length > 0)
> - usb_ep_queue(ep, req, GFP_ATOMIC);
> + 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);
> }
> --
> 2.1.4
>
--
Best Regards,
Peter Chen
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] usb: gadget: f_midi: check for error on usb_ep_queue
2015-09-21 8:16 ` Felipe Tonello
@ 2015-09-21 7:49 ` Peter Chen
2015-09-21 9:18 ` Felipe Tonello
0 siblings, 1 reply; 15+ messages in thread
From: Peter Chen @ 2015-09-21 7:49 UTC (permalink / raw)
To: Felipe Tonello
Cc: USB list, Kernel development list, Greg Kroah-Hartman,
Felipe Balbi, Andrzej Pietrasiewicz
On Mon, Sep 21, 2015 at 09:16:05AM +0100, Felipe Tonello wrote:
> Hi Chen,
>
> On Mon, Sep 21, 2015 at 7:30 AM, Peter Chen <peter.chen@freescale.com> wrote:
> > On Fri, Sep 18, 2015 at 06:12:41PM +0100, 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
> >
> > %s/weather/whether
> > %s/the/there
>
> I fixed it on v3. Did you receive it?
>
oh, get it, You may add "--subject-prefix="PATCH v3" when you format
patch, in that case, the reader can see "PATCH v3 2/2" at the subject,
and know it is the third version.
Your two changes for chipidea are ok, I will queue them.
--
Best Regards,
Peter Chen
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] usb: gadget: f_midi: check for error on usb_ep_queue
2015-09-21 6:30 ` Peter Chen
@ 2015-09-21 8:16 ` Felipe Tonello
2015-09-21 7:49 ` Peter Chen
0 siblings, 1 reply; 15+ messages in thread
From: Felipe Tonello @ 2015-09-21 8:16 UTC (permalink / raw)
To: Peter Chen
Cc: USB list, Kernel development list, Greg Kroah-Hartman,
Felipe Balbi, Andrzej Pietrasiewicz
Hi Chen,
On Mon, Sep 21, 2015 at 7:30 AM, Peter Chen <peter.chen@freescale.com> wrote:
> On Fri, Sep 18, 2015 at 06:12:41PM +0100, 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
>
> %s/weather/whether
> %s/the/there
I fixed it on v3. Did you receive it?
Felipe
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] usb: chipidea: udc: improve error handling on ep_queue
2015-09-21 6:29 ` Peter Chen
@ 2015-09-21 9:17 ` Felipe Tonello
0 siblings, 0 replies; 15+ messages in thread
From: Felipe Tonello @ 2015-09-21 9:17 UTC (permalink / raw)
To: Peter Chen
Cc: USB list, Kernel development list, Greg Kroah-Hartman,
Felipe Balbi, Andrzej Pietrasiewicz
Hi Peter,
On Mon, Sep 21, 2015 at 7:29 AM, Peter Chen <peter.chen@freescale.com> wrote:
> On Fri, Sep 18, 2015 at 06:12:40PM +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
>
> Would you find the root cause why dma_pool_alloc fails?
Partially. For some reason the udc_irq() is not been called after a
f_midi_transmit(). I am looking into that.
Felipe
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] usb: gadget: f_midi: check for error on usb_ep_queue
2015-09-21 7:49 ` Peter Chen
@ 2015-09-21 9:18 ` Felipe Tonello
0 siblings, 0 replies; 15+ messages in thread
From: Felipe Tonello @ 2015-09-21 9:18 UTC (permalink / raw)
To: Peter Chen
Cc: USB list, Kernel development list, Greg Kroah-Hartman,
Felipe Balbi, Andrzej Pietrasiewicz
Hi Peter,
On Mon, Sep 21, 2015 at 8:49 AM, Peter Chen <peter.chen@freescale.com> wrote:
> On Mon, Sep 21, 2015 at 09:16:05AM +0100, Felipe Tonello wrote:
>> Hi Chen,
>>
>> On Mon, Sep 21, 2015 at 7:30 AM, Peter Chen <peter.chen@freescale.com> wrote:
>> > On Fri, Sep 18, 2015 at 06:12:41PM +0100, 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
>> >
>> > %s/weather/whether
>> > %s/the/there
>>
>> I fixed it on v3. Did you receive it?
>>
>
> oh, get it, You may add "--subject-prefix="PATCH v3" when you format
> patch, in that case, the reader can see "PATCH v3 2/2" at the subject,
> and know it is the third version.
>
> Your two changes for chipidea are ok, I will queue them.
Thanks. Sorry about that, I thought I did it.
Felipe
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2015-09-21 9:18 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-18 17:12 [PATCH 0/2] improve error handling on chipidea/udc.c and f_midi.c eu
2015-09-18 17:12 ` [PATCH 1/2] usb: chipidea: udc: improve error handling on ep_queue eu
2015-09-18 17:17 ` Felipe Balbi
2015-09-18 17:30 ` Felipe Tonello
2015-09-21 6:29 ` Peter Chen
2015-09-21 9:17 ` Felipe Tonello
2015-09-18 17:12 ` [PATCH 2/2] usb: gadget: f_midi: check for error on usb_ep_queue eu
2015-09-18 17:19 ` Felipe Balbi
2015-09-19 16:09 ` Sergei Shtylyov
2015-09-21 6:30 ` Peter Chen
2015-09-21 8:16 ` Felipe Tonello
2015-09-21 7:49 ` Peter Chen
2015-09-21 9:18 ` Felipe Tonello
2015-09-18 20:56 ` [PATCH 0/2] improve error handling on chipidea/udc.c and f_midi.c Albino B Neto
2015-09-18 20:58 ` Felipe Balbi
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).