Commit 93ee31f1 authored by Daniel Lezcano's avatar Daniel Lezcano Committed by David S. Miller

[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: default avatarDaniel Lezcano <dlezcano@fr.ibm.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 5c41542b
...@@ -3496,6 +3496,60 @@ static void net_set_todo(struct net_device *dev) ...@@ -3496,6 +3496,60 @@ static void net_set_todo(struct net_device *dev)
spin_unlock(&net_todo_list_lock); 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 * register_netdevice - register a network device
* @dev: device to register * @dev: device to register
...@@ -3633,8 +3687,10 @@ int register_netdevice(struct net_device *dev) ...@@ -3633,8 +3687,10 @@ int register_netdevice(struct net_device *dev)
/* Notify protocols, that a new device appeared. */ /* Notify protocols, that a new device appeared. */
ret = call_netdevice_notifiers(NETDEV_REGISTER, dev); ret = call_netdevice_notifiers(NETDEV_REGISTER, dev);
ret = notifier_to_errno(ret); ret = notifier_to_errno(ret);
if (ret) if (ret) {
unregister_netdevice(dev); rollback_registered(dev);
dev->reg_state = NETREG_UNREGISTERED;
}
out: out:
return ret; return ret;
...@@ -3911,59 +3967,9 @@ void synchronize_net(void) ...@@ -3911,59 +3967,9 @@ void synchronize_net(void)
void unregister_netdevice(struct net_device *dev) void unregister_netdevice(struct net_device *dev)
{ {
BUG_ON(dev_boot_phase); rollback_registered(dev);
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);
/* Finish processing unregister after unlock */ /* Finish processing unregister after unlock */
net_set_todo(dev); net_set_todo(dev);
synchronize_net();
dev_put(dev);
} }
/** /**
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment