Thread (5 messages) 5 messages, 2 authors, 2021-01-21

Re: [PATCH v2] can: mcba_usb: Fix memory leak when cancelling urb

From: Bui Quang Minh <hidden>
Date: 2021-01-11 14:32:14
Also in: lkml

On Mon, Jan 11, 2021 at 01:00:31PM +0100, Oliver Neukum wrote:
Am Montag, den 11.01.2021, 10:49 +0000 schrieb Bui Quang Minh:
quoted
In mcba_usb_read_bulk_callback(), when we don't resubmit or fails to
resubmit the urb, we need to deallocate the transfer buffer that is
allocated in mcba_usb_start().

Reported-by: syzbot+57281c762a3922e14dfe@syzkaller.appspotmail.com
Signed-off-by: Bui Quang Minh <redacted>
---
v1: add memory leak fix when not resubmitting urb
v2: add memory leak fix when failing to resubmit urb

 drivers/net/can/usb/mcba_usb.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/net/can/usb/mcba_usb.c b/drivers/net/can/usb/mcba_usb.c
index df54eb7d4b36..30236e640116 100644
--- a/drivers/net/can/usb/mcba_usb.c
+++ b/drivers/net/can/usb/mcba_usb.c
@@ -584,6 +584,8 @@ static void mcba_usb_read_bulk_callback(struct urb *urb)
 	case -EPIPE:
 	case -EPROTO:
 	case -ESHUTDOWN:
+		usb_free_coherent(urb->dev, urb->transfer_buffer_length,
+				  urb->transfer_buffer, urb->transfer_dma);
 		return;
 
Can you call usb_free_coherent() in what can be hard IRQ context?
You are right, I digged in the code and saw some comments that on some
architectures, usb_free_coherent() cannot be called in hard IRQ context.
I see the usb_free_coherent() is called in write_bulk_callback too. I will
send a patch that uses usb_anchor to keep track of these urbs and cleanup 
the transfer buffer later in disconnect().

Thank you for your review,
Quang Minh.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help