* [PATCH nft 1/2] evaluate: reject: remove unused expr function argument
@ 2025-03-31 12:43 Florian Westphal
2025-03-31 12:43 ` [PATCH nft 2/2] evaluate: fix crash when generating reject statement error Florian Westphal
2025-04-01 20:01 ` [PATCH nft 1/2] evaluate: reject: remove unused expr function argument Pablo Neira Ayuso
0 siblings, 2 replies; 4+ messages in thread
From: Florian Westphal @ 2025-03-31 12:43 UTC (permalink / raw)
To: netfilter-devel; +Cc: Florian Westphal
stmt_evaluate_reject passes cmd->expr argument but its never used.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
src/evaluate.c | 26 ++++++++++----------------
1 file changed, 10 insertions(+), 16 deletions(-)
diff --git a/src/evaluate.c b/src/evaluate.c
index 0db3d80f8b56..507b1c86cafc 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -3624,8 +3624,7 @@ static int reject_payload_gen_dependency_family(struct eval_ctx *ctx,
return 1;
}
-static int stmt_reject_gen_dependency(struct eval_ctx *ctx, struct stmt *stmt,
- struct expr *expr)
+static int stmt_reject_gen_dependency(struct eval_ctx *ctx, struct stmt *stmt)
{
struct expr *payload = NULL;
struct stmt *nstmt;
@@ -3705,8 +3704,7 @@ static int stmt_evaluate_reject_inet_family(struct eval_ctx *ctx,
return 0;
}
-static int stmt_evaluate_reject_inet(struct eval_ctx *ctx, struct stmt *stmt,
- struct expr *expr)
+static int stmt_evaluate_reject_inet(struct eval_ctx *ctx, struct stmt *stmt)
{
struct proto_ctx *pctx = eval_proto_ctx(ctx);
const struct proto_desc *desc;
@@ -3717,7 +3715,7 @@ static int stmt_evaluate_reject_inet(struct eval_ctx *ctx, struct stmt *stmt,
return -1;
if (stmt->reject.type == NFT_REJECT_ICMPX_UNREACH)
return 0;
- if (stmt_reject_gen_dependency(ctx, stmt, expr) < 0)
+ if (stmt_reject_gen_dependency(ctx, stmt) < 0)
return -1;
return 0;
}
@@ -3772,8 +3770,7 @@ static int stmt_evaluate_reject_bridge_family(struct eval_ctx *ctx,
return 0;
}
-static int stmt_evaluate_reject_bridge(struct eval_ctx *ctx, struct stmt *stmt,
- struct expr *expr)
+static int stmt_evaluate_reject_bridge(struct eval_ctx *ctx, struct stmt *stmt)
{
struct proto_ctx *pctx = eval_proto_ctx(ctx);
const struct proto_desc *desc;
@@ -3789,13 +3786,12 @@ static int stmt_evaluate_reject_bridge(struct eval_ctx *ctx, struct stmt *stmt,
return -1;
if (stmt->reject.type == NFT_REJECT_ICMPX_UNREACH)
return 0;
- if (stmt_reject_gen_dependency(ctx, stmt, expr) < 0)
+ if (stmt_reject_gen_dependency(ctx, stmt) < 0)
return -1;
return 0;
}
-static int stmt_evaluate_reject_family(struct eval_ctx *ctx, struct stmt *stmt,
- struct expr *expr)
+static int stmt_evaluate_reject_family(struct eval_ctx *ctx, struct stmt *stmt)
{
struct proto_ctx *pctx = eval_proto_ctx(ctx);
@@ -3806,7 +3802,7 @@ static int stmt_evaluate_reject_family(struct eval_ctx *ctx, struct stmt *stmt,
case NFPROTO_IPV6:
switch (stmt->reject.type) {
case NFT_REJECT_TCP_RST:
- if (stmt_reject_gen_dependency(ctx, stmt, expr) < 0)
+ if (stmt_reject_gen_dependency(ctx, stmt) < 0)
return -1;
break;
case NFT_REJECT_ICMPX_UNREACH:
@@ -3821,11 +3817,11 @@ static int stmt_evaluate_reject_family(struct eval_ctx *ctx, struct stmt *stmt,
break;
case NFPROTO_BRIDGE:
case NFPROTO_NETDEV:
- if (stmt_evaluate_reject_bridge(ctx, stmt, expr) < 0)
+ if (stmt_evaluate_reject_bridge(ctx, stmt) < 0)
return -1;
break;
case NFPROTO_INET:
- if (stmt_evaluate_reject_inet(ctx, stmt, expr) < 0)
+ if (stmt_evaluate_reject_inet(ctx, stmt) < 0)
return -1;
break;
}
@@ -3958,8 +3954,6 @@ static int stmt_evaluate_reset(struct eval_ctx *ctx, struct stmt *stmt)
static int stmt_evaluate_reject(struct eval_ctx *ctx, struct stmt *stmt)
{
- struct expr *expr = ctx->cmd->expr;
-
if (stmt->reject.icmp_code < 0) {
if (stmt_evaluate_reject_default(ctx, stmt) < 0)
return -1;
@@ -3971,7 +3965,7 @@ static int stmt_evaluate_reject(struct eval_ctx *ctx, struct stmt *stmt)
return -1;
}
- return stmt_evaluate_reject_family(ctx, stmt, expr);
+ return stmt_evaluate_reject_family(ctx, stmt);
}
static int nat_evaluate_family(struct eval_ctx *ctx, struct stmt *stmt)
--
2.49.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH nft 2/2] evaluate: fix crash when generating reject statement error
2025-03-31 12:43 [PATCH nft 1/2] evaluate: reject: remove unused expr function argument Florian Westphal
@ 2025-03-31 12:43 ` Florian Westphal
2025-04-01 20:04 ` Pablo Neira Ayuso
2025-04-01 20:01 ` [PATCH nft 1/2] evaluate: reject: remove unused expr function argument Pablo Neira Ayuso
1 sibling, 1 reply; 4+ messages in thread
From: Florian Westphal @ 2025-03-31 12:43 UTC (permalink / raw)
To: netfilter-devel; +Cc: Florian Westphal
After patch, this gets rejected with:
internal:0:0-0: Error: conflicting protocols specified: ip vs ip6
Without patch, we crash with a NULL dereference: we cannot use
reject.expr->location unconditionally.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
src/evaluate.c | 16 ++++++++--
.../reject_stmt_with_no_expression_crash | 32 +++++++++++++++++++
2 files changed, 46 insertions(+), 2 deletions(-)
create mode 100644 tests/shell/testcases/bogons/nft-j-f/reject_stmt_with_no_expression_crash
diff --git a/src/evaluate.c b/src/evaluate.c
index 507b1c86cafc..e4a7b5ceaafa 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -3791,6 +3791,18 @@ static int stmt_evaluate_reject_bridge(struct eval_ctx *ctx, struct stmt *stmt)
return 0;
}
+static int stmt_reject_error(struct eval_ctx *ctx,
+ const struct stmt *stmt,
+ const char *msg)
+{
+ struct expr *e = stmt->reject.expr;
+
+ if (e)
+ return stmt_binary_error(ctx, e, stmt, "%s", msg);
+
+ return stmt_error(ctx, stmt, "%s", msg);
+}
+
static int stmt_evaluate_reject_family(struct eval_ctx *ctx, struct stmt *stmt)
{
struct proto_ctx *pctx = eval_proto_ctx(ctx);
@@ -3806,12 +3818,12 @@ static int stmt_evaluate_reject_family(struct eval_ctx *ctx, struct stmt *stmt)
return -1;
break;
case NFT_REJECT_ICMPX_UNREACH:
- return stmt_binary_error(ctx, stmt->reject.expr, stmt,
+ return stmt_reject_error(ctx, stmt,
"abstracted ICMP unreachable not supported");
case NFT_REJECT_ICMP_UNREACH:
if (stmt->reject.family == pctx->family)
break;
- return stmt_binary_error(ctx, stmt->reject.expr, stmt,
+ return stmt_reject_error(ctx, stmt,
"conflicting protocols specified: ip vs ip6");
}
break;
diff --git a/tests/shell/testcases/bogons/nft-j-f/reject_stmt_with_no_expression_crash b/tests/shell/testcases/bogons/nft-j-f/reject_stmt_with_no_expression_crash
new file mode 100644
index 000000000000..04c01aa77a29
--- /dev/null
+++ b/tests/shell/testcases/bogons/nft-j-f/reject_stmt_with_no_expression_crash
@@ -0,0 +1,32 @@
+{
+ "nftables": [
+ {
+ "table": { "family": "ip", "name": "x",
+ "handle": 0
+ }
+ },
+ {
+ "chain": {
+ "family": "ip",
+ "table": "x",
+ "name": "c",
+ "handle": 0
+ }
+ },
+ {
+ "rule": {
+ "family": "ip",
+ "table": "x",
+ "chain": "c",
+ "expr": [
+ {
+ "reject": {
+ "type": "icmpv6",
+ "exprlimit": "port-unreachable"
+ }
+ }
+ ]
+ }
+ }
+ ]
+}
--
2.49.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH nft 1/2] evaluate: reject: remove unused expr function argument
2025-03-31 12:43 [PATCH nft 1/2] evaluate: reject: remove unused expr function argument Florian Westphal
2025-03-31 12:43 ` [PATCH nft 2/2] evaluate: fix crash when generating reject statement error Florian Westphal
@ 2025-04-01 20:01 ` Pablo Neira Ayuso
1 sibling, 0 replies; 4+ messages in thread
From: Pablo Neira Ayuso @ 2025-04-01 20:01 UTC (permalink / raw)
To: Florian Westphal; +Cc: netfilter-devel
On Mon, Mar 31, 2025 at 02:43:33PM +0200, Florian Westphal wrote:
> stmt_evaluate_reject passes cmd->expr argument but its never used.
>
> Signed-off-by: Florian Westphal <fw@strlen.de>
Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
Thanks!
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH nft 2/2] evaluate: fix crash when generating reject statement error
2025-03-31 12:43 ` [PATCH nft 2/2] evaluate: fix crash when generating reject statement error Florian Westphal
@ 2025-04-01 20:04 ` Pablo Neira Ayuso
0 siblings, 0 replies; 4+ messages in thread
From: Pablo Neira Ayuso @ 2025-04-01 20:04 UTC (permalink / raw)
To: Florian Westphal; +Cc: netfilter-devel
On Mon, Mar 31, 2025 at 02:43:34PM +0200, Florian Westphal wrote:
> After patch, this gets rejected with:
> internal:0:0-0: Error: conflicting protocols specified: ip vs ip6
>
> Without patch, we crash with a NULL dereference: we cannot use
> reject.expr->location unconditionally.
>
> Signed-off-by: Florian Westphal <fw@strlen.de>
Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
Thanks!
> ---
> src/evaluate.c | 16 ++++++++--
> .../reject_stmt_with_no_expression_crash | 32 +++++++++++++++++++
> 2 files changed, 46 insertions(+), 2 deletions(-)
> create mode 100644 tests/shell/testcases/bogons/nft-j-f/reject_stmt_with_no_expression_crash
>
> diff --git a/src/evaluate.c b/src/evaluate.c
> index 507b1c86cafc..e4a7b5ceaafa 100644
> --- a/src/evaluate.c
> +++ b/src/evaluate.c
> @@ -3791,6 +3791,18 @@ static int stmt_evaluate_reject_bridge(struct eval_ctx *ctx, struct stmt *stmt)
> return 0;
> }
>
> +static int stmt_reject_error(struct eval_ctx *ctx,
> + const struct stmt *stmt,
> + const char *msg)
> +{
> + struct expr *e = stmt->reject.expr;
> +
> + if (e)
> + return stmt_binary_error(ctx, e, stmt, "%s", msg);
> +
> + return stmt_error(ctx, stmt, "%s", msg);
> +}
> +
> static int stmt_evaluate_reject_family(struct eval_ctx *ctx, struct stmt *stmt)
> {
> struct proto_ctx *pctx = eval_proto_ctx(ctx);
> @@ -3806,12 +3818,12 @@ static int stmt_evaluate_reject_family(struct eval_ctx *ctx, struct stmt *stmt)
> return -1;
> break;
> case NFT_REJECT_ICMPX_UNREACH:
> - return stmt_binary_error(ctx, stmt->reject.expr, stmt,
> + return stmt_reject_error(ctx, stmt,
> "abstracted ICMP unreachable not supported");
> case NFT_REJECT_ICMP_UNREACH:
> if (stmt->reject.family == pctx->family)
> break;
> - return stmt_binary_error(ctx, stmt->reject.expr, stmt,
> + return stmt_reject_error(ctx, stmt,
> "conflicting protocols specified: ip vs ip6");
> }
> break;
> diff --git a/tests/shell/testcases/bogons/nft-j-f/reject_stmt_with_no_expression_crash b/tests/shell/testcases/bogons/nft-j-f/reject_stmt_with_no_expression_crash
> new file mode 100644
> index 000000000000..04c01aa77a29
> --- /dev/null
> +++ b/tests/shell/testcases/bogons/nft-j-f/reject_stmt_with_no_expression_crash
> @@ -0,0 +1,32 @@
> +{
> + "nftables": [
> + {
> + "table": { "family": "ip", "name": "x",
> + "handle": 0
> + }
> + },
> + {
> + "chain": {
> + "family": "ip",
> + "table": "x",
> + "name": "c",
> + "handle": 0
> + }
> + },
> + {
> + "rule": {
> + "family": "ip",
> + "table": "x",
> + "chain": "c",
> + "expr": [
> + {
> + "reject": {
> + "type": "icmpv6",
> + "exprlimit": "port-unreachable"
> + }
> + }
> + ]
> + }
> + }
> + ]
> +}
> --
> 2.49.0
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-04-01 20:04 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-31 12:43 [PATCH nft 1/2] evaluate: reject: remove unused expr function argument Florian Westphal
2025-03-31 12:43 ` [PATCH nft 2/2] evaluate: fix crash when generating reject statement error Florian Westphal
2025-04-01 20:04 ` Pablo Neira Ayuso
2025-04-01 20:01 ` [PATCH nft 1/2] evaluate: reject: remove unused expr function argument Pablo Neira Ayuso
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).