lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <alpine.LNX.2.00.1101022150120.11481@swampdragon.chaosbits.net>
Date:	Sun, 2 Jan 2011 21:57:24 +0100 (CET)
From:	Jesper Juhl <jj@...osbits.net>
To:	linux-media@...r.kernel.org
cc:	Huang Shijie <shijie8@...il.com>,
	Kang Yong <kangyong@...egent.com>,
	Zhang Xiaobing <xbzhang@...egent.com>,
	Mauro Carvalho Chehab <mchehab@...radead.org>,
	linux-kernel@...r.kernel.org
Subject: [PATCH] media, tlg2300: Fix memory leak in
 alloc_bulk_urbs_generic()

Hi,

While reading 
drivers/media/video/tlg2300/pd-video.c::alloc_bulk_urbs_generic() I 
noticed that

 - We don't free the memory allocated to 'urb' if the call to 
   usb_alloc_coherent() fails.
 - If the 'num' argument to the function is ever <= 0 we'll return an 
   uninitialized variable 'i' to the caller.

The following patch addresses both of the above by a) calling 
usb_free_urb() when usb_alloc_coherent() fails and by explicitly 
initializing 'i' to zero.
I also moved the variables 'mem' and 'urb' inside the for loop. This does 
not actually make any difference, it just seemed more correct to me to let 
variables exist only in the innermost scope they are used.


Signed-off-by: Jesper Juhl <jj@...osbits.net>
---
 pd-video.c |   13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

  compile tested only.

diff --git a/drivers/media/video/tlg2300/pd-video.c b/drivers/media/video/tlg2300/pd-video.c
index a1ffe18..df33a1d 100644
--- a/drivers/media/video/tlg2300/pd-video.c
+++ b/drivers/media/video/tlg2300/pd-video.c
@@ -512,19 +512,20 @@ int alloc_bulk_urbs_generic(struct urb **urb_array, int num,
 			int buf_size, gfp_t gfp_flags,
 			usb_complete_t complete_fn, void *context)
 {
-	struct urb *urb;
-	void *mem;
-	int i;
+	int i = 0;
 
-	for (i = 0; i < num; i++) {
-		urb = usb_alloc_urb(0, gfp_flags);
+	for (; i < num; i++) {
+		void *mem;
+		struct urb *urb = usb_alloc_urb(0, gfp_flags);
 		if (urb == NULL)
 			return i;
 
 		mem = usb_alloc_coherent(udev, buf_size, gfp_flags,
 					 &urb->transfer_dma);
-		if (mem == NULL)
+		if (mem == NULL) {
+			usb_free_urb(urb);
 			return i;
+		}
 
 		usb_fill_bulk_urb(urb, udev, usb_rcvbulkpipe(udev, ep_addr),
 				mem, buf_size, complete_fn, context);



-- 
Jesper Juhl <jj@...osbits.net>            http://www.chaosbits.net/
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