[PATCH] HDLC driver: use unregister_netdev instead of unregister_netdevice

[PATCH] HDLC driver: use unregister_netdev instead of unregister_netdevice

[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.

Signed-off-by: Wang Chen <wangchen@cn.fujitsu.com>
---
 hdlc_fr.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

--- linux-2.6.24.rc5.org/drivers/net/wan/hdlc_fr.c	2007-10-10 04:31:38.000000000 +0800
+++ linux-2.6.24.rc5/drivers/net/wan/hdlc_fr.c	2007-12-12 10:25:13.000000000 +0800
@@ -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;

Re: [PATCH] HDLC driver: use unregister_netdev instead of unregister_netdevice

[David Miller]

From: Wang Chen <wangchen@cn.fujitsu.com>
Date: Wed, 12 Dec 2007 10:35:56 +0800

> [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 <wangchen@cn.fujitsu.com>

Applied, thanks for finding and fixing this bug.

Re: [PATCH] HDLC driver: use unregister_netdev instead of unregister_netdevice

[Herbert Xu]

David Miller <davem@davemloft.net> wrote:
> From: Wang Chen <wangchen@cn.fujitsu.com>
> Date: Wed, 12 Dec 2007 10:35:56 +0800
> 
>> [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 <herbert@gondor.apana.org.au>

diff --git a/net/core/dev.c b/net/core/dev.c
index 06615df..b254e52 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -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,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH] HDLC driver: use unregister_netdev instead of unregister_netdevice

[David Miller]

From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Thu, 13 Dec 2007 11:13:44 +0800

> 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 <herbert@gondor.apana.org.au>

Good idea, applied to net-2.6.25, thanks.

Re: [PATCH] HDLC driver: use unregister_netdev instead of unregister_netdevice

[Krzysztof Halasa]

Wang Chen <wangchen@cn.fujitsu.com> writes:

> [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().
-- 
Krzysztof Halasa

Re: [PATCH] HDLC driver: use unregister_netdev instead of unregister_netdevice

[David Miller]

From: Krzysztof Halasa <khc@pm.waw.pl>
Date: Fri, 14 Dec 2007 22:28:07 +0100

> Wang Chen <wangchen@cn.fujitsu.com> writes:
> 
> > [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.

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