can: Fix possible NULL pointer dereference in ems_usb.c

PATCH Applied Hans J. Koch 3 participants 6 messages Apr 20, 2010 – Apr 21, 2010 3 messages not in the archive

Subsystems (3)

Networking Network drivers CAN network drivers

lore

Share & tools

lore

Hans J. Koch

author

2010-04-20 13:55 UTC #0 lore raw

PATCH 1/1 can: Fix possible NULL pointer dereference in ems_usb.c

In ems_usb_probe(), a pointer is dereferenced after making sure it is NULL...

This patch replaces dev_err() with printk() to avoid this.

Signed-off-by "Hans J. Koch" <[email protected]>

---
 drivers/net/can/usb/ems_usb.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/net/can/usb/ems_usb.c +1 -1

Index: linux-2.6.34-rc/drivers/net/can/usb/ems_usb.c
===================================================================
@@ -1006,7 +1006,7 @@
 
 	netdev = alloc_candev(sizeof(struct ems_usb), MAX_TX_URBS);
 	if (!netdev) {
-		dev_err(netdev->dev.parent, "Couldn't alloc candev\n");
+		printk(KERN_ERR "ems_usb: Couldn't alloc candev\n");
 		return -ENOMEM;
 	}

A message that is not in the archive (sent off-list or not yet received): [email protected]

Wolfgang Grandegger

2010-04-20 19:47 UTC #2 lore raw

Quoted 14 lines · Hans J. Koch

In ems_usb_probe(), a pointer is dereferenced after making sure it is NULL...
This patch replaces dev_err() with printk() to avoid this.
Signed-off-by "Hans J. Koch" <[email protected]>
---
 drivers/net/can/usb/ems_usb.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Index: linux-2.6.34-rc/drivers/net/can/usb/ems_usb.c
===================================================================

drivers/net/can/usb/ems_usb.c line 1011

Quoted 7 lines

drivers/net/can/usb/ems_usb.c +1 -1

@@ -1006,7 +1006,7 @@
 
 	netdev = alloc_candev(sizeof(struct ems_usb), MAX_TX_URBS);
 	if (!netdev) {
-		dev_err(netdev->dev.parent, "Couldn't alloc candev\n");
+		printk(KERN_ERR "ems_usb: Couldn't alloc candev\n");
 		return -ENOMEM;
 	}

I think "dev_err(&intf->dev, ...)" should be used before SET_NETDEV_DEV(netdev, &intf->dev) is called. I see two "dev_err()" calls which need to be fixed.

Wolfgang.

David Miller

maintainer

2010-04-21 01:04 UTC #3 lore raw

From: Wolfgang Grandegger <[email protected]>
Date: Tue, 20 Apr 2010 21:47:37 +0200

Quoted 16 lines

Quoted 15 lines · Hans J. Koch
In ems_usb_probe(), a pointer is dereferenced after making sure it is NULL...
This patch replaces dev_err() with printk() to avoid this.
Signed-off-by "Hans J. Koch" <[email protected]>
---
 drivers/net/can/usb/ems_usb.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Index: linux-2.6.34-rc/drivers/net/can/usb/ems_usb.c
===================================================================
drivers/net/can/usb/ems_usb.c +0 -0
@@ -1006,7 +1006,7 @@
 

Quoted 10 lines

drivers/net/can/usb/ems_usb.c line 1011
Quoted 6 lines
drivers/net/can/usb/ems_usb.c +1 -1
 	netdev = alloc_candev(sizeof(struct ems_usb), MAX_TX_URBS);
 	if (!netdev) {
-		dev_err(netdev->dev.parent, "Couldn't alloc candev\n");
+		printk(KERN_ERR "ems_usb: Couldn't alloc candev\n");
 		return -ENOMEM;
 	}
I think "dev_err(&intf->dev, ...)" should be used before SET_NETDEV_DEV(netdev, &intf->dev) is called. I see two "dev_err()" calls which need to be fixed.

Agreed.

A message that is not in the archive (sent off-list or not yet received): 20100420.180501.08643239.davem-fT/PcQaiUtIeIZ0/[email protected]

Hans J. Koch

author

2010-04-21 10:18 UTC #5 lore raw

Quoted 6 lines · David Miller

From: Wolfgang Grandegger <[email protected]>
Quoted 3 lines
I think "dev_err(&intf->dev, ...)" should be used before SET_NETDEV_DEV(netdev, &intf->dev) is called. I see two "dev_err()" calls which need to be fixed.
Agreed.

Then it should probably look like this:

From: "Hans J. Koch" <[email protected]>
To: [email protected]
Cc: [email protected],
Sebastian Haas <[email protected]>
Subject: [PATCH] can: Fix possible NULL pointer dereference in ems_usb.c

In ems_usb_probe(), a pointer is dereferenced after making sure it is NULL...

This patch replaces netdev->dev.parent with &intf->dev in dev_err() calls to avoid this.

Signed-off-by "Hans J. Koch" <[email protected]>

---
 drivers/net/can/usb/ems_usb.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

drivers/net/can/usb/ems_usb.c +4 -4

