Skip to contentSkip to conversation

Remove notifier block from chain when register_netdevice_notifier fails

PATCH In review Pavel Emelyanov 3 participants 5 messages Nov 14, 2007

Subsystems (2)
Networking Network drivers
lore
Share & tools
lore

Pavel Emelyanov

author
#0 lore raw

PATCH 1/1 Remove notifier block from chain when register_netdevice_notifier fails

Commit fcc5a03ac42564e9e255c1134dda47442289e466 makes the register_netdevice_notifier() handle the error from the NETDEV_REGISTER event, sent to the registering block.

The bad news is that in this case the notifier block is not removed from the list, but the error is returned to the caller. In case the caller is in module init function and handles this error this can abort the module loading. The notifier block will be then removed from the kernel, but will be left in the list. Oops :(

I think that the notifier block should be removed from the chain in case of error, regardless whether this error is handled by the caller or not. In the worst case (the error is _not_ handled) module will not receive the events any longer.

Signed-off-by Pavel Emelyanov <[email protected]>

---

net/core/dev.c +2 -0
index dd40b35..86d6261 100644
@@ -1171,6 +1171,8 @@ rollback:
 			nb->notifier_call(nb, NETDEV_UNREGISTER, dev);
 		}
 	}
+
+	raw_notifier_chain_unregister(&netdev_chain, nb);
 	goto unlock;
 }
Acked-by on v1 1/1
#1 lore raw
Quoted 11 lines · Pavel Emelyanov

Commit fcc5a03ac42564e9e255c1134dda47442289e466 makes the register_netdevice_notifier() handle the error from the NETDEV_REGISTER event, sent to the registering block.

The bad news is that in this case the notifier block is not removed from the list, but the error is returned to the caller. In case the caller is in module init function and handles this error this can abort the module loading. The notifier block will be then removed from the kernel, but will be left in the list. Oops :(

Quoted 7 lines

I think that the notifier block should be removed from the chain in case of error, regardless whether this error is handled by the caller or not. In the worst case (the error is _not_ handled) module will not receive the events any longer.

Signed-off-by Pavel Emelyanov <[email protected]>

Good catch. Thanks Pavel!

Acked-by Herbert Xu <[email protected]>
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
author
#2 lore raw
Quoted 18 lines · Herbert Xu
Quoted 17 lines · Pavel Emelyanov

Commit fcc5a03ac42564e9e255c1134dda47442289e466 makes the register_netdevice_notifier() handle the error from the NETDEV_REGISTER event, sent to the registering block.

The bad news is that in this case the notifier block is not removed from the list, but the error is returned to the caller. In case the caller is in module init function and handles this error this can abort the module loading. The notifier block will be then removed from the kernel, but will be left in the list. Oops :(

I think that the notifier block should be removed from the chain in case of error, regardless whether this error is handled by the caller or not. In the worst case (the error is _not_ handled) module will not receive the events any longer.

Quoted 5 lines
Quoted 1 line
Signed-off-by Pavel Emelyanov <[email protected]>

Good catch. Thanks Pavel!

Acked-by Herbert Xu <[email protected]>

Thanks :)

BTW, I have one more question I can't find the answer to by myself. Why the unregister_netdevice_notifier doesn't produce the GOING_DOWN-DOWN-UNREGISTER event sequence for the notifier block which is being unregistered? Was this made deliberately?

Pavel

#3 lore raw
Quoted 5 lines · Pavel Emelyanov

BTW, I have one more question I can't find the answer to by myself. Why the unregister_netdevice_notifier doesn't produce the GOING_DOWN-DOWN-UNREGISTER event sequence for the notifier block which is being unregistered? Was this made deliberately?

I think it's more of an oversight :)

Although it has no real impact because the only user that matters here is IPv6 and it can't be unloaded.

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