Skip to contentSkip to conversation

usb: iowarrior: replace kmalloc with kmalloc_array

PATCH Superseded Gustavo A. R. Silva 2 participants 4 messages Aug 23, 2018 USB

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Gustavo A. R. Silva

author
#0 lore raw

PATCH 1/1 usb: iowarrior: replace kmalloc with kmalloc_array

A common flaw in the kernel is integer overflow during memory allocation size calculations. In an effort to reduce the frequency of these bugs, kmalloc_array was implemented, which allocates memory for an array, while at the same time detects integer overflow.

This patch replaces cases of:

kmalloc(a * b, gfp)

with:
kmalloc_array(a * b, gfp)

Signed-off-by Gustavo A. R. Silva <[email protected]>
---
 drivers/usb/misc/iowarrior.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/usb/misc/iowarrior.c +1 -1
index c2991b8..7a22400 100644
@@ -808,7 +808,7 @@ static int iowarrior_probe(struct usb_interface *interface,
 			 dev->int_in_endpoint->bInterval);
 	/* create an internal buffer for interrupt data from the device */
 	dev->read_queue =
-	    kmalloc(((dev->report_size + 1) * MAX_INTERRUPT_BUFFER),
+	    kmalloc_array(dev->report_size + 1, MAX_INTERRUPT_BUFFER,
 		    GFP_KERNEL);
 	if (!dev->read_queue)
 		goto error;
author
#1 lore raw

Hi,

Please, drop this.

I'll align GFP_KERNEL and send v2.

Thanks
---
Gustavo

Quoted 31 lines · Gustavo A. R. Silva

A common flaw in the kernel is integer overflow during memory allocation size calculations. In an effort to reduce the frequency of these bugs, kmalloc_array was implemented, which allocates memory for an array, while at the same time detects integer overflow.

This patch replaces cases of:

kmalloc(a * b, gfp)

with:
kmalloc_array(a * b, gfp)

Signed-off-by Gustavo A. R. Silva <[email protected]>
---
 drivers/usb/misc/iowarrior.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/usb/misc/iowarrior.c +1 -1
index c2991b8..7a22400 100644
@@ -808,7 +808,7 @@ static int iowarrior_probe(struct usb_interface *interface,
 			 dev->int_in_endpoint->bInterval);
 	/* create an internal buffer for interrupt data from the device */
 	dev->read_queue =
-	    kmalloc(((dev->report_size + 1) * MAX_INTERRUPT_BUFFER),
+	    kmalloc_array(dev->report_size + 1, MAX_INTERRUPT_BUFFER,
 		    GFP_KERNEL);
 	if (!dev->read_queue)
 		goto error;
Reviewed-by on v1 1/1
#2 lore raw
In reply to Gustavo A. R. Silva
Quoted 11 lines · Gustavo A. R. Silva

A common flaw in the kernel is integer overflow during memory allocation size calculations. In an effort to reduce the frequency of these bugs, kmalloc_array was implemented, which allocates memory for an array, while at the same time detects integer overflow.

This patch replaces cases of:

kmalloc(a * b, gfp)

with:
kmalloc_array(a * b, gfp)

nit: this should be "kmalloc_array(a, b, gfp)" (This was a typo from my treewide commit message... :P)

Quoted 10 lines
Signed-off-by Gustavo A. R. Silva <[email protected]>
---
 drivers/usb/misc/iowarrior.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/usb/misc/iowarrior.c +0 -0
index c2991b8..7a22400 100644

drivers/usb/misc/iowarrior.c static int iowarrior_probe(struct usb_interface *interface, · line 812

Quoted 6 lines
drivers/usb/misc/iowarrior.c +1 -1
@@ -808,7 +808,7 @@ static int iowarrior_probe(struct usb_interface *interface,
                         dev->int_in_endpoint->bInterval);
        /* create an internal buffer for interrupt data from the device */
        dev->read_queue =
-           kmalloc(((dev->report_size + 1) * MAX_INTERRUPT_BUFFER),
+           kmalloc_array(dev->report_size + 1, MAX_INTERRUPT_BUFFER,
                    GFP_KERNEL);

For this patch with the commit log updated:

Reviewed-by Kees Cook <[email protected]>

Thanks!

-Kees

Signature · 2 lines
Kees Cook
Pixel Security
author
#3 lore raw
Quoted 9 lines · Kees Cook
Quoted 7 lines · Gustavo A. R. Silva

A common flaw in the kernel is integer overflow during memory allocation size calculations. In an effort to reduce the frequency of these bugs, kmalloc_array was implemented, which allocates memory for an array, while at the same time detects integer overflow.

This patch replaces cases of:

Quoted 8 lines
Quoted 4 lines

kmalloc(a * b, gfp)

with:
kmalloc_array(a * b, gfp)

nit: this should be "kmalloc_array(a, b, gfp)" (This was a typo from my treewide commit message... :P)

Oops... OK. V3 is on the way. :P

Quoted 19 lines
Quoted 10 lines
Signed-off-by Gustavo A. R. Silva <[email protected]>
---
 drivers/usb/misc/iowarrior.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/usb/misc/iowarrior.c +0 -0
index c2991b8..7a22400 100644

drivers/usb/misc/iowarrior.c static int iowarrior_probe(struct usb_interface *interface, · line 812

Quoted 6 lines
drivers/usb/misc/iowarrior.c +1 -1
@@ -808,7 +808,7 @@ static int iowarrior_probe(struct usb_interface *interface,
                         dev->int_in_endpoint->bInterval);
        /* create an internal buffer for interrupt data from the device */
        dev->read_queue =
-           kmalloc(((dev->report_size + 1) * MAX_INTERRUPT_BUFFER),
+           kmalloc_array(dev->report_size + 1, MAX_INTERRUPT_BUFFER,
                    GFP_KERNEL);

For this patch with the commit log updated:

Quoted 2 lines
Reviewed-by Kees Cook <[email protected]>

Thanks!
---
Gustavo