netfilter: nf_tables: can't fail after linking rule into active rule list (569ccae6) · Commits · 戴 / test

rules in nftables a free'd using kfree, but protected by rcu, i.e. we must wait for a grace period to elapse. Normal removal patch does this, but nf_tables_newrule() doesn't obey this rule during error handling. It calls nft_trans_rule_add() *after* linking rule, and, if that fails to allocate memory, it unlinks the rule and then kfree() it -- this is unsafe. Switch order -- first add rule to transaction list, THEN link it to public list. Note: nft_trans_rule_add() uses GFP_KERNEL; it will not fail so this is not a problem in practice (spotted only during code review). Fixes: ("netfilter: nfnetlink: add batch support and use it from nf_tables") Signed-off-by:

Florian Westphal <fw@strlen.de> Signed-off-by:

Pablo Neira Ayuso <pablo@netfilter.org>

net/netfilter/nf_tables_api.c

+32 −27

Original line number	Diff line number	Diff line
		@@ -2361,7 +2361,10 @@ static int nf_tables_newrule(struct net net, struct sock nlsk,
		}

		if (nlh->nlmsg_flags & NLM_F_REPLACE) {
		if (nft_is_active_next(net, old_rule)) {
		if (!nft_is_active_next(net, old_rule)) {
		err = -ENOENT;
		goto err2;
		}
		trans = nft_trans_rule_add(&ctx, NFT_MSG_DELRULE,
		old_rule);
		if (trans == NULL) {
		@@ -2370,32 +2373,34 @@ static int nf_tables_newrule(struct net net, struct sock nlsk,
		}
		nft_deactivate_next(net, old_rule);
		chain->use--;

		if (nft_trans_rule_add(&ctx, NFT_MSG_NEWRULE, rule) == NULL) {
		err = -ENOMEM;
		goto err2;
		}

		list_add_tail_rcu(&rule->list, &old_rule->list);
		} else {
		err = -ENOENT;
		if (nft_trans_rule_add(&ctx, NFT_MSG_NEWRULE, rule) == NULL) {
		err = -ENOMEM;
		goto err2;
		}
		} else if (nlh->nlmsg_flags & NLM_F_APPEND)

		if (nlh->nlmsg_flags & NLM_F_APPEND) {
		if (old_rule)
		list_add_rcu(&rule->list, &old_rule->list);
		else
		list_add_tail_rcu(&rule->list, &chain->rules);
		else {
		} else {
		if (old_rule)
		list_add_tail_rcu(&rule->list, &old_rule->list);
		else
		list_add_rcu(&rule->list, &chain->rules);
		}

		if (nft_trans_rule_add(&ctx, NFT_MSG_NEWRULE, rule) == NULL) {
		err = -ENOMEM;
		goto err3;
		}
		chain->use++;
		return 0;

		err3:
		list_del_rcu(&rule->list);
		err2:
		nf_tables_rule_destroy(&ctx, rule);
		err1:

Admin message