From 48021b277a1ab92480c43e1fa7573b00e33f5212 Mon Sep 17 00:00:00 2001 From: Phil Sutter Date: Fri, 14 Jan 2022 11:39:17 +0100 Subject: [PATCH] evaluate: attempt to set_eval flag if dynamic updates requested Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2039594 Upstream Status: nftables commit 8d443adfcc8c1 Conflicts: * Context change due to missing commit 242965f452e64 ("src: add support for multi-statement in dynamic sets and maps") * Adjusted test-case: Due to missing kernel commit 7b1394892de8d ("netfilter: nft_dynset: relax superfluous check on set updates"), 'update' statement is allowed only if timeout flag is present commit 8d443adfcc8c19effd6be9a9c903ee96e374f2e8 Author: Florian Westphal Date: Tue Jan 11 12:08:59 2022 +0100 evaluate: attempt to set_eval flag if dynamic updates requested When passing no upper size limit, the dynset expression forces an internal 64k upperlimit. In some cases, this can result in 'nft -f' to restore the ruleset. Avoid this by always setting the EVAL flag on a set definition when we encounter packet-path update attempt in the batch. Reported-by: Yi Chen Suggested-by: Pablo Neira Ayuso Signed-off-by: Florian Westphal --- src/evaluate.c | 11 +++++++ .../testcases/sets/dumps/dynset_missing.nft | 12 +++++++ tests/shell/testcases/sets/dynset_missing | 32 +++++++++++++++++++ 3 files changed, 55 insertions(+) create mode 100644 tests/shell/testcases/sets/dumps/dynset_missing.nft create mode 100755 tests/shell/testcases/sets/dynset_missing diff --git a/src/evaluate.c b/src/evaluate.c index 00ec20b..9381f23 100644 --- a/src/evaluate.c +++ b/src/evaluate.c @@ -3076,6 +3076,8 @@ static int stmt_evaluate_log(struct eval_ctx *ctx, struct stmt *stmt) static int stmt_evaluate_set(struct eval_ctx *ctx, struct stmt *stmt) { + struct set *this_set; + expr_set_context(&ctx->ectx, NULL, 0); if (expr_evaluate(ctx, &stmt->set.set) < 0) return -1; @@ -3103,6 +3105,15 @@ static int stmt_evaluate_set(struct eval_ctx *ctx, struct stmt *stmt) "meter statement must be stateful"); } + this_set = stmt->set.set->set; + + /* Make sure EVAL flag is set on set definition so that kernel + * picks a set that allows updates from the packet path. + * + * Alternatively we could error out in case 'flags dynamic' was + * not given, but we can repair this here. + */ + this_set->flags |= NFT_SET_EVAL; return 0; } diff --git a/tests/shell/testcases/sets/dumps/dynset_missing.nft b/tests/shell/testcases/sets/dumps/dynset_missing.nft new file mode 100644 index 0000000..fdb1b97 --- /dev/null +++ b/tests/shell/testcases/sets/dumps/dynset_missing.nft @@ -0,0 +1,12 @@ +table ip test { + set dlist { + type ipv4_addr + size 65535 + flags dynamic,timeout + } + + chain output { + type filter hook output priority filter; policy accept; + udp dport 1234 update @dlist { ip daddr } counter packets 0 bytes 0 + } +} diff --git a/tests/shell/testcases/sets/dynset_missing b/tests/shell/testcases/sets/dynset_missing new file mode 100755 index 0000000..89afcd5 --- /dev/null +++ b/tests/shell/testcases/sets/dynset_missing @@ -0,0 +1,32 @@ +#!/bin/bash + +set -e + +$NFT -f /dev/stdin < $tmpfile + +# this restore works, because set is still the rhash backend. +$NFT -f $tmpfile # success +$NFT flush ruleset + +# fails without commit 'attempt to set_eval flag if dynamic updates requested', +# because set in $tmpfile has 'size x' but no 'flags dynamic'. +$NFT -f $tmpfile -- 2.31.1