Skip to contentSkip to conversation

usb: use kcalloc instead of kzalloc

PATCH In review Arjun Sreedharan 3 participants 3 messages Aug 19, 2014 USB

lore
Share & tools
lore

Arjun Sreedharan

author
#0 lore raw

PATCH 1/1 usb: use kcalloc instead of kzalloc

kcalloc has protection from integer overflows
which could arise from the multiplication of
number of elements and size.

Signed-off-by Arjun Sreedharan <[email protected]>
---
 drivers/usb/core/config.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)
drivers/usb/core/config.c +4 -7
index b2a540b..abde67d 100644
@@ -318,7 +318,7 @@ static int usb_parse_interface(struct device *ddev, int cfgno,
 	struct usb_interface_cache *intfc;
 	struct usb_host_interface *alt;
 	int i, n;
-	int len, retval;
+	int retval;
 	int num_ep, num_ep_orig;
 
 	d = (struct usb_interface_descriptor *) buffer;

@@ -380,8 +380,7 @@ static int usb_parse_interface(struct device *ddev, int cfgno,
 
 	if (num_ep > 0) {
 		/* Can't allocate 0 bytes */
-		len = sizeof(struct usb_host_endpoint) * num_ep;
-		alt->endpoint = kzalloc(len, GFP_KERNEL);
+		alt->endpoint = kcalloc(num_ep, sizeof(struct usb_host_endpoint), GFP_KERNEL);
 		if (!alt->endpoint)
 			return -ENOMEM;
 	}

@@ -683,13 +682,11 @@ int usb_get_configuration(struct usb_device *dev)
 		return -EINVAL;
 	}
 
-	length = ncfg * sizeof(struct usb_host_config);
-	dev->config = kzalloc(length, GFP_KERNEL);
+	dev->config = kcalloc(ncfg, sizeof(struct usb_host_config), GFP_KERNEL);
 	if (!dev->config)
 		goto err2;
 
-	length = ncfg * sizeof(char *);
-	dev->rawdescriptors = kzalloc(length, GFP_KERNEL);
+	dev->rawdescriptors = kcalloc(ncfg, sizeof(char *), GFP_KERNEL);
 	if (!dev->rawdescriptors)
 		goto err2;
Signature · 1 line
1.8.1.msysgit.1
#1 lore raw
Quoted 3 lines · Arjun Sreedharan

kcalloc has protection from integer overflows
which could arise from the multiplication of
number of elements and size.

You are implying that such an overflow could arise in this code. This is false.

drivers/usb/core/config.c static int usb_parse_interface(struct device *ddev, int cfgno, · line 383

Quoted 7 lines
drivers/usb/core/config.c +1 -2
@@ -380,8 +380,7 @@ static int usb_parse_interface(struct device *ddev, int cfgno,
 
 	if (num_ep > 0) {
 		/* Can't allocate 0 bytes */
-		len = sizeof(struct usb_host_endpoint) * num_ep;
-		alt->endpoint = kzalloc(len, GFP_KERNEL);
+		alt->endpoint = kcalloc(num_ep, sizeof(struct usb_host_endpoint), GFP_KERNEL);

bNumEndpoints is an unsigned 8-bit integer.

And even if you did not notice this, you should have noticed the if() directly before this one.

drivers/usb/core/config.c int usb_get_configuration(struct usb_device *dev) · line 689

Quoted 13 lines
drivers/usb/core/config.c +2 -4
@@ -683,13 +682,11 @@ int usb_get_configuration(struct usb_device *dev)
 		return -EINVAL;
 	}
 
-	length = ncfg * sizeof(struct usb_host_config);
-	dev->config = kzalloc(length, GFP_KERNEL);
+	dev->config = kcalloc(ncfg, sizeof(struct usb_host_config), GFP_KERNEL);
 	if (!dev->config)
 		goto err2;
 
-	length = ncfg * sizeof(char *);
-	dev->rawdescriptors = kzalloc(length, GFP_KERNEL);
+	dev->rawdescriptors = kcalloc(ncfg, sizeof(char *), GFP_KERNEL);

Same oversights here.

Regards,
Clemens