net_sched: reorder pernet ops and act ops registrations (ab102b80) · Commits · 戴 / test

Krister reported a kernel NULL pointer dereference after tcf_action_init_1() invokes a_o->init(), it is a race condition where one thread calling tcf_register_action() to initialize the netns data after putting act ops in the global list and the other thread searching the list and then calling a_o->init(net, ...). Fix this by moving the pernet ops registration before making the action ops visible. This is fine because: a) we don't rely on act_base in pernet ops->init(), b) in the worst case we have a fully initialized netns but ops is still not ready so new actions still can't be created. Reported-by:

Krister Johansen <kjlx@templeofstupid.com> Tested-by:

Krister Johansen <kjlx@templeofstupid.com> Cc: Jamal Hadi Salim <jhs@mojatatu.com> Signed-off-by:

Cong Wang <xiyou.wangcong@gmail.com> Acked-by:

Jamal Hadi Salim <jhs@mojatatu.com> Signed-off-by:

David S. Miller <davem@davemloft.net>

net/sched/act_api.c

+11 −8

Original line number	Diff line number	Diff line
		@@ -341,22 +341,25 @@ int tcf_register_action(struct tc_action_ops *act,
		if (!act->act \|\| !act->dump \|\| !act->init \|\| !act->walk \|\| !act->lookup)
		return -EINVAL;

		/* We have to register pernet ops before making the action ops visible,
		* otherwise tcf_action_init_1() could get a partially initialized
		* netns.
		*/
		ret = register_pernet_subsys(ops);
		if (ret)
		return ret;

		write_lock(&act_mod_lock);
		list_for_each_entry(a, &act_base, head) {
		if (act->type == a->type \|\| (strcmp(act->kind, a->kind) == 0)) {
		write_unlock(&act_mod_lock);
		unregister_pernet_subsys(ops);
		return -EEXIST;
		}
		}
		list_add_tail(&act->head, &act_base);
		write_unlock(&act_mod_lock);

		ret = register_pernet_subsys(ops);
		if (ret) {
		tcf_unregister_action(act, ops);
		return ret;
		}

		return 0;
		}
		EXPORT_SYMBOL(tcf_register_action);
		@@ -367,8 +370,6 @@ int tcf_unregister_action(struct tc_action_ops *act,
		struct tc_action_ops *a;
		int err = -ENOENT;

		unregister_pernet_subsys(ops);

		write_lock(&act_mod_lock);
		list_for_each_entry(a, &act_base, head) {
		if (a == act) {
		@@ -378,6 +379,8 @@ int tcf_unregister_action(struct tc_action_ops *act,
		}
		}
		write_unlock(&act_mod_lock);
		if (!err)
		unregister_pernet_subsys(ops);
		return err;
		}
		EXPORT_SYMBOL(tcf_unregister_action);

Admin message