diff mbox

usbnet: make sure no NULL pointer is passed through

Message ID 20170405121439.22523-1-oneukum@suse.com
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Oliver Neukum April 5, 2017, 12:14 p.m. UTC 
Coverity reports:

** CID 751368:  Null pointer dereferences  (FORWARD_NULL)
/drivers/net/usb/usbnet.c: 1925 in __usbnet_read_cmd()

Comments

David Miller April 6, 2017, 8:19 p.m. UTC | #1 
From: Oliver Neukum <oneukum@suse.com>
Date: Wed,  5 Apr 2017 14:14:39 +0200

> Coverity reports:
 ...
> It is valid to offer commands without a buffer, but then you need a size
> of zero. This should actually be checked.
> 
> Signed-off-by: Oliver Neukum <oneukum@suse.com>

Applied, thanks Oliver.

I had to apply this by hand via my inbox rather than using patchwork
because those coverity report delimiters cause patchwork to chop off
most of your commit message.

Just FYI...
Oliver Neukum April 10, 2017, 9:01 a.m. UTC | #2 
Am Donnerstag, den 06.04.2017, 13:19 -0700 schrieb David Miller:
> From: Oliver Neukum <oneukum@suse.com>
> Date: Wed,  5 Apr 2017 14:14:39 +0200
> 
> > Coverity reports:
>  ...
> > It is valid to offer commands without a buffer, but then you need a size
> > of zero. This should actually be checked.
> > 
> > Signed-off-by: Oliver Neukum <oneukum@suse.com>
> 
> Applied, thanks Oliver.
> 
> I had to apply this by hand via my inbox rather than using patchwork
> because those coverity report delimiters cause patchwork to chop off
> most of your commit message.
> 

Hi,

thanks. That is not good. It seems to me that a Coverity report
should be in the change log. Do you have suggestions how to do
this in a friendly manner?

	Regards
		Oliver
diff mbox

Patch

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 3de65ea..4532448 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1929,7 +1929,7 @@  static int __usbnet_read_cmd(struct usbnet *dev, u8 cmd, u8 reqtype,
 		   " value=0x%04x index=0x%04x size=%d\n",
 		   cmd, reqtype, value, index, size);
 
-	if (data) {
+	if (size) {
 		buf = kmalloc(size, GFP_KERNEL);
 		if (!buf)
 			goto out;
@@ -1938,8 +1938,13 @@  static int __usbnet_read_cmd(struct usbnet *dev, u8 cmd, u8 reqtype,
 	err = usb_control_msg(dev->udev, usb_rcvctrlpipe(dev->udev, 0),
 			      cmd, reqtype, value, index, buf, size,
 			      USB_CTRL_GET_TIMEOUT);
-	if (err > 0 && err <= size)
-		memcpy(data, buf, err);
+	if (err > 0 && err <= size) {
+        if (data)
+            memcpy(data, buf, err);
+        else
+            netdev_dbg(dev->net,
+                "Huh? Data requested but thrown away.\n");
+    }
 	kfree(buf);
 out:
 	return err;
@@ -1960,7 +1965,13 @@  static int __usbnet_write_cmd(struct usbnet *dev, u8 cmd, u8 reqtype,
 		buf = kmemdup(data, size, GFP_KERNEL);
 		if (!buf)
 			goto out;
-	}
+	} else {
+        if (size) {
+            WARN_ON_ONCE(1);
+            err = -EINVAL;
+            goto out;
+        }
+    }
 
 	err = usb_control_msg(dev->udev, usb_sndctrlpipe(dev->udev, 0),
 			      cmd, reqtype, value, index, buf, size,