usb: Use memdup_user to reuse the code

PATCH Superseded Pathak, Rahul (R.) 2 participants 2 messages Dec 8, 2015 USB

lore

Share & tools

lore

Pathak, Rahul (R.)

author

2015-12-08 12:12 UTC #0 lore raw

PATCH 1/1 usb: Use memdup_user to reuse the code

From: Rahul Pathak <[email protected]>

Fixing coccicheck warning which recommends to use memdup_user instead to reimplement its code, using memdup_user simplifies the code

./drivers/usb/core/devio.c:1398:11-18: WARNING opportunity for memdup_user

Signed-off-by Rahul Pathak <[email protected]>

---
 drivers/usb/core/devio.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

drivers/usb/core/devio.c +3 -5

index 38ae877c..05266f0 100644
@@ -1395,11 +1395,9 @@ static int proc_do_submiturb(struct usb_dev_state *ps, struct usbdevfs_urb *uurb
 		number_of_packets = uurb->number_of_packets;
 		isofrmlen = sizeof(struct usbdevfs_iso_packet_desc) *
 				   number_of_packets;
-		isopkt = kmalloc(isofrmlen, GFP_KERNEL);
-		if (!isopkt)
-			return -ENOMEM;
-		if (copy_from_user(isopkt, iso_frame_desc, isofrmlen)) {
-			ret = -EFAULT;
+		isopkt = memdup_user(iso_frame_desc, isofrmlen);
+		if (IS_ERR(isopkt)) {
+			ret = PTR_ERR(isopkt);
 			goto error;
 		}
 		for (totlen = u = 0; u < number_of_packets; u++) {

Signature · 1 line

1.9.1

Dan Carpenter

2015-12-08 11:52 UTC #1 lore raw

Quoted 7 lines · Rahul (R.)

drivers/usb/core/devio.c +0 -0

index 38ae877c..05266f0 100644
@@ -1395,11 +1395,9 @@ static int proc_do_submiturb(struct usb_dev_state *ps, struct usbdevfs_urb *uurb
 		number_of_packets = uurb->number_of_packets;
 		isofrmlen = sizeof(struct usbdevfs_iso_packet_desc) *

drivers/usb/core/devio.c static int proc_do_submiturb(struct usb_dev_state *ps, struct usbdevfs_urb *uurb · line 1401

Quoted 10 lines

drivers/usb/core/devio.c +3 -5

 				   number_of_packets;
-		isopkt = kmalloc(isofrmlen, GFP_KERNEL);
-		if (!isopkt)
-			return -ENOMEM;
-		if (copy_from_user(isopkt, iso_frame_desc, isofrmlen)) {
-			ret = -EFAULT;
+		isopkt = memdup_user(iso_frame_desc, isofrmlen);
+		if (IS_ERR(isopkt)) {
+			ret = PTR_ERR(isopkt);
 			goto error;

This introduces a one err bug.
https://plus.google.com/106378716002406849458/posts/dnanfhQ4mHQ
We can't call kfree(isopkt) when it is an ERR_PTR.

Set it to NULL:

		isopkt = memdup_user(iso_frame_desc, isofrmlen);
		if (IS_ERR(isopkt)) {
			ret = PTR_ERR(isopkt);
			isopkt = NULL;
			goto error;
		}

regards,
dan carpenter