Skip to contentSkip to conversation

HDLC driver: use unregister_netdev instead of unregister_netdevice

PATCH Applied Wang Chen 4 participants 6 messages Dec 12, 2007 – Dec 14, 2007

Subsystems (3)
Networking Network drivers Generic hdlc (wan) drivers
lore
Share & tools
lore

Wang Chen

author
#0 lore raw

PATCH 1/1 HDLC driver: use unregister_netdev instead of unregister_netdevice

[PATCH] HDLC driver: use unregister_netdev instead of unregister_netdevice

Since the caller and the upper caller doesn't hod the rtnl semaphore. We should use unregister_netdev instead of unregister_netdevice.

Signed-off-by Wang Chen <[email protected]>
---
 hdlc_fr.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
drivers/net/wan/hdlc_fr.c +3 -3
@@ -1177,7 +1177,7 @@ static int fr_del_pvc(hdlc_device *hdlc,
 	if (dev->flags & IFF_UP)
 		return -EBUSY;		/* PVC in use */
 
-	unregister_netdevice(dev); /* the destructor will free_netdev(dev) */
+	unregister_netdev(dev); /* the destructor will free_netdev(dev) */
 	*get_dev_p(pvc, type) = NULL;
 
 	if (!pvc_is_used(pvc)) {

@@ -1202,10 +1202,10 @@ static void fr_destroy(struct net_device
 		pvc_device *next = pvc->next;
 		/* destructors will free_netdev() main and ether */
 		if (pvc->main)
-			unregister_netdevice(pvc->main);
+			unregister_netdev(pvc->main);
 
 		if (pvc->ether)
-			unregister_netdevice(pvc->ether);
+			unregister_netdev(pvc->ether);
 
 		kfree(pvc);
 		pvc = next;
maintainer Applied
#1 lore raw

From: Wang Chen <[email protected]>
Date: Wed, 12 Dec 2007 10:35:56 +0800

Quoted 6 lines

[PATCH] HDLC driver: use unregister_netdev instead of unregister_netdevice

Since the caller and the upper caller doesn't hod the rtnl semaphore. We should use unregister_netdev instead of unregister_netdevice.

Signed-off-by Wang Chen <[email protected]>

Applied, thanks for finding and fixing this bug.

#2 lore raw
Quoted 6 lines · David Miller

From: Wang Chen <[email protected]>
Date: Wed, 12 Dec 2007 10:35:56 +0800

Quoted 1 line

[PATCH] HDLC driver: use unregister_netdev instead of unregister_netdevice

Applied, thanks for finding and fixing this bug.

Let's see who else is doing this:

[NET]: Check RTNL status in unregister_netdevice

The caller must hold the RTNL so let's check it in unregister_netdevice.

Signed-off-by Herbert Xu <[email protected]>
net/core/dev.c +2 -0
index 06615df..b254e52 100644
@@ -3967,6 +3967,8 @@ void synchronize_net(void)
 
 void unregister_netdevice(struct net_device *dev)
 {
+	ASSERT_RTNL();
+
 	rollback_registered(dev);
 	/* Finish processing unregister after unlock */
 	net_set_todo(dev);

Cheers,

Signature · 4 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
maintainer
#3 lore raw

From: Herbert Xu <[email protected]>
Date: Thu, 13 Dec 2007 11:13:44 +0800

Quoted 7 lines

Let's see who else is doing this:

[NET]: Check RTNL status in unregister_netdevice

The caller must hold the RTNL so let's check it in unregister_netdevice.

Signed-off-by Herbert Xu <[email protected]>

Good idea, applied to net-2.6.25, thanks.

maintainer NAK
#4 lore raw
In reply to Wang Chen
Quoted 4 lines · Wang Chen

[PATCH] HDLC driver: use unregister_netdev instead of unregister_netdevice

Since the caller and the upper caller doesn't hod the rtnl semaphore. We should use unregister_netdev instead of unregister_netdevice.

NAK, not-a-bug. The caller actually holds rtnl, it goes through the netdev core ioctl dispatcher:

(unregister_netdevice+0x0/0x24) from (fr_ioctl+0x688/0x75c) /* fr_del_pvc() and fr_add_pvc() optimized out by gcc */ (fr_ioctl+0x0/0x75c) from (hdlc_ioctl+0x4c/0x8c) (hdlc_ioctl+0x0/0x8c) from (hss_ioctl+0x3c/0x324) (hss_ioctl+0x0/0x324) from (dev_ifsioc+0x428/0x4e8) (dev_ifsioc+0x0/0x4e8) from (dev_ioctl+0x5d8/0x664) (dev_ioctl+0x0/0x664) from (sock_ioctl+0x90/0x254) (sock_ioctl+0x0/0x254) from (do_ioctl+0x34/0x78) (do_ioctl+0x0/0x78) from (vfs_ioctl+0x78/0x2a8) (vfs_ioctl+0x0/0x2a8) from (sys_ioctl+0x40/0x64) (sys_ioctl+0x0/0x64) from (ret_fast_syscall+0x0/0x2c)

The patch would make it deadlock.

Please note that sister fr_add_pvc() uses register_netdevice(). The same applies to fr_destroy().

Signature · 1 line
Krzysztof Halasa
maintainer
#5 lore raw

From: Krzysztof Halasa <[email protected]>
Date: Fri, 14 Dec 2007 22:28:07 +0100

Quoted 22 lines
Quoted 4 lines · Wang Chen

[PATCH] HDLC driver: use unregister_netdev instead of unregister_netdevice

Since the caller and the upper caller doesn't hod the rtnl semaphore. We should use unregister_netdev instead of unregister_netdevice.

NAK, not-a-bug. The caller actually holds rtnl, it goes through the netdev core ioctl dispatcher:

(unregister_netdevice+0x0/0x24) from (fr_ioctl+0x688/0x75c) /* fr_del_pvc() and fr_add_pvc() optimized out by gcc */ (fr_ioctl+0x0/0x75c) from (hdlc_ioctl+0x4c/0x8c) (hdlc_ioctl+0x0/0x8c) from (hss_ioctl+0x3c/0x324) (hss_ioctl+0x0/0x324) from (dev_ifsioc+0x428/0x4e8) (dev_ifsioc+0x0/0x4e8) from (dev_ioctl+0x5d8/0x664) (dev_ioctl+0x0/0x664) from (sock_ioctl+0x90/0x254) (sock_ioctl+0x0/0x254) from (do_ioctl+0x34/0x78) (do_ioctl+0x0/0x78) from (vfs_ioctl+0x78/0x2a8) (vfs_ioctl+0x0/0x2a8) from (sys_ioctl+0x40/0x64) (sys_ioctl+0x0/0x64) from (ret_fast_syscall+0x0/0x2c)

Quoted 1 line

The patch would make it deadlock.

Ok, I'll drop this patch, thanks for checking.