[alsa-devel] [PATCH] ALSA: usx2y: fix a memory leak bug
In usX2Y_In04_init(), a new urb is firstly created through usb_alloc_urb() and saved to 'usX2Y->In04urb'. Then, a buffer is allocated through kmalloc() and saved to 'usX2Y->In04Buf'. After the urb is initialized, a sanity check is performed for the endpoint in the urb by invoking usb_urb_ep_type_check(). If the check fails, the error code EINVAL will be returned. In that case, however, the created urb and the allocated buffer are not freed, leading to memory leaks.
To fix the above issue, free the urb and the buffer if the check fails.
Signed-off-by: Wenwen Wang wang6495@umn.edu --- sound/usb/usx2y/usbusx2y.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/sound/usb/usx2y/usbusx2y.c b/sound/usb/usx2y/usbusx2y.c index da4a5a5..0817018 100644 --- a/sound/usb/usx2y/usbusx2y.c +++ b/sound/usb/usx2y/usbusx2y.c @@ -303,8 +303,11 @@ int usX2Y_In04_init(struct usX2Ydev *usX2Y) usX2Y->In04Buf, 21, i_usX2Y_In04Int, usX2Y, 10); - if (usb_urb_ep_type_check(usX2Y->In04urb)) + if (usb_urb_ep_type_check(usX2Y->In04urb)) { + kfree(usX2Y->In04Buf); + usb_put_urb(usX2Y->In04urb); return -EINVAL; + } return usb_submit_urb(usX2Y->In04urb, GFP_KERNEL); }
On Sun, 28 Apr 2019 08:42:32 +0200, Wenwen Wang wrote:
In usX2Y_In04_init(), a new urb is firstly created through usb_alloc_urb() and saved to 'usX2Y->In04urb'. Then, a buffer is allocated through kmalloc() and saved to 'usX2Y->In04Buf'. After the urb is initialized, a sanity check is performed for the endpoint in the urb by invoking usb_urb_ep_type_check(). If the check fails, the error code EINVAL will be returned. In that case, however, the created urb and the allocated buffer are not freed, leading to memory leaks.
To fix the above issue, free the urb and the buffer if the check fails.
Signed-off-by: Wenwen Wang wang6495@umn.edu
Applied now, thanks.
Takashi
On Sun, 28 Apr 2019 09:18:40 +0200, Takashi Iwai wrote:
On Sun, 28 Apr 2019 08:42:32 +0200, Wenwen Wang wrote:
In usX2Y_In04_init(), a new urb is firstly created through usb_alloc_urb() and saved to 'usX2Y->In04urb'. Then, a buffer is allocated through kmalloc() and saved to 'usX2Y->In04Buf'. After the urb is initialized, a sanity check is performed for the endpoint in the urb by invoking usb_urb_ep_type_check(). If the check fails, the error code EINVAL will be returned. In that case, however, the created urb and the allocated buffer are not freed, leading to memory leaks.
To fix the above issue, free the urb and the buffer if the check fails.
Signed-off-by: Wenwen Wang wang6495@umn.edu
Applied now, thanks.
... and looking at the code again, this patch turned out to be wrong. The in04 urb and transfer buffer are freed at card->private_free callback (snd_usX2Y_card_private_free()) later, so this patch would lead to double-free.
I dropped the patch now.
thanks,
Takashi
On Mon, Apr 29, 2019 at 12:36 AM Takashi Iwai tiwai@suse.de wrote:
On Sun, 28 Apr 2019 09:18:40 +0200, Takashi Iwai wrote:
On Sun, 28 Apr 2019 08:42:32 +0200, Wenwen Wang wrote:
In usX2Y_In04_init(), a new urb is firstly created through usb_alloc_urb() and saved to 'usX2Y->In04urb'. Then, a buffer is allocated through kmalloc() and saved to 'usX2Y->In04Buf'. After the urb is initialized, a sanity check is performed for the endpoint in the urb by invoking usb_urb_ep_type_check(). If the check fails, the error code EINVAL will be returned. In that case, however, the created urb and the allocated buffer are not freed, leading to memory leaks.
To fix the above issue, free the urb and the buffer if the check fails.
Signed-off-by: Wenwen Wang wang6495@umn.edu
Applied now, thanks.
... and looking at the code again, this patch turned out to be wrong. The in04 urb and transfer buffer are freed at card->private_free callback (snd_usX2Y_card_private_free()) later, so this patch would lead to double-free.
Thanks for your comment! Does that mean we should remove usb_free_urb() in the if statement of allocating 'usX2Y->In04Buf', because it may also lead to double free?
Wenwen
On Mon, 29 Apr 2019 07:50:11 +0200, Wenwen Wang wrote:
On Mon, Apr 29, 2019 at 12:36 AM Takashi Iwai tiwai@suse.de wrote:
On Sun, 28 Apr 2019 09:18:40 +0200, Takashi Iwai wrote:
On Sun, 28 Apr 2019 08:42:32 +0200, Wenwen Wang wrote:
In usX2Y_In04_init(), a new urb is firstly created through usb_alloc_urb() and saved to 'usX2Y->In04urb'. Then, a buffer is allocated through kmalloc() and saved to 'usX2Y->In04Buf'. After the urb is initialized, a sanity check is performed for the endpoint in the urb by invoking usb_urb_ep_type_check(). If the check fails, the error code EINVAL will be returned. In that case, however, the created urb and the allocated buffer are not freed, leading to memory leaks.
To fix the above issue, free the urb and the buffer if the check fails.
Signed-off-by: Wenwen Wang wang6495@umn.edu
Applied now, thanks.
... and looking at the code again, this patch turned out to be wrong. The in04 urb and transfer buffer are freed at card->private_free callback (snd_usX2Y_card_private_free()) later, so this patch would lead to double-free.
Thanks for your comment! Does that mean we should remove usb_free_urb() in the if statement of allocating 'usX2Y->In04Buf', because it may also lead to double free?
Yes, that's another superfluous code.
Takashi
On Mon, Apr 29, 2019 at 1:42 AM Takashi Iwai tiwai@suse.de wrote:
On Mon, 29 Apr 2019 07:50:11 +0200, Wenwen Wang wrote:
On Mon, Apr 29, 2019 at 12:36 AM Takashi Iwai tiwai@suse.de wrote:
On Sun, 28 Apr 2019 09:18:40 +0200, Takashi Iwai wrote:
On Sun, 28 Apr 2019 08:42:32 +0200, Wenwen Wang wrote:
In usX2Y_In04_init(), a new urb is firstly created through usb_alloc_urb() and saved to 'usX2Y->In04urb'. Then, a buffer is allocated through kmalloc() and saved to 'usX2Y->In04Buf'. After the urb is initialized, a sanity check is performed for the endpoint in the urb by invoking usb_urb_ep_type_check(). If the check fails, the error code EINVAL will be returned. In that case, however, the created urb and the allocated buffer are not freed, leading to memory leaks.
To fix the above issue, free the urb and the buffer if the check fails.
Signed-off-by: Wenwen Wang wang6495@umn.edu
Applied now, thanks.
... and looking at the code again, this patch turned out to be wrong. The in04 urb and transfer buffer are freed at card->private_free callback (snd_usX2Y_card_private_free()) later, so this patch would lead to double-free.
Thanks for your comment! Does that mean we should remove usb_free_urb() in the if statement of allocating 'usX2Y->In04Buf', because it may also lead to double free?
Yes, that's another superfluous code.
Thanks! I will rework the patch.
Wenwen
participants (2)
-
Takashi Iwai -
Wenwen Wang