rpms/nftables
7
0
Fork 0
Code Issues Pull Requests Releases Activity
c9
nftables/SOURCES/0093-evaluate-bogus-protocol-conflicts-in-vlan-with-impli.patch
AlmaLinux RelEng Bot 0ea6bbe225 import CS git nftables-1.0.9-9.el9_8
2026-08-26 08:03:54 -04:00

228 lines
7.8 KiB
Diff
Raw Permalink Blame History

From da2b7b622c703aafa4fb42442b5b74da9828f842 Mon Sep 17 00:00:00 2001
From: Phil Sutter <psutter@redhat.com>
Date: Fri, 17 Jul 2026 11:09:25 +0200
Subject: [PATCH] evaluate: bogus protocol conflicts in vlan with implicit
dependencies
JIRA: https://issues.redhat.com/browse/RHEL-190549
Upstream Status: nftables commit d1a7e74d1e065d244439fdb0f1c1cba83f921609
commit d1a7e74d1e065d244439fdb0f1c1cba83f921609
Author: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Wed May 15 19:23:49 2024 +0200
evaluate: bogus protocol conflicts in vlan with implicit dependencies
The following command:
# nft add rule netdev x y ip saddr 10.1.1.1 icmp type echo-request vlan id set 321
fails with:
Error: conflicting link layer protocols specified: ether vs. vlan
netdev x y ip saddr 10.1.1.1 icmp type echo-request vlan id set 321
^^^^^^^
Users can work around this issue by prepending an explicit match for
vlan ethertype field, that is:
ether type vlan ip saddr 10.1.1.1 ...
^-------------^
but nft should really handle this itself.
The error above is triggered by the following check in
resolve_ll_protocol_conflict():
/* This payload and the existing context don't match, conflict. */
if (pctx->protocol[base + 1].desc != NULL)
return 1;
This check was added by 39f15c243912 ("nft: support listing expressions
that use non-byte header fields") and f7d5590688a6 ("tests: vlan tests")
to deal with conflicting link layer protocols, for instance:
ether type ip vlan id 1
this is matching ethertype ip at offset 12, but then it matches for vlan
id at offset 14 which is not present given the previous check.
One possibility is to remove such check, but nft does not bail out for
the example above and it results in bytecode that never matches:
# nft --debug=netlink netdev x y ether type ip vlan id 10
netdev x y
[ meta load iiftype => reg 1 ]
[ cmp eq reg 1 0x00000001 ]
[ payload load 2b @ link header + 12 => reg 1 ] <---- ether type
[ cmp eq reg 1 0x00000008 ] <---- ip
[ payload load 2b @ link header + 12 => reg 1 ] <---- ether type
[ cmp eq reg 1 0x00000081 ] <---- vlan
[ payload load 2b @ link header + 14 => reg 1 ]
[ bitwise reg 1 = ( reg 1 & 0x0000ff0f ) ^ 0x00000000 ]
[ cmp eq reg 1 0x00000a00 ]
This is due to resolve_ll_protocol_conflict() which deals with the
conflict by updating protocol context and emitting an implicit
dependency, but there is already an explicit match coming from the user.
This patch adds a new helper function to check if an implicit dependency
clashes with an existing statement, which results in:
# nft add rule netdev x y ether type ip vlan id 1
Error: conflicting statements
add rule netdev x y ether type ip vlan id 1
^^^^^^^^^^^^^ ~~~~~~~
Theoretically, no duplicated implicit dependency should ever be emitted
if protocol context is correctly handled.
Only implicit payload expressions are considered at this stage for this
conflict check, this patch can be extended to deal with other dependency
types.
Fixes: 39f15c243912 ("nft: support listing expressions that use non-byte header fields")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Signed-off-by: Phil Sutter <psutter@redhat.com>
---
src/evaluate.c | 69 +++++++++++++++++++++++++++++++++++++++++---------
1 file changed, 57 insertions(+), 12 deletions(-)
diff --git a/src/evaluate.c b/src/evaluate.c
index 38a80f2..d8e5dfd 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -759,6 +759,46 @@ static bool proto_is_dummy(const struct proto_desc *desc)
return desc == &proto_inet || desc == &proto_netdev;
}
+static int stmt_dep_conflict(struct eval_ctx *ctx, const struct stmt *nstmt)
+{
+ struct stmt *stmt;
+
+ list_for_each_entry(stmt, &ctx->rule->stmts, list) {
+ if (stmt == nstmt)
+ break;
+
+ if (stmt->ops->type != STMT_EXPRESSION ||
+ stmt->expr->etype != EXPR_RELATIONAL ||
+ stmt->expr->right->etype != EXPR_VALUE ||
+ stmt->expr->left->etype != EXPR_PAYLOAD ||
+ stmt->expr->left->etype != nstmt->expr->left->etype ||
+ stmt->expr->left->len != nstmt->expr->left->len)
+ continue;
+
+ if (stmt->expr->left->payload.desc != nstmt->expr->left->payload.desc ||
+ stmt->expr->left->payload.inner_desc != nstmt->expr->left->payload.inner_desc ||
+ stmt->expr->left->payload.base != nstmt->expr->left->payload.base ||
+ stmt->expr->left->payload.offset != nstmt->expr->left->payload.offset)
+ continue;
+
+ return stmt_binary_error(ctx, stmt, nstmt,
+ "conflicting statements");
+ }
+
+ return 0;
+}
+
+static int rule_stmt_dep_add(struct eval_ctx *ctx,
+ struct stmt *nstmt, struct stmt *stmt)
+{
+ rule_stmt_insert_at(ctx->rule, nstmt, ctx->stmt);
+
+ if (stmt_dep_conflict(ctx, nstmt) < 0)
+ return -1;
+
+ return 0;
+}
+
static int resolve_ll_protocol_conflict(struct eval_ctx *ctx,
const struct proto_desc *desc,
struct expr *payload)
@@ -782,7 +822,8 @@ static int resolve_ll_protocol_conflict(struct eval_ctx *ctx,
return err;
desc = payload->payload.desc;
- rule_stmt_insert_at(ctx->rule, nstmt, ctx->stmt);
+ if (rule_stmt_dep_add(ctx, nstmt, ctx->stmt) < 0)
+ return -1;
}
} else {
unsigned int i;
@@ -794,10 +835,6 @@ static int resolve_ll_protocol_conflict(struct eval_ctx *ctx,
}
}
- /* This payload and the existing context don't match, conflict. */
- if (pctx->protocol[base + 1].desc != NULL)
- return 1;
-
link = proto_find_num(desc, payload->payload.desc);
if (link < 0 ||
ll_conflict_resolution_gen_dependency(ctx, link, payload, &nstmt) < 0)
@@ -806,7 +843,8 @@ static int resolve_ll_protocol_conflict(struct eval_ctx *ctx,
for (i = 0; i < pctx->stacked_ll_count; i++)
payload->payload.offset += pctx->stacked_ll[i]->length;
- rule_stmt_insert_at(ctx->rule, nstmt, ctx->stmt);
+ if (rule_stmt_dep_add(ctx, nstmt, ctx->stmt) < 0)
+ return -1;
return 0;
}
@@ -834,7 +872,8 @@ static int __expr_evaluate_payload(struct eval_ctx *ctx, struct expr *expr)
if (payload_gen_dependency(ctx, payload, &nstmt) < 0)
return -1;
- rule_stmt_insert_at(ctx->rule, nstmt, ctx->stmt);
+ if (rule_stmt_dep_add(ctx, nstmt, ctx->stmt) < 0)
+ return -1;
desc = pctx->protocol[base].desc;
@@ -854,7 +893,10 @@ static int __expr_evaluate_payload(struct eval_ctx *ctx, struct expr *expr)
assert(pctx->stacked_ll_count);
payload->payload.offset += pctx->stacked_ll[0]->length;
- rule_stmt_insert_at(ctx->rule, nstmt, ctx->stmt);
+
+ if (rule_stmt_dep_add(ctx, nstmt, ctx->stmt) < 0)
+ return -1;
+
return 1;
}
goto check_icmp;
@@ -895,8 +937,8 @@ check_icmp:
if (payload_gen_icmp_dependency(ctx, expr, &nstmt) < 0)
return -1;
- if (nstmt)
- rule_stmt_insert_at(ctx->rule, nstmt, ctx->stmt);
+ if (nstmt && rule_stmt_dep_add(ctx, nstmt, ctx->stmt) < 0)
+ return -1;
return 0;
}
@@ -968,7 +1010,8 @@ static int expr_evaluate_inner(struct eval_ctx *ctx, struct expr **exprp)
if (payload_gen_inner_dependency(ctx, expr, &nstmt) < 0)
return -1;
- rule_stmt_insert_at(ctx->rule, nstmt, ctx->stmt);
+ if (rule_stmt_dep_add(ctx, nstmt, ctx->stmt) < 0)
+ return -1;
proto_ctx_update(pctx, PROTO_BASE_TRANSPORT_HDR, &expr->location, expr->payload.inner_desc);
}
@@ -1099,7 +1142,9 @@ static int ct_gen_nh_dependency(struct eval_ctx *ctx, struct expr *ct)
relational_expr_pctx_update(pctx, dep);
nstmt = expr_stmt_alloc(&dep->location, dep);
- rule_stmt_insert_at(ctx->rule, nstmt, ctx->stmt);
+
+ if (rule_stmt_dep_add(ctx, nstmt, ctx->stmt) < 0)
+ return -1;
return 0;
}
Reference in New Issue View Git Blame Copy Permalink