Skip to content

Commit d2fd2e4

Browse files
committed
netfilter: nf_tables: work around newrule after chain binding
JIRA: https://issues.redhat.com/browse/RHEL-1720 JIRA: https://issues.redhat.com/browse/RHEL-1721 Upstream Status: RHEL only RHEL only. Proposed upstream but was rejected. I don't think we can force a rebase of nftables userland in RHEL <= 9.4. Even if we can do this, we would still need this change for z-stream. This change SHOULD NOT be forwarded into versions later than RHEL 9.4. For those releases nftables userspace should be updated to release 1.0.7 or later instead. 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: 0ebc106 ("netfilter: nf_tables: disallow rule addition to bound chain via NFTA_RULE_CHAIN_ID") Reported-by: Timo Sigurdsson <[email protected]> Closes: https://lore.kernel.org/netfilter-devel/[email protected]/ Signed-off-by: Florian Westphal <[email protected]> Signed-off-by: Florian Westphal <[email protected]>
1 parent c46c169 commit d2fd2e4

File tree

1 file changed

+30
-2
lines changed

1 file changed

+30
-2
lines changed

net/netfilter/nf_tables_api.c

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3526,6 +3526,26 @@ static struct nft_rule *nft_rule_lookup_byid(const struct net *net,
35263526
const struct nft_chain *chain,
35273527
const struct nlattr *nla);
35283528

3529+
/* nft <= 1.0.6 appends rules to anon chains after they have been bound */
3530+
static bool nft_rule_work_around_old_version(const struct nfnl_info *info,
3531+
struct nft_chain *chain)
3532+
{
3533+
/* bound (anonymous) chain is already used */
3534+
if (nft_is_active(info->net, chain))
3535+
return false;
3536+
3537+
/* nft never asks to replace rules here */
3538+
if (info->nlh->nlmsg_flags & (NLM_F_REPLACE | NLM_F_EXCL))
3539+
return false;
3540+
3541+
/* nft and it only ever appends. */
3542+
if ((info->nlh->nlmsg_flags & NLM_F_APPEND) == 0)
3543+
return false;
3544+
3545+
pr_warn_once("enabling workaround for nftables 1.0.6 and older\n");
3546+
return true;
3547+
}
3548+
35293549
#define NFT_RULE_MAXEXPRS 128
35303550

35313551
static int nf_tables_newrule(struct sk_buff *skb, const struct nfnl_info *info,
@@ -3539,6 +3559,7 @@ static int nf_tables_newrule(struct sk_buff *skb, const struct nfnl_info *info,
35393559
struct nft_expr_info *expr_info = NULL;
35403560
u8 family = info->nfmsg->nfgen_family;
35413561
struct nft_flow_rule *flow = NULL;
3562+
bool add_after_bind = false;
35423563
struct net *net = info->net;
35433564
struct nft_userdata *udata;
35443565
struct nft_table *table;
@@ -3578,8 +3599,12 @@ static int nf_tables_newrule(struct sk_buff *skb, const struct nfnl_info *info,
35783599
return -EINVAL;
35793600
}
35803601

3581-
if (nft_chain_is_bound(chain))
3582-
return -EOPNOTSUPP;
3602+
if (nft_chain_is_bound(chain)) {
3603+
if (!nft_rule_work_around_old_version(info, chain))
3604+
return -EOPNOTSUPP;
3605+
3606+
add_after_bind = true;
3607+
}
35833608

35843609
if (nla[NFTA_RULE_HANDLE]) {
35853610
handle = be64_to_cpu(nla_get_be64(nla[NFTA_RULE_HANDLE]));
@@ -3724,6 +3749,9 @@ static int nf_tables_newrule(struct sk_buff *skb, const struct nfnl_info *info,
37243749
goto err_destroy_flow_rule;
37253750
}
37263751

3752+
if (add_after_bind)
3753+
nft_trans_rule_bound(trans) = true;
3754+
37273755
if (info->nlh->nlmsg_flags & NLM_F_APPEND) {
37283756
if (old_rule)
37293757
list_add_rcu(&rule->list, &old_rule->list);

0 commit comments

Comments
 (0)