[PATCH nf] netfilter: nf_tables: work around newrule after chain binding

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]


nftables versions prior to
commit 3975430b12d9 ("src: expand table command before evaluation"), i.e.
1.0.6 and earlier, will handle the following snippet in the wrong order:

table ip t {
	chain c {
		jump { counter; }

1. create the table, chain,c and an anon chain.
2. append a rule to chain c to jump to the anon chain.
3. append the rule(s) (here: "counter") to the anon chain.

(step 3 should be before 2).

With below commit, this is now rejected by the kernel.

Reason is that the 'jump {' rule added to chain c adds an explicit binding
(dependency), i.e. the kernel will automatically remove the anon chain when
userspace later asks to delete the 'jump {' rule from chain c.

This caused crashes in the kernel in case of a errors further down
in the same transaction.

The abort part has to unroll all pending changes, including the request to
add the rule 'jump {'.  As its already bound, all the rules added to it
get deleted as well.

Because we tolerated late-add-after-bind, the transaction log also contains
the NEWRULE requests (here "counter"), so those were deleted again.

Instead of rejecting newrule-to-bound-chain, allow it iff the anon chain
is new in this transaction and we are appending.

Mark the newrule transaction as already_bound so abort path skips them.

Fixes: 0ebc1064e487 ("netfilter: nf_tables: disallow rule addition to bound chain via NFTA_RULE_CHAIN_ID")
Reported-by: Timo Sigurdsson <public_timo.s@xxxxxxxxxxxxxx>
Closes: https://lore.kernel.org/netfilter-devel/20230911213750.5B4B663206F5@xxxxxxxxxxxxxxxxxxxxx/
Signed-off-by: Florian Westphal <fw@xxxxxxxxx>
 net/netfilter/nf_tables_api.c | 33 +++++++++++++++++++++++++++++++--
 1 file changed, 31 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index a72b6aeefb1b..dd5b78a58023 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -3805,6 +3805,26 @@ static struct nft_rule *nft_rule_lookup_byid(const struct net *net,
 					     const struct nft_chain *chain,
 					     const struct nlattr *nla);
+/* nft <= 1.0.6 appends rules to anon chains after they have been bound */
+static bool nft_rule_work_around_old_version(const struct nfnl_info *info,
+					     struct nft_chain *chain)
+	/* bound (anonymous) chain is already used */
+	if (nft_is_active(info->net, chain))
+		return false;
+	/* nft never asks to replace rules here */
+	if (info->nlh->nlmsg_flags & (NLM_F_REPLACE | NLM_F_EXCL))
+		return false;
+	/* nft and it only ever appends. */
+	if ((info->nlh->nlmsg_flags & NLM_F_APPEND) == 0)
+		return false;
+	pr_warn_once("enabling workaround for nftables 1.0.6 and older\n");
+	return true;
 #define NFT_RULE_MAXEXPRS	128
 static int nf_tables_newrule(struct sk_buff *skb, const struct nfnl_info *info,
@@ -3818,6 +3838,7 @@ static int nf_tables_newrule(struct sk_buff *skb, const struct nfnl_info *info,
 	struct nft_expr_info *expr_info = NULL;
 	u8 family = info->nfmsg->nfgen_family;
 	struct nft_flow_rule *flow = NULL;
+	bool add_after_bind = false;
 	struct net *net = info->net;
 	struct nft_userdata *udata;
 	struct nft_table *table;
@@ -3857,8 +3878,12 @@ static int nf_tables_newrule(struct sk_buff *skb, const struct nfnl_info *info,
 		return -EINVAL;
-	if (nft_chain_is_bound(chain))
-		return -EOPNOTSUPP;
+	if (nft_chain_is_bound(chain)) {
+		if (!nft_rule_work_around_old_version(info, chain))
+			return -EOPNOTSUPP;
+		add_after_bind = true;
+	}
 	if (nla[NFTA_RULE_HANDLE]) {
 		handle = be64_to_cpu(nla_get_be64(nla[NFTA_RULE_HANDLE]));
@@ -4003,6 +4028,10 @@ static int nf_tables_newrule(struct sk_buff *skb, const struct nfnl_info *info,
 			goto err_destroy_flow_rule;
+		/* work around nftables versions <= 1.0.5 that append rules after bind. */
+		if (add_after_bind)
+			nft_trans_rule_bound(trans) = true;
 		if (info->nlh->nlmsg_flags & NLM_F_APPEND) {
 			if (old_rule)
 				list_add_rcu(&rule->list, &old_rule->list);

[Index of Archives]     [Netfitler Users]     [Berkeley Packet Filter]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux