Skip to contentSkip to conversation

usb: Use memdup_user to reuse the code

PATCH Superseded Pathak, Rahul (R.) 4 participants 6 messages Dec 8, 2015 – Dec 12, 2015 USB

lore
Share & tools
lore
#0 lore raw

PATCH v2 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

Changes after v1: setting isopkt=NULL for proper kfree() call

---
 drivers/usb/core/devio.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)
drivers/usb/core/devio.c +4 -5
index 38ae877c..844407c 100644
@@ -1395,11 +1395,10 @@ 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);
+			isopkt = NULL;
 			goto error;
 		}
 		for (totlen = u = 0; u < number_of_packets; u++) {
Signature · 1 line
1.9.1
Acked-by on v2 1/1
#1 lore raw
Quoted 8 lines · Rahul (R.)

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

Changes after v1: setting isopkt=NULL for proper kfree() call

This line belongs below the "---" tear line, so that it doesn't show up in the final commit changelog. People reading the final commit won't care about earlier versions of the patch.

Quoted 15 lines
---
 drivers/usb/core/devio.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)
drivers/usb/core/devio.c +0 -2
index 38ae877c..844407c 100644
@@ -1395,11 +1395,10 @@ 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)

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

Quoted 10 lines
drivers/usb/core/devio.c +4 -3
-			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);
+			isopkt = NULL;
 			goto error;
 		}
 		for (totlen = u = 0; u < number_of_packets; u++) {

Aside from that,

Alan Stern

author Acked-by Alan Stern
#2 lore raw

Hello Alan,

Should I resend this patch version with the tear line correction?

Regards
Rahul Pathak

________________________________________
From: Alan Stern <[email protected]>
Sent: Tuesday, December 8, 2015 8:53 PM
To: Pathak, Rahul (R.)
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: Re: [PATCH v2] usb: Use memdup_user to reuse the code

Quoted 8 lines · Rahul (R.)

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

Changes after v1: setting isopkt=NULL for proper kfree() call

This line belongs below the "---" tear line, so that it doesn't show up in the final commit changelog. People reading the final commit won't care about earlier versions of the patch.

Quoted 15 lines
---
 drivers/usb/core/devio.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)
drivers/usb/core/devio.c +0 -2
index 38ae877c..844407c 100644
@@ -1395,11 +1395,10 @@ 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)

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

Quoted 10 lines
drivers/usb/core/devio.c +4 -3
-                     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);
+                     isopkt = NULL;
                      goto error;
              }
              for (totlen = u = 0; u < number_of_packets; u++) {

Aside from that,

Alan Stern

#3 lore raw
Quoted 3 lines · Rahul (R.)

Hello Alan,

Should I resend this patch version with the tear line correction?

Why wouldn't you?

#5 lore raw

Hello Greg,

I have already resent the patch yesterday in new mail.

-- Sending this mail again because previous mail was blocked as spam.

Regards
Rahul

Quoted 10 lines · On Fri, Dec 11, 2015 at 10:01 PM, [email protected]…
Quoted 3 lines · Rahul (R.)

Hello Alan,

Should I resend this patch version with the tear line correction?

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