Index: net-next-2.6/drivers/net/can/usb/ems_usb.c
===================================================================
@@ -1006,7 +1006,7 @@
 
 	netdev = alloc_candev(sizeof(struct ems_usb), MAX_TX_URBS);
 	if (!netdev) {
-		dev_err(netdev->dev.parent, "Couldn't alloc candev\n");
+		dev_err(&intf->dev, "ems_usb: Couldn't alloc candev\n");
 		return -ENOMEM;
 	}

@@ -1036,20 +1036,20 @@
 
 	dev->intr_urb = usb_alloc_urb(0, GFP_KERNEL);
 	if (!dev->intr_urb) {
-		dev_err(netdev->dev.parent, "Couldn't alloc intr URB\n");
+		dev_err(&intf->dev, "Couldn't alloc intr URB\n");
 		goto cleanup_candev;
 	}
 
 	dev->intr_in_buffer = kzalloc(INTR_IN_BUFFER_SIZE, GFP_KERNEL);
 	if (!dev->intr_in_buffer) {
-		dev_err(netdev->dev.parent, "Couldn't alloc Intr buffer\n");
+		dev_err(&intf->dev, "Couldn't alloc Intr buffer\n");
 		goto cleanup_intr_urb;
 	}
 
 	dev->tx_msg_buffer = kzalloc(CPC_HEADER_SIZE +
 				     sizeof(struct ems_cpc_msg), GFP_KERNEL);
 	if (!dev->tx_msg_buffer) {
-		dev_err(netdev->dev.parent, "Couldn't alloc Tx buffer\n");
+		dev_err(&intf->dev, "Couldn't alloc Tx buffer\n");
 		goto cleanup_intr_in_buffer;
 	}

A message that is not in the archive (sent off-list or not yet received): [email protected]

Wolfgang Grandegger

Acked-by

2010-04-21 10:27 UTC #7 lore raw

Quoted 8 lines · Hans J. Koch

Quoted 5 lines · David Miller
From: Wolfgang Grandegger <[email protected]>
Quoted 3 lines
I think "dev_err(&intf->dev, ...)" should be used before SET_NETDEV_DEV(netdev, &intf->dev) is called. I see two "dev_err()" calls which need to be fixed.
Agreed.
Then it should probably look like this:

There are even more dev_err's to be fixed...

Quoted 42 lines

From: "Hans J. Koch" <[email protected]>
To: [email protected]
Cc: [email protected],
Sebastian Haas <[email protected]>
Subject: [PATCH] can: Fix possible NULL pointer dereference in ems_usb.c
In ems_usb_probe(), a pointer is dereferenced after making sure it is NULL...
This patch replaces netdev->dev.parent with &intf->dev in dev_err() calls to avoid this.
Signed-off-by "Hans J. Koch" <[email protected]>
---
 drivers/net/can/usb/ems_usb.c |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
Index: net-next-2.6/drivers/net/can/usb/ems_usb.c
===================================================================
drivers/net/can/usb/ems_usb.c +3 -3
@@ -1006,7 +1006,7 @@
 
 	netdev = alloc_candev(sizeof(struct ems_usb), MAX_TX_URBS);
 	if (!netdev) {
-		dev_err(netdev->dev.parent, "Couldn't alloc candev\n");
+		dev_err(&intf->dev, "ems_usb: Couldn't alloc candev\n");
 		return -ENOMEM;
 	}
 
@@ -1036,20 +1036,20 @@
 
 	dev->intr_urb = usb_alloc_urb(0, GFP_KERNEL);
 	if (!dev->intr_urb) {
-		dev_err(netdev->dev.parent, "Couldn't alloc intr URB\n");
+		dev_err(&intf->dev, "Couldn't alloc intr URB\n");
 		goto cleanup_candev;
 	}
 
 	dev->intr_in_buffer = kzalloc(INTR_IN_BUFFER_SIZE, GFP_KERNEL);
 	if (!dev->intr_in_buffer) {
-		dev_err(netdev->dev.parent, "Couldn't alloc Intr buffer\n");
+		dev_err(&intf->dev, "Couldn't alloc Intr buffer\n");

drivers/net/can/usb/ems_usb.c line 1054

Quoted 10 lines

drivers/net/can/usb/ems_usb.c +1 -1

 		goto cleanup_intr_urb;
 	}
 
 	dev->tx_msg_buffer = kzalloc(CPC_HEADER_SIZE +
 				     sizeof(struct ems_cpc_msg), GFP_KERNEL);
 	if (!dev->tx_msg_buffer) {
-		dev_err(netdev->dev.parent, "Couldn't alloc Tx buffer\n");
+		dev_err(&intf->dev, "Couldn't alloc Tx buffer\n");
 		goto cleanup_intr_in_buffer;
 	}

Acked-by Wolfgang Grandegger <[email protected]>

David Miller

maintainer Applied

2010-04-21 23:15 UTC #8 lore raw

From: Wolfgang Grandegger <[email protected]>
Date: Wed, 21 Apr 2010 12:27:50 +0200

Quoted 8 lines

Quoted 8 lines
Subject: [PATCH] can: Fix possible NULL pointer dereference in ems_usb.c
In ems_usb_probe(), a pointer is dereferenced after making sure it is NULL...
This patch replaces netdev->dev.parent with &intf->dev in dev_err() calls to avoid this.
Signed-off-by "Hans J. Koch" <[email protected]>

...

Quoted 1 line

Acked-by Wolfgang Grandegger <[email protected]>

Applied, thanks.