diff options
author | Daniel Lezcano <dlezcano@fr.ibm.com> | 2007-10-30 18:38:18 -0400 |
---|---|---|
committer | David S. Miller <davem@sunset.davemloft.net> | 2007-10-31 00:16:18 -0400 |
commit | 93ee31f14f6f7b5b427c2fdc715d5571eb0be9e5 (patch) | |
tree | b2edac4817e12bc85c45d02d77a2226dd5df2d10 /net | |
parent | 5c41542bdeaafe922a07bcdebc10d96a3b8ffeee (diff) |
[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')
-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 02e7d8377c4a..91ece48e127e 100644 --- a/net/core/dev.c +++ b/net/core/dev.c | |||
@@ -3496,6 +3496,60 @@ static void net_set_todo(struct net_device *dev) | |||
3496 | spin_unlock(&net_todo_list_lock); | 3496 | spin_unlock(&net_todo_list_lock); |
3497 | } | 3497 | } |
3498 | 3498 | ||
3499 | static void rollback_registered(struct net_device *dev) | ||
3500 | { | ||
3501 | BUG_ON(dev_boot_phase); | ||
3502 | ASSERT_RTNL(); | ||
3503 | |||
3504 | /* Some devices call without registering for initialization unwind. */ | ||
3505 | if (dev->reg_state == NETREG_UNINITIALIZED) { | ||
3506 | printk(KERN_DEBUG "unregister_netdevice: device %s/%p never " | ||
3507 | "was registered\n", dev->name, dev); | ||
3508 | |||
3509 | WARN_ON(1); | ||
3510 | return; | ||
3511 | } | ||
3512 | |||
3513 | BUG_ON(dev->reg_state != NETREG_REGISTERED); | ||
3514 | |||
3515 | /* If device is running, close it first. */ | ||
3516 | dev_close(dev); | ||
3517 | |||
3518 | /* And unlink it from device chain. */ | ||
3519 | unlist_netdevice(dev); | ||
3520 | |||
3521 | dev->reg_state = NETREG_UNREGISTERING; | ||
3522 | |||
3523 | synchronize_net(); | ||
3524 | |||
3525 | /* Shutdown queueing discipline. */ | ||
3526 | dev_shutdown(dev); | ||
3527 | |||
3528 | |||
3529 | /* Notify protocols, that we are about to destroy | ||
3530 | this device. They should clean all the things. | ||
3531 | */ | ||
3532 | call_netdevice_notifiers(NETDEV_UNREGISTER, dev); | ||
3533 | |||
3534 | /* | ||
3535 | * Flush the unicast and multicast chains | ||
3536 | */ | ||
3537 | dev_addr_discard(dev); | ||
3538 | |||
3539 | if (dev->uninit) | ||
3540 | dev->uninit(dev); | ||
3541 | |||
3542 | /* Notifier chain MUST detach us from master device. */ | ||
3543 | BUG_TRAP(!dev->master); | ||
3544 | |||
3545 | /* Remove entries from kobject tree */ | ||
3546 | netdev_unregister_kobject(dev); | ||
3547 | |||
3548 | synchronize_net(); | ||
3549 | |||
3550 | dev_put(dev); | ||
3551 | } | ||
3552 | |||
3499 | /** | 3553 | /** |
3500 | * register_netdevice - register a network device | 3554 | * register_netdevice - register a network device |
3501 | * @dev: device to register | 3555 | * @dev: device to register |
@@ -3633,8 +3687,10 @@ int register_netdevice(struct net_device *dev) | |||
3633 | /* Notify protocols, that a new device appeared. */ | 3687 | /* Notify protocols, that a new device appeared. */ |
3634 | ret = call_netdevice_notifiers(NETDEV_REGISTER, dev); | 3688 | ret = call_netdevice_notifiers(NETDEV_REGISTER, dev); |
3635 | ret = notifier_to_errno(ret); | 3689 | ret = notifier_to_errno(ret); |
3636 | if (ret) | 3690 | if (ret) { |
3637 | unregister_netdevice(dev); | 3691 | rollback_registered(dev); |
3692 | dev->reg_state = NETREG_UNREGISTERED; | ||
3693 | } | ||
3638 | 3694 | ||
3639 | out: | 3695 | out: |
3640 | return ret; | 3696 | return ret; |
@@ -3911,59 +3967,9 @@ void synchronize_net(void) | |||
3911 | 3967 | ||
3912 | void unregister_netdevice(struct net_device *dev) | 3968 | void unregister_netdevice(struct net_device *dev) |
3913 | { | 3969 | { |
3914 | BUG_ON(dev_boot_phase); | 3970 | rollback_registered(dev); |
3915 | ASSERT_RTNL(); | ||
3916 | |||
3917 | /* Some devices call without registering for initialization unwind. */ | ||
3918 | if (dev->reg_state == NETREG_UNINITIALIZED) { | ||
3919 | printk(KERN_DEBUG "unregister_netdevice: device %s/%p never " | ||
3920 | "was registered\n", dev->name, dev); | ||
3921 | |||
3922 | WARN_ON(1); | ||
3923 | return; | ||
3924 | } | ||
3925 | |||
3926 | BUG_ON(dev->reg_state != NETREG_REGISTERED); | ||
3927 | |||
3928 | /* If device is running, close it first. */ | ||
3929 | dev_close(dev); | ||
3930 | |||
3931 | /* And unlink it from device chain. */ | ||
3932 | unlist_netdevice(dev); | ||
3933 | |||
3934 | dev->reg_state = NETREG_UNREGISTERING; | ||
3935 | |||
3936 | synchronize_net(); | ||
3937 | |||
3938 | /* Shutdown queueing discipline. */ | ||
3939 | dev_shutdown(dev); | ||
3940 | |||
3941 | |||
3942 | /* Notify protocols, that we are about to destroy | ||
3943 | this device. They should clean all the things. | ||
3944 | */ | ||
3945 | call_netdevice_notifiers(NETDEV_UNREGISTER, dev); | ||
3946 | |||
3947 | /* | ||
3948 | * Flush the unicast and multicast chains | ||
3949 | */ | ||
3950 | dev_addr_discard(dev); | ||
3951 | |||
3952 | if (dev->uninit) | ||
3953 | dev->uninit(dev); | ||
3954 | |||
3955 | /* Notifier chain MUST detach us from master device. */ | ||
3956 | BUG_TRAP(!dev->master); | ||
3957 | |||
3958 | /* Remove entries from kobject tree */ | ||
3959 | netdev_unregister_kobject(dev); | ||
3960 | |||
3961 | /* Finish processing unregister after unlock */ | 3971 | /* Finish processing unregister after unlock */ |
3962 | net_set_todo(dev); | 3972 | net_set_todo(dev); |
3963 | |||
3964 | synchronize_net(); | ||
3965 | |||
3966 | dev_put(dev); | ||
3967 | } | 3973 | } |
3968 | 3974 | ||
3969 | /** | 3975 | /** |