diff options
author | Daniel Lezcano <dlezcano@fr.ibm.com> | 2007-10-30 22:38:18 (GMT) |
---|---|---|
committer | David S. Miller <davem@sunset.davemloft.net> | 2007-10-31 04:16:18 (GMT) |
commit | 93ee31f14f6f7b5b427c2fdc715d5571eb0be9e5 (patch) | |
tree | b2edac4817e12bc85c45d02d77a2226dd5df2d10 /net/core | |
parent | 5c41542bdeaafe922a07bcdebc10d96a3b8ffeee (diff) | |
download | linux-fsl-qoriq-93ee31f14f6f7b5b427c2fdc715d5571eb0be9e5.tar.xz |
[NET]: Fix free_netdev on register_netdev failure.
Point 1:
The unregistering of a network device schedule a netdev_run_todo.
This function calls dev->destructor when it is set and the
destructor calls free_netdev.
Point 2:
In the case of an initialization of a network device the usual code
is:
* alloc_netdev
* register_netdev
-> if this one fails, call free_netdev and exit with error.
Point 3:
In the register_netdevice function at the later state, when the device
is at the registered state, a call to the netdevice_notifiers is made.
If one of the notification falls into an error, a rollback to the
registered state is done using unregister_netdevice.
Conclusion:
When a network device fails to register during initialization because
one network subsystem returned an error during a notification call
chain, the network device is freed twice because of fact 1 and fact 2.
The second free_netdev will be done with an invalid pointer.
Proposed solution:
The following patch move all the code of unregister_netdevice *except*
the call to net_set_todo, to a new function "rollback_registered".
The following functions are changed in this way:
* register_netdevice: calls rollback_registered when a notification fails
* unregister_netdevice: calls rollback_register + net_set_todo, the call
order to net_set_todo is changed because it is the
latest now. Since it justs add an element to a list
that should not break anything.
Signed-off-by: Daniel Lezcano <dlezcano@fr.ibm.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'net/core')
-rw-r--r-- | net/core/dev.c | 112 |
1 files changed, 59 insertions, 53 deletions
diff --git a/net/core/dev.c b/net/core/dev.c index 02e7d83..91ece48 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -3496,6 +3496,60 @@ static void net_set_todo(struct net_device *dev) spin_unlock(&net_todo_list_lock); } +static void rollback_registered(struct net_device *dev) +{ + BUG_ON(dev_boot_phase); + ASSERT_RTNL(); + + /* Some devices call without registering for initialization unwind. */ + if (dev->reg_state == NETREG_UNINITIALIZED) { + printk(KERN_DEBUG "unregister_netdevice: device %s/%p never " + "was registered\n", dev->name, dev); + + WARN_ON(1); + return; + } + + BUG_ON(dev->reg_state != NETREG_REGISTERED); + + /* If device is running, close it first. */ + dev_close(dev); + + /* And unlink it from device chain. */ + unlist_netdevice(dev); + + dev->reg_state = NETREG_UNREGISTERING; + + synchronize_net(); + + /* Shutdown queueing discipline. */ + dev_shutdown(dev); + + + /* Notify protocols, that we are about to destroy + this device. They should clean all the things. + */ + call_netdevice_notifiers(NETDEV_UNREGISTER, dev); + + /* + * Flush the unicast and multicast chains + */ + dev_addr_discard(dev); + + if (dev->uninit) + dev->uninit(dev); + + /* Notifier chain MUST detach us from master device. */ + BUG_TRAP(!dev->master); + + /* Remove entries from kobject tree */ + netdev_unregister_kobject(dev); + + synchronize_net(); + + dev_put(dev); +} + /** * register_netdevice - register a network device * @dev: device to register @@ -3633,8 +3687,10 @@ int register_netdevice(struct net_device *dev) /* Notify protocols, that a new device appeared. */ ret = call_netdevice_notifiers(NETDEV_REGISTER, dev); ret = notifier_to_errno(ret); - if (ret) - unregister_netdevice(dev); + if (ret) { + rollback_registered(dev); + dev->reg_state = NETREG_UNREGISTERED; + } out: return ret; @@ -3911,59 +3967,9 @@ void synchronize_net(void) void unregister_netdevice(struct net_device *dev) { - BUG_ON(dev_boot_phase); - ASSERT_RTNL(); - - /* Some devices call without registering for initialization unwind. */ - if (dev->reg_state == NETREG_UNINITIALIZED) { - printk(KERN_DEBUG "unregister_netdevice: device %s/%p never " - "was registered\n", dev->name, dev); - - WARN_ON(1); - return; - } - - BUG_ON(dev->reg_state != NETREG_REGISTERED); - - /* If device is running, close it first. */ - dev_close(dev); - - /* And unlink it from device chain. */ - unlist_netdevice(dev); - - dev->reg_state = NETREG_UNREGISTERING; - - synchronize_net(); - - /* Shutdown queueing discipline. */ - dev_shutdown(dev); - - - /* Notify protocols, that we are about to destroy - this device. They should clean all the things. - */ - call_netdevice_notifiers(NETDEV_UNREGISTER, dev); - - /* - * Flush the unicast and multicast chains - */ - dev_addr_discard(dev); - - if (dev->uninit) - dev->uninit(dev); - - /* Notifier chain MUST detach us from master device. */ - BUG_TRAP(!dev->master); - - /* Remove entries from kobject tree */ - netdev_unregister_kobject(dev); - + rollback_registered(dev); /* Finish processing unregister after unlock */ net_set_todo(dev); - - synchronize_net(); - - dev_put(dev); } /** |