Skip to contentSkip to conversation

USB: usbtest: prevent a divide by zero bug

PATCH In review Dan Carpenter 3 participants 4 messages Nov 17, 2012 – Nov 20, 2012

Subsystems (2)
USB Kernel janitors
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 USB: usbtest: prevent a divide by zero bug

If param->length is zero, then this could lead to a divide by zero bug later in the function when we do: size %= max;

Signed-off-by Dan Carpenter <[email protected]>
drivers/usb/misc/usbtest.c +3 -0
index f10bd97..7667b12 100644
@@ -423,6 +423,9 @@ alloc_sglist(int nents, int max, int vary)
 	unsigned		i;
 	unsigned		size = max;
 
+	if (max = 0)
+		return NULL;
+
 	sg = kmalloc_array(nents, sizeof *sg, GFP_KERNEL);
 	if (!sg)
 		return NULL;
#1 lore raw

Am 17.11.2012 16:06, schrieb Dan Carpenter:

Quoted 10 lines

If param->length is zero, then this could lead to a divide by zero bug later in the function when we do: size %= max;

Signed-off-by Dan Carpenter <[email protected]>
drivers/usb/misc/usbtest.c +0 -0
index f10bd97..7667b12 100644

drivers/usb/misc/usbtest.c alloc_sglist(int nents, int max, int vary) · line 428

Quoted 6 lines
drivers/usb/misc/usbtest.c +3 -0
@@ -423,6 +423,9 @@ alloc_sglist(int nents, int max, int vary)
 	unsigned		i;
 	unsigned		size = max;
 
+	if (max = 0)
+		return NULL;
+

maybe you should be more defensive and check from (max <= 0)

re,
wh

Quoted 9 lines
drivers/usb/misc/usbtest.c +0 -1
 	sg = kmalloc_array(nents, sizeof *sg, GFP_KERNEL);
 	if (!sg)
 		return NULL;
--

To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#2 lore raw
In reply to Dan Carpenter
Quoted 16 lines · walter harms

Am 17.11.2012 16:06, schrieb Dan Carpenter:

Quoted 13 lines

If param->length is zero, then this could lead to a divide by zero bug later in the function when we do: size %= max;

Signed-off-by Dan Carpenter <[email protected]>
drivers/usb/misc/usbtest.c +0 -0
index f10bd97..7667b12 100644
@@ -423,6 +423,9 @@ alloc_sglist(int nents, int max, int vary)
 	unsigned		i;
 	unsigned		size = max;
Quoted 6 lines

drivers/usb/misc/usbtest.c alloc_sglist(int nents, int max, int vary) · line 428

Quoted 3 lines
drivers/usb/misc/usbtest.c +3 -0
+	if (max = 0)
+		return NULL;
+

maybe you should be more defensive and check from (max <= 0)

Nah... Testing for = 0 is ok.

regards,
dan carpenter

#3 lore raw
In reply to Dan Carpenter
Quoted 17 lines · Dan Carpenter
Quoted 16 lines · walter harms

Am 17.11.2012 16:06, schrieb Dan Carpenter:

Quoted 13 lines

If param->length is zero, then this could lead to a divide by zero bug later in the function when we do: size %= max;

Signed-off-by Dan Carpenter <[email protected]>
drivers/usb/misc/usbtest.c +0 -0
index f10bd97..7667b12 100644
@@ -423,6 +423,9 @@ alloc_sglist(int nents, int max, int vary)
 	unsigned		i;
 	unsigned		size = max;
Quoted 8 lines
Quoted 6 lines

drivers/usb/misc/usbtest.c alloc_sglist(int nents, int max, int vary) · line 428

Quoted 3 lines
drivers/usb/misc/usbtest.c +3 -0
+	if (max = 0)
+		return NULL;
+

maybe you should be more defensive and check from (max <= 0)

Nah... Testing for = 0 is ok.

The parameter comes from user. -1 is hardly possible because the parameter is defined as unsigned and only alloc_sglist() parameters are signed. Could you please convert the int to unsigned so it matches the original source of the parameter?

Passing -1 from user space leads to

|WARNING: at /home/bigeasy/work/new/TI/linux/mm/page_alloc.c:2403 |__alloc_pages_nodemask+0x24d/0x6d0()

aka ENOMEM so it is not that big of deal.

0 on the hand is more critical.

Quoted 2 lines

regards,
dan carpenter

Sebastian