Re: [PATCH 3/3] usb: gadget: f_midi: free request when usb_ep_queue fails
On Fri, Sep 25, 2015 at 10:02 AM, Peter Chen wrote:
> On Fri, Sep 25, 2015 at 09:27:49AM +0100, Felipe Tonello wrote:
>> On Thu, Sep 24, 2015 at 2:20 AM, Peter Chen wrote:
>> > On Wed, Sep 23, 2015 at 12:40:46PM +0100, Felipe Tonello wrote:
>> >> Hi Peter,
>> >>
>> >> On Wed, Sep 23, 2015 at 8:09 AM, Peter Chen
>> >> wrote:
>> >> > On Tue, Sep 22, 2015 at 07:59:10PM +0100, Felipe F. Tonello wrote:
>> >> >> This fix a memory leak that will occur in this case.
>> >> >>
>> >> >> Signed-off-by: Felipe F. Tonello
>> >> >> ---
>> >> >> drivers/usb/gadget/function/f_midi.c | 4 +++-
>> >> >> 1 file changed, 3 insertions(+), 1 deletion(-)
>> >> >>
>> >> >> diff --git a/drivers/usb/gadget/function/f_midi.c
>> >> >> b/drivers/usb/gadget/function/f_midi.c
>> >> >> index e92aff5..e6a114b 100644
>> >> >> --- a/drivers/usb/gadget/function/f_midi.c
>> >> >> +++ b/drivers/usb/gadget/function/f_midi.c
>> >> >> @@ -550,9 +550,11 @@ static void f_midi_transmit(struct f_midi *midi,
>> >> >> struct usb_request *req)
>> >> >> int err;
>> >> >>
>> >> >> err = usb_ep_queue(ep, req, GFP_ATOMIC);
>> >> >> - if (err < 0)
>> >> >> + if (err < 0) {
>> >> >> ERROR(midi, "%s queue req: %d\n",
>> >> >> midi->in_ep->name, err);
>> >> >> + free_ep_req(ep, req);
>> >> >> + }
>> >> >> } else {
>> >> >> free_ep_req(ep, req);
>> >> >> }
>> >> >> --
>> >> >> 2.1.4
>> >> >>
>> >> >
>> >> > I may know your problem, current midi library, alsa and this driver
>> >> > allow device sends as much data as possible, but without block the
>> >> > sending until host reads data, it only allocates the request buffer
>> >> > (using midi_alloc_ep_req), but without free, so after you send
>> >> > enough data, it is out of memory.
>> >>
>> >> Yes. Also there is the case where the usb cable is not conected, thus
>> >> failing to hardware enqueue the request, causing a memory leak on this
>> >> request.
>> >>
>> >
>> > If the usb cable is not connected, the related endpoints should be
>> > not enabled. Would you really observe enqueue the request without
>> > cable connected?
>>
>> The usb_ep_queue() returns an error if it is not connected, causing
>> the request never to be freed and never to be queued. Thus a memory
>> leak happens.
>>
>
> If it is not connected, the ep is not enabled, why we will call
> usb_ep_queue? If it really does, there must be something wrong.
Ok. I can do another patch to check for that. But this patch still
applies, because if for some reason the usb_ep_queue() returns an
error, it will not leak memory.
Felipe
--
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 3/3] usb: gadget: f_midi: free request when usb_ep_queue fails
On Fri, Sep 25, 2015 at 09:27:49AM +0100, Felipe Tonello wrote:
> On Thu, Sep 24, 2015 at 2:20 AM, Peter Chen wrote:
> > On Wed, Sep 23, 2015 at 12:40:46PM +0100, Felipe Tonello wrote:
> >> Hi Peter,
> >>
> >> On Wed, Sep 23, 2015 at 8:09 AM, Peter Chen
> >> wrote:
> >> > On Tue, Sep 22, 2015 at 07:59:10PM +0100, Felipe F. Tonello wrote:
> >> >> This fix a memory leak that will occur in this case.
> >> >>
> >> >> Signed-off-by: Felipe F. Tonello
> >> >> ---
> >> >> drivers/usb/gadget/function/f_midi.c | 4 +++-
> >> >> 1 file changed, 3 insertions(+), 1 deletion(-)
> >> >>
> >> >> diff --git a/drivers/usb/gadget/function/f_midi.c
> >> >> b/drivers/usb/gadget/function/f_midi.c
> >> >> index e92aff5..e6a114b 100644
> >> >> --- a/drivers/usb/gadget/function/f_midi.c
> >> >> +++ b/drivers/usb/gadget/function/f_midi.c
> >> >> @@ -550,9 +550,11 @@ static void f_midi_transmit(struct f_midi *midi,
> >> >> struct usb_request *req)
> >> >> int err;
> >> >>
> >> >> err = usb_ep_queue(ep, req, GFP_ATOMIC);
> >> >> - if (err < 0)
> >> >> + if (err < 0) {
> >> >> ERROR(midi, "%s queue req: %d\n",
> >> >> midi->in_ep->name, err);
> >> >> + free_ep_req(ep, req);
> >> >> + }
> >> >> } else {
> >> >> free_ep_req(ep, req);
> >> >> }
> >> >> --
> >> >> 2.1.4
> >> >>
> >> >
> >> > I may know your problem, current midi library, alsa and this driver
> >> > allow device sends as much data as possible, but without block the
> >> > sending until host reads data, it only allocates the request buffer
> >> > (using midi_alloc_ep_req), but without free, so after you send
> >> > enough data, it is out of memory.
> >>
> >> Yes. Also there is the case where the usb cable is not conected, thus
> >> failing to hardware enqueue the request, causing a memory leak on this
> >> request.
> >>
> >
> > If the usb cable is not connected, the related endpoints should be
> > not enabled. Would you really observe enqueue the request without
> > cable connected?
>
> The usb_ep_queue() returns an error if it is not connected, causing
> the request never to be freed and never to be queued. Thus a memory
> leak happens.
>
If it is not connected, the ep is not enabled, why we will call
usb_ep_queue? If it really does, there must be something wrong.
--
Best Regards,
Peter Chen
--
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 3/3] usb: gadget: f_midi: free request when usb_ep_queue fails
On Thu, Sep 24, 2015 at 2:20 AM, Peter Chen wrote:
> On Wed, Sep 23, 2015 at 12:40:46PM +0100, Felipe Tonello wrote:
>> Hi Peter,
>>
>> On Wed, Sep 23, 2015 at 8:09 AM, Peter Chen wrote:
>> > On Tue, Sep 22, 2015 at 07:59:10PM +0100, Felipe F. Tonello wrote:
>> >> This fix a memory leak that will occur in this case.
>> >>
>> >> Signed-off-by: Felipe F. Tonello
>> >> ---
>> >> drivers/usb/gadget/function/f_midi.c | 4 +++-
>> >> 1 file changed, 3 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/drivers/usb/gadget/function/f_midi.c
>> >> b/drivers/usb/gadget/function/f_midi.c
>> >> index e92aff5..e6a114b 100644
>> >> --- a/drivers/usb/gadget/function/f_midi.c
>> >> +++ b/drivers/usb/gadget/function/f_midi.c
>> >> @@ -550,9 +550,11 @@ static void f_midi_transmit(struct f_midi *midi,
>> >> struct usb_request *req)
>> >> int err;
>> >>
>> >> err = usb_ep_queue(ep, req, GFP_ATOMIC);
>> >> - if (err < 0)
>> >> + if (err < 0) {
>> >> ERROR(midi, "%s queue req: %d\n",
>> >> midi->in_ep->name, err);
>> >> + free_ep_req(ep, req);
>> >> + }
>> >> } else {
>> >> free_ep_req(ep, req);
>> >> }
>> >> --
>> >> 2.1.4
>> >>
>> >
>> > I may know your problem, current midi library, alsa and this driver
>> > allow device sends as much data as possible, but without block the
>> > sending until host reads data, it only allocates the request buffer
>> > (using midi_alloc_ep_req), but without free, so after you send
>> > enough data, it is out of memory.
>>
>> Yes. Also there is the case where the usb cable is not conected, thus
>> failing to hardware enqueue the request, causing a memory leak on this
>> request.
>>
>
> If the usb cable is not connected, the related endpoints should be
> not enabled. Would you really observe enqueue the request without
> cable connected?
The usb_ep_queue() returns an error if it is not connected, causing
the request never to be freed and never to be queued. Thus a memory
leak happens.
Felipe
--
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 3/3] usb: gadget: f_midi: free request when usb_ep_queue fails
On Wed, Sep 23, 2015 at 12:40:46PM +0100, Felipe Tonello wrote:
> Hi Peter,
>
> On Wed, Sep 23, 2015 at 8:09 AM, Peter Chen wrote:
> > On Tue, Sep 22, 2015 at 07:59:10PM +0100, Felipe F. Tonello wrote:
> >> This fix a memory leak that will occur in this case.
> >>
> >> Signed-off-by: Felipe F. Tonello
> >> ---
> >> drivers/usb/gadget/function/f_midi.c | 4 +++-
> >> 1 file changed, 3 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/usb/gadget/function/f_midi.c
> >> b/drivers/usb/gadget/function/f_midi.c
> >> index e92aff5..e6a114b 100644
> >> --- a/drivers/usb/gadget/function/f_midi.c
> >> +++ b/drivers/usb/gadget/function/f_midi.c
> >> @@ -550,9 +550,11 @@ static void f_midi_transmit(struct f_midi *midi,
> >> struct usb_request *req)
> >> int err;
> >>
> >> err = usb_ep_queue(ep, req, GFP_ATOMIC);
> >> - if (err < 0)
> >> + if (err < 0) {
> >> ERROR(midi, "%s queue req: %d\n",
> >> midi->in_ep->name, err);
> >> + free_ep_req(ep, req);
> >> + }
> >> } else {
> >> free_ep_req(ep, req);
> >> }
> >> --
> >> 2.1.4
> >>
> >
> > I may know your problem, current midi library, alsa and this driver
> > allow device sends as much data as possible, but without block the
> > sending until host reads data, it only allocates the request buffer
> > (using midi_alloc_ep_req), but without free, so after you send
> > enough data, it is out of memory.
>
> Yes. Also there is the case where the usb cable is not conected, thus
> failing to hardware enqueue the request, causing a memory leak on this
> request.
>
If the usb cable is not connected, the related endpoints should be
not enabled. Would you really observe enqueue the request without
cable connected?
--
Best Regards,
Peter Chen
--
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 3/3] usb: gadget: f_midi: free request when usb_ep_queue fails
Hi Peter,
On Wed, Sep 23, 2015 at 8:09 AM, Peter Chen wrote:
> On Tue, Sep 22, 2015 at 07:59:10PM +0100, Felipe F. Tonello wrote:
>> This fix a memory leak that will occur in this case.
>>
>> Signed-off-by: Felipe F. Tonello
>> ---
>> drivers/usb/gadget/function/f_midi.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/gadget/function/f_midi.c
>> b/drivers/usb/gadget/function/f_midi.c
>> index e92aff5..e6a114b 100644
>> --- a/drivers/usb/gadget/function/f_midi.c
>> +++ b/drivers/usb/gadget/function/f_midi.c
>> @@ -550,9 +550,11 @@ static void f_midi_transmit(struct f_midi *midi, struct
>> usb_request *req)
>> int err;
>>
>> err = usb_ep_queue(ep, req, GFP_ATOMIC);
>> - if (err < 0)
>> + if (err < 0) {
>> ERROR(midi, "%s queue req: %d\n",
>> midi->in_ep->name, err);
>> + free_ep_req(ep, req);
>> + }
>> } else {
>> free_ep_req(ep, req);
>> }
>> --
>> 2.1.4
>>
>
> I may know your problem, current midi library, alsa and this driver
> allow device sends as much data as possible, but without block the
> sending until host reads data, it only allocates the request buffer
> (using midi_alloc_ep_req), but without free, so after you send
> enough data, it is out of memory.
Yes. Also there is the case where the usb cable is not conected, thus
failing to hardware enqueue the request, causing a memory leak on this
request.
Felipe
--
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 3/3] usb: gadget: f_midi: free request when usb_ep_queue fails
On Tue, Sep 22, 2015 at 07:59:10PM +0100, Felipe F. Tonello wrote:
> This fix a memory leak that will occur in this case.
>
> Signed-off-by: Felipe F. Tonello
> ---
> drivers/usb/gadget/function/f_midi.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/gadget/function/f_midi.c
> b/drivers/usb/gadget/function/f_midi.c
> index e92aff5..e6a114b 100644
> --- a/drivers/usb/gadget/function/f_midi.c
> +++ b/drivers/usb/gadget/function/f_midi.c
> @@ -550,9 +550,11 @@ static void f_midi_transmit(struct f_midi *midi, struct
> usb_request *req)
> int err;
>
> err = usb_ep_queue(ep, req, GFP_ATOMIC);
> - if (err < 0)
> + if (err < 0) {
> ERROR(midi, "%s queue req: %d\n",
> midi->in_ep->name, err);
> + free_ep_req(ep, req);
> + }
> } else {
> free_ep_req(ep, req);
> }
> --
> 2.1.4
>
I may know your problem, current midi library, alsa and this driver
allow device sends as much data as possible, but without block the
sending until host reads data, it only allocates the request buffer
(using midi_alloc_ep_req), but without free, so after you send
enough data, it is out of memory.
--
Best Regards,
Peter Chen
--
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 3/3] usb: gadget: f_midi: free request when usb_ep_queue fails
On Tue, Sep 22, 2015 at 07:59:10PM +0100, Felipe F. Tonello wrote:
> This fix a memory leak that will occur in this case.
>
> Signed-off-by: Felipe F. Tonello
> ---
> drivers/usb/gadget/function/f_midi.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/gadget/function/f_midi.c
> b/drivers/usb/gadget/function/f_midi.c
> index e92aff5..e6a114b 100644
> --- a/drivers/usb/gadget/function/f_midi.c
> +++ b/drivers/usb/gadget/function/f_midi.c
> @@ -550,9 +550,11 @@ static void f_midi_transmit(struct f_midi *midi, struct
> usb_request *req)
> int err;
>
> err = usb_ep_queue(ep, req, GFP_ATOMIC);
> - if (err < 0)
> + if (err < 0) {
> ERROR(midi, "%s queue req: %d\n",
> midi->in_ep->name, err);
> + free_ep_req(ep, req);
sure this request won't be needed on the next time the tasklet is queued ?
Although, this looks correct, let's try to make sure really is correct.
--
balbi
signature.asc
Description: PGP signature
[PATCH 3/3] usb: gadget: f_midi: free request when usb_ep_queue fails
This fix a memory leak that will occur in this case.
Signed-off-by: Felipe F. Tonello
---
drivers/usb/gadget/function/f_midi.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/function/f_midi.c
b/drivers/usb/gadget/function/f_midi.c
index e92aff5..e6a114b 100644
--- a/drivers/usb/gadget/function/f_midi.c
+++ b/drivers/usb/gadget/function/f_midi.c
@@ -550,9 +550,11 @@ static void f_midi_transmit(struct f_midi *midi, struct
usb_request *req)
int err;
err = usb_ep_queue(ep, req, GFP_ATOMIC);
- if (err < 0)
+ if (err < 0) {
ERROR(midi, "%s queue req: %d\n",
midi->in_ep->name, err);
+ free_ep_req(ep, req);
+ }
} else {
free_ep_req(ep, req);
}
--
2.1.4
--
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/
