Skip to content

Commit f4d4c49

Browse files
committed
Merge branch 'rtnetlink-rework-handler-registration'
Florian Westphal says: ==================== rtnetlink: rework handler (un)registering Peter Zijlstra reported (referring to commit 019a316, "rtnetlink: add reference counting to prevent module unload while dump is in progress"): 1) it not in fact a refcount, so using refcount_t is silly 2) there is a distinct lack of memory barriers, so we can easily observe the decrement while the msg_handler is still in progress. 3) waiting with a schedule()/yield() loop is complete crap and subject life-locks, imagine doing that rtnl_unregister_all() from a RT task. In ancient times rtnetlink exposed a statically-sized table with preset doit/dumpit handlers to be called for a protocol/type pair. Later the rtnl_register interface was added and the table was allocated on demand. Eventually these were also used by modules. Problem is that nothing prevents module unload while a netlink dump is in progress. netlink dumps can be span multiple recv calls and netlink core saves the to-be-repeated dumper address for later invocation. To prevent rmmod the netlink core expects callers to pass in the owning module so a reference can be taken. So far rtnetlink wasn't doing this, add new interface to pass THIS_MODULE. Moreover, when converting parts of the rtnetlink handling to rcu this code gained way too many READ_ONCE spots, remove them and the extra refcounting. Take a module reference when running dumpit and doit callbacks and never alter content of rtnl_link structures after they have been published via rcu_assign_pointer. Based partially on earlier patch from Peter. ==================== Signed-off-by: David S. Miller <[email protected]>
2 parents 9753c21 + 16feebc commit f4d4c49

File tree

14 files changed

+282
-160
lines changed

14 files changed

+282
-160
lines changed

include/net/rtnetlink.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,10 @@ enum rtnl_link_flags {
1313
RTNL_FLAG_DOIT_UNLOCKED = 1,
1414
};
1515

16-
int __rtnl_register(int protocol, int msgtype,
17-
rtnl_doit_func, rtnl_dumpit_func, unsigned int flags);
1816
void rtnl_register(int protocol, int msgtype,
1917
rtnl_doit_func, rtnl_dumpit_func, unsigned int flags);
18+
int rtnl_register_module(struct module *owner, int protocol, int msgtype,
19+
rtnl_doit_func, rtnl_dumpit_func, unsigned int flags);
2020
int rtnl_unregister(int protocol, int msgtype);
2121
void rtnl_unregister_all(int protocol);
2222

net/bridge/br_mdb.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -760,9 +760,9 @@ static int br_mdb_del(struct sk_buff *skb, struct nlmsghdr *nlh,
760760

761761
void br_mdb_init(void)
762762
{
763-
rtnl_register(PF_BRIDGE, RTM_GETMDB, NULL, br_mdb_dump, 0);
764-
rtnl_register(PF_BRIDGE, RTM_NEWMDB, br_mdb_add, NULL, 0);
765-
rtnl_register(PF_BRIDGE, RTM_DELMDB, br_mdb_del, NULL, 0);
763+
rtnl_register_module(THIS_MODULE, PF_BRIDGE, RTM_GETMDB, NULL, br_mdb_dump, 0);
764+
rtnl_register_module(THIS_MODULE, PF_BRIDGE, RTM_NEWMDB, br_mdb_add, NULL, 0);
765+
rtnl_register_module(THIS_MODULE, PF_BRIDGE, RTM_DELMDB, br_mdb_del, NULL, 0);
766766
}
767767

768768
void br_mdb_uninit(void)

net/can/gw.c

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1014,6 +1014,8 @@ static struct pernet_operations cangw_pernet_ops = {
10141014

10151015
static __init int cgw_module_init(void)
10161016
{
1017+
int ret;
1018+
10171019
/* sanitize given module parameter */
10181020
max_hops = clamp_t(unsigned int, max_hops, CGW_MIN_HOPS, CGW_MAX_HOPS);
10191021

@@ -1031,15 +1033,19 @@ static __init int cgw_module_init(void)
10311033
notifier.notifier_call = cgw_notifier;
10321034
register_netdevice_notifier(&notifier);
10331035

1034-
if (__rtnl_register(PF_CAN, RTM_GETROUTE, NULL, cgw_dump_jobs, 0)) {
1036+
ret = rtnl_register_module(THIS_MODULE, PF_CAN, RTM_GETROUTE,
1037+
NULL, cgw_dump_jobs, 0);
1038+
if (ret) {
10351039
unregister_netdevice_notifier(&notifier);
10361040
kmem_cache_destroy(cgw_cache);
10371041
return -ENOBUFS;
10381042
}
10391043

1040-
/* Only the first call to __rtnl_register can fail */
1041-
__rtnl_register(PF_CAN, RTM_NEWROUTE, cgw_create_job, NULL, 0);
1042-
__rtnl_register(PF_CAN, RTM_DELROUTE, cgw_remove_job, NULL, 0);
1044+
/* Only the first call to rtnl_register_module can fail */
1045+
rtnl_register_module(THIS_MODULE, PF_CAN, RTM_NEWROUTE,
1046+
cgw_create_job, NULL, 0);
1047+
rtnl_register_module(THIS_MODULE, PF_CAN, RTM_DELROUTE,
1048+
cgw_remove_job, NULL, 0);
10431049

10441050
return 0;
10451051
}

0 commit comments

Comments
 (0)