Skip to contentSkip to conversation

eth: remove usb-ethernet devices before re-enumerating them

PATCH New Vincent Palatin 6 participants 9 messages Jan 9, 2012 – Mar 8, 2012

Subsystems (2)
Networking USB
lore
Share & tools
lore

Vincent Palatin

author
#0 lore raw

PATCH 1/1 eth: remove usb-ethernet devices before re-enumerating them

Fix the crash when running several times usb_init() with a USB ethernet device plugged.

Signed-off-by Vincent Palatin <[email protected]>
Tested-by Wolfgang Grandegger <[email protected]>
---
 drivers/usb/eth/usb_ether.c |    7 +++++--
 include/net.h               |    1 +
 net/eth.c                   |   29 +++++++++++++++++++++++++++++
 3 files changed, 35 insertions(+), 2 deletions(-)
drivers/usb/eth/usb_ether.c +5 -2
index 6565ea5..73a0790 100644
@@ -127,8 +127,11 @@ int usb_host_eth_scan(int mode)
 
 	old_async = usb_disable_asynch(1); /* asynch transfer not allowed */
 
-	for (i = 0; i < USB_MAX_ETH_DEV; i++)
-		memset(&usb_eth[i], 0, sizeof(usb_eth[i]));
+	/* unregister a previously detected device */
+	for (i = 0; i < usb_max_eth_dev; i++)
+		eth_unregister(&usb_eth[i].eth_dev);
+
+	memset(usb_eth, 0, sizeof(usb_eth));
 
 	for (i = 0; prob_dev[i].probe; i++) {
 		if (prob_dev[i].before_probe)
include/net.h +1 -0
index e4d42c2..1707a7f 100644
@@ -96,6 +96,7 @@ struct eth_device {
 
 extern int eth_initialize(bd_t *bis);	/* Initialize network subsystem */
 extern int eth_register(struct eth_device* dev);/* Register network device */
+extern int eth_unregister(struct eth_device *dev);/* Remove network device */
 extern void eth_try_another(int first_restart);	/* Change the device */
 extern void eth_set_current(void);		/* set nterface to ethcur var */
 extern struct eth_device *eth_get_dev(void);	/* get the current device MAC */
net/eth.c +29 -0
index b4b9b43..3fb5fb6 100644
@@ -224,6 +224,35 @@ int eth_register(struct eth_device *dev)
 	return 0;
 }
 
+int eth_unregister(struct eth_device *dev)
+{
+	struct eth_device *cur;
+
+	/* No device */
+	if (!eth_devices)
+		return -1;
+
+	for (cur = eth_devices; cur->next != eth_devices && cur->next != dev;
+	     cur = cur->next)
+		;
+
+	/* Device not found */
+	if (cur->next != dev)
+		return -1;
+
+	cur->next = dev->next;
+
+	if (eth_devices == dev)
+		eth_devices = dev->next == eth_devices ? NULL : dev->next;
+
+	if (eth_current == dev) {
+		eth_current = eth_devices;
+		eth_current_changed();
+	}
+
+	return 0;
+}
+
 int eth_initialize(bd_t *bis)
 {
 	int num_devices = 0;
Signature · 1 line
1.7.3.1
#1 lore raw

PATCH 1/1 eth: remove usb-ethernet devices before re-enumerating them

Quoted 34 lines

Fix the crash when running several times usb_init() with a USB ethernet device plugged.

Signed-off-by Vincent Palatin <[email protected]>
Tested-by Wolfgang Grandegger <[email protected]>
---
 drivers/usb/eth/usb_ether.c |    7 +++++--
 include/net.h               |    1 +
 net/eth.c                   |   29 +++++++++++++++++++++++++++++
 3 files changed, 35 insertions(+), 2 deletions(-)
drivers/usb/eth/usb_ether.c +5 -2
index 6565ea5..73a0790 100644
@@ -127,8 +127,11 @@ int usb_host_eth_scan(int mode)
 
 	old_async = usb_disable_asynch(1); /* asynch transfer not allowed */
 
-	for (i = 0; i < USB_MAX_ETH_DEV; i++)
-		memset(&usb_eth[i], 0, sizeof(usb_eth[i]));
+	/* unregister a previously detected device */
+	for (i = 0; i < usb_max_eth_dev; i++)
+		eth_unregister(&usb_eth[i].eth_dev);
+
+	memset(usb_eth, 0, sizeof(usb_eth));
 
 	for (i = 0; prob_dev[i].probe; i++) {
 		if (prob_dev[i].before_probe)
include/net.h +0 -0
index e4d42c2..1707a7f 100644

include/net.h struct eth_device { · line 97

Quoted 2 lines
include/net.h +0 -0
@@ -96,6 +96,7 @@ struct eth_device {
 
 extern int eth_initialize(bd_t *bis);	/* Initialize network subsystem

*/

Quoted 4 lines
include/net.h +0 -0
 extern int eth_register(struct eth_device* dev);/* Register network device
 */ +extern int eth_unregister(struct eth_device *dev);/* Remove network

device */ extern void eth_try_another(int first_restart); /* Change the
device */ extern void eth_set_current(void); /* set nterface to

ethcur

Quoted 31 lines

var */ extern struct eth_device *eth_get_dev(void); /* get the current
device MAC */ diff --git a/net/eth.c b/net/eth.c
index b4b9b43..3fb5fb6 100644

net/eth.c +22 -0
@@ -224,6 +224,35 @@ int eth_register(struct eth_device *dev)
 	return 0;
 }
 
+int eth_unregister(struct eth_device *dev)
+{
+	struct eth_device *cur;
+
+	/* No device */
+	if (!eth_devices)
+		return -1;
+
+	for (cur = eth_devices; cur->next != eth_devices && cur->next != dev;
+	     cur = cur->next)
+		;
+
+	/* Device not found */
+	if (cur->next != dev)
+		return -1;
+
+	cur->next = dev->next;
+
+	if (eth_devices == dev)
+		eth_devices = dev->next == eth_devices ? NULL : dev->next;
+
+	if (eth_current == dev) {

net/eth.c int eth_register(struct eth_device *dev) · line 258

Quoted 10 lines
net/eth.c +7 -0
+		eth_current = eth_devices;
+		eth_current_changed();
+	}
+
+	return 0;
+}
+
 int eth_initialize(bd_t *bis)
 {
 	int num_devices = 0;

Looks sane. On what hardware did this happen (just for the record)?

M

author
#2 lore raw

PATCH 1/1 eth: remove usb-ethernet devices before re-enumerating them

Quoted 77 lines · Marek Vasut
Quoted 35 lines

Fix the crash when running several times usb_init() with a USB ethernet device plugged.

Signed-off-by Vincent Palatin <[email protected]>
Tested-by Wolfgang Grandegger <[email protected]>

---

?drivers/usb/eth/usb_ether.c | ? ?7 +++++--
?include/net.h ? ? ? ? ? ? ? | ? ?1 +
?net/eth.c ? ? ? ? ? ? ? ? ? | ? 29 +++++++++++++++++++++++++++++

?3 files changed, 35 insertions(+), 2 deletions(-)

drivers/usb/eth/usb_ether.c +0 -0
index 6565ea5..73a0790 100644
@@ -127,8 +127,11 @@ int usb_host_eth_scan(int mode)

? ? ? old_async = usb_disable_asynch(1); /* asynch transfer not allowed */

- ? ? for (i = 0; i < USB_MAX_ETH_DEV; i++)
- ? ? ? ? ? ? memset(&usb_eth[i], 0, sizeof(usb_eth[i]));
+ ? ? /* unregister a previously detected device */
+ ? ? for (i = 0; i < usb_max_eth_dev; i++)
+ ? ? ? ? ? ? eth_unregister(&usb_eth[i].eth_dev); +
+ ? ? memset(usb_eth, 0, sizeof(usb_eth));

? ? ? for (i = 0; prob_dev[i].probe; i++) {
? ? ? ? ? ? ? if (prob_dev[i].before_probe)

include/net.h +0 -0
index e4d42c2..1707a7f 100644
@@ -96,6 +96,7 @@ struct eth_device {
Quoted 1 line

?extern int eth_initialize(bd_t *bis); ? ? ? ?/* Initialize network subsystem

*/

Quoted 4 lines

?extern int eth_register(struct eth_device* dev);/* Register network device */ +extern int eth_unregister(struct eth_device *dev);/* Remove network device */ extern void eth_try_another(int first_restart); ? ? /* Change the device */ extern void eth_set_current(void); ? ? ? ? ?/* set nterface to

ethcur

Quoted 35 lines

var */ extern struct eth_device *eth_get_dev(void); ? /* get the current device MAC */ diff --git a/net/eth.c b/net/eth.c
index b4b9b43..3fb5fb6 100644

net/eth.c +26 -0
@@ -224,6 +224,35 @@ int eth_register(struct eth_device *dev)
 ? ? ? return 0;
 ?}
 
+int eth_unregister(struct eth_device *dev)
+{
+ ? ? struct eth_device *cur;
+
+ ? ? /* No device */
+ ? ? if (!eth_devices)
+ ? ? ? ? ? ? return -1;
+
+ ? ? for (cur = eth_devices; cur->next != eth_devices && cur->next != dev;
+ ? ? ? ? ?cur = cur->next)
+ ? ? ? ? ? ? ;
+
+ ? ? /* Device not found */
+ ? ? if (cur->next != dev)
+ ? ? ? ? ? ? return -1;
+
+ ? ? cur->next = dev->next;
+
+ ? ? if (eth_devices == dev)
+ ? ? ? ? ? ? eth_devices = dev->next == eth_devices ? NULL : dev->next;
+
+ ? ? if (eth_current == dev) {
+ ? ? ? ? ? ? eth_current = eth_devices;
+ ? ? ? ? ? ? eth_current_changed();
+ ? ? }
+
Quoted 8 lines

net/eth.c int eth_register(struct eth_device *dev) · line 258

Quoted 6 lines
net/eth.c +3 -0
+ ? ? return 0;
+}
+
 ?int eth_initialize(bd_t *bis)
 ?{
 ? ? ? int num_devices = 0;

Looks sane. On what hardware did this happen (just for the record)?

I got it on x86 and ARM (tegra2) platforms (both with an EHCI controller). I did most of my testing with ASIX-based USB-ethernet dongle, but the issue seems pretty generic.

Signature · 1 line
Vincent
#3 lore raw

PATCH 1/1 eth: remove usb-ethernet devices before re-enumerating them

Quoted 87 lines · Vincent Palatin
Quoted 77 lines · Marek Vasut
Quoted 34 lines

Fix the crash when running several times usb_init() with a USB ethernet device plugged.

Signed-off-by Vincent Palatin <[email protected]>
Tested-by Wolfgang Grandegger <[email protected]>
---
 drivers/usb/eth/usb_ether.c |    7 +++++--
 include/net.h               |    1 +
 net/eth.c                   |   29 +++++++++++++++++++++++++++++
 3 files changed, 35 insertions(+), 2 deletions(-)
drivers/usb/eth/usb_ether.c +5 -2
index 6565ea5..73a0790 100644
@@ -127,8 +127,11 @@ int usb_host_eth_scan(int mode)
 
      old_async = usb_disable_asynch(1); /* asynch transfer not allowed */
 
-     for (i = 0; i < USB_MAX_ETH_DEV; i++)
-             memset(&usb_eth[i], 0, sizeof(usb_eth[i]));
+     /* unregister a previously detected device */
+     for (i = 0; i < usb_max_eth_dev; i++)
+             eth_unregister(&usb_eth[i].eth_dev);
+
+     memset(usb_eth, 0, sizeof(usb_eth));
 
      for (i = 0; prob_dev[i].probe; i++) {
              if (prob_dev[i].before_probe)
include/net.h +0 -0
index e4d42c2..1707a7f 100644

include/net.h struct eth_device { · line 97

Quoted 2 lines
include/net.h +0 -0
@@ -96,6 +96,7 @@ struct eth_device {
 
 extern int eth_initialize(bd_t *bis);        /* Initialize network subsystem

*/

Quoted 4 lines
include/net.h +0 -0
 extern int eth_register(struct eth_device* dev);/* Register network device
 */ +extern int eth_unregister(struct eth_device *dev);/* Remove network

device */ extern void eth_try_another(int first_restart); /* Change the
device */ extern void eth_set_current(void); /* set nterface to

ethcur

Quoted 35 lines

var */ extern struct eth_device *eth_get_dev(void); /* get the current
device MAC */ diff --git a/net/eth.c b/net/eth.c
index b4b9b43..3fb5fb6 100644

net/eth.c +26 -0
@@ -224,6 +224,35 @@ int eth_register(struct eth_device *dev)
      return 0;
 }
 
+int eth_unregister(struct eth_device *dev)
+{
+     struct eth_device *cur;
+
+     /* No device */
+     if (!eth_devices)
+             return -1;
+
+     for (cur = eth_devices; cur->next != eth_devices && cur->next != dev;
+          cur = cur->next)
+             ;
+
+     /* Device not found */
+     if (cur->next != dev)
+             return -1;
+
+     cur->next = dev->next;
+
+     if (eth_devices == dev)
+             eth_devices = dev->next == eth_devices ? NULL : dev->next;
+
+     if (eth_current == dev) {
+             eth_current = eth_devices;
+             eth_current_changed();
+     }
+
Quoted 8 lines

net/eth.c int eth_register(struct eth_device *dev) · line 258

Quoted 6 lines
net/eth.c +3 -0
+     return 0;
+}
+
 int eth_initialize(bd_t *bis)
 {
      int num_devices = 0;

Looks sane. On what hardware did this happen (just for the record)?

Quoted 3 lines

I got it on x86 and ARM (tegra2) platforms (both with an EHCI controller). I did most of my testing with ASIX-based USB-ethernet dongle, but the issue seems pretty generic.

It happens with *any* hardware calling eth_register() more than once because "struct eth_device *dev*" is memset 0 before calling eth_register() resulting in an endless loop when scanning the devices.

Viele Gr??e,

Wolfgang

#4 lore raw

PATCH 1/1 eth: remove usb-ethernet devices before re-enumerating them

Quoted 104 lines · Vincent Palatin
Quoted 103 lines · Marek Vasut
Quoted 101 lines
Quoted 38 lines

Fix the crash when running several times usb_init() with a USB ethernet device plugged.

Signed-off-by Vincent Palatin <[email protected]>
Tested-by Wolfgang Grandegger <[email protected]>

---

 drivers/usb/eth/usb_ether.c |    7 +++++--
 include/net.h               |    1 +
 net/eth.c                   |   29 +++++++++++++++++++++++++++++
 3 files changed, 35 insertions(+), 2 deletions(-)
drivers/usb/eth/usb_ether.c +5 -2
index 6565ea5..73a0790 100644
@@ -127,8 +127,11 @@ int usb_host_eth_scan(int mode)
 
      old_async = usb_disable_asynch(1); /* asynch transfer not allowed
      */
 
-     for (i = 0; i < USB_MAX_ETH_DEV; i++)
-             memset(&usb_eth[i], 0, sizeof(usb_eth[i]));
+     /* unregister a previously detected device */
+     for (i = 0; i < usb_max_eth_dev; i++)
+             eth_unregister(&usb_eth[i].eth_dev);
+
+     memset(usb_eth, 0, sizeof(usb_eth));
 
      for (i = 0; prob_dev[i].probe; i++) {

if (prob_dev[i].before_probe)

include/net.h +0 -0
index e4d42c2..1707a7f 100644

include/net.h struct eth_device { · line 98

Quoted 3 lines
include/net.h +0 -0
@@ -96,6 +96,7 @@ struct eth_device {
 
 extern int eth_initialize(bd_t *bis);        /* Initialize network
 subsystem

*/

Quoted 7 lines
include/net.h +0 -0
 extern int eth_register(struct eth_device* dev);/* Register network
 device

*/ +extern int eth_unregister(struct eth_device *dev);/* Remove network
device */ extern void eth_try_another(int first_restart); /* Change
the device */ extern void eth_set_current(void); /* set
nterface to

ethcur

Quoted 35 lines

var */ extern struct eth_device *eth_get_dev(void); /* get the
current device MAC */ diff --git a/net/eth.c b/net/eth.c index b4b9b43..3fb5fb6 100644

net/eth.c +19 -0
@@ -224,6 +224,35 @@ int eth_register(struct eth_device *dev)
 
      return 0;
 
 }
 
+int eth_unregister(struct eth_device *dev)
+{
+     struct eth_device *cur;
+
+     /* No device */
+     if (!eth_devices)
+             return -1;
+
+     for (cur = eth_devices; cur->next != eth_devices && cur->next !=
 dev; +          cur = cur->next)
+             ;
+
+     /* Device not found */
+     if (cur->next != dev)
+             return -1;
+
+     cur->next = dev->next;
+
+     if (eth_devices == dev)
+             eth_devices = dev->next == eth_devices ? NULL :

dev->next; +

net/eth.c +3 -0
+     if (eth_current == dev) {
+             eth_current = eth_devices;
+             eth_current_changed();

net/eth.c int eth_register(struct eth_device *dev) · line 258

Quoted 10 lines
net/eth.c +5 -0
+     }
+
+     return 0;
+}
+
 
 int eth_initialize(bd_t *bis)
 {
 
      int num_devices = 0;

Looks sane. On what hardware did this happen (just for the record)?

Quoted 7 lines
Quoted 3 lines

I got it on x86 and ARM (tegra2) platforms (both with an EHCI controller). I did most of my testing with ASIX-based USB-ethernet dongle, but the issue seems pretty generic.

It happens with *any* hardware calling eth_register() more than once because "struct eth_device *dev*" is memset 0 before calling eth_register() resulting in an endless loop when scanning the devices.

Yes, I'll try to remember to test this in the evening.

M

Quoted 4 lines

Viele Gr??e,

Wolfgang

#5 lore raw

PATCH 1/1 eth: remove usb-ethernet devices before re-enumerating them

In reply to Vincent Palatin

Hi,

2012/1/9 Vincent Palatin <[email protected]>:

Quoted 11 lines

Fix the crash when running several times usb_init() with a USB ethernet device plugged.

Signed-off-by Vincent Palatin <[email protected]>
Tested-by Wolfgang Grandegger <[email protected]>

---

?drivers/usb/eth/usb_ether.c | ? ?7 +++++--
?include/net.h ? ? ? ? ? ? ? | ? ?1 +
?net/eth.c ? ? ? ? ? ? ? ? ? | ? 29 +++++++++++++++++++++++++++++

?3 files changed, 35 insertions(+), 2 deletions(-)

Applied to u-boot-usb. Thanks.

Kind regards,

Remy

#7 lore raw

PATCH 1/1 eth: remove usb-ethernet devices before re-enumerating them

Hi Marek,

Quoted 10 lines · Marek Vasut
Quoted 6 lines

Fix the crash when running several times usb_init() with a USB ethernet device plugged.

Signed-off-by Vincent Palatin <[email protected]>
Tested-by Wolfgang Grandegger <[email protected]>

---

Hi,

what's the status of this patch/patchset?

I believe it should be applied.

Regards,
Simon

Quoted 7 lines

Thanks
M
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

#8 lore raw

PATCH 1/1 eth: remove usb-ethernet devices before re-enumerating them

Dear Simon Glass,

Quoted 10 lines

Hi Marek,

Quoted 7 lines · Marek Vasut
Quoted 6 lines

Fix the crash when running several times usb_init() with a USB ethernet device plugged.

Signed-off-by Vincent Palatin <[email protected]>
Tested-by Wolfgang Grandegger <[email protected]>

---

Quoted 9 lines
Quoted 3 lines

Hi,

what's the status of this patch/patchset?

I believe it should be applied.

Regards,
Simon

Should be applied to linux-usb now, thanks!

Best regards,
Marek Vasut