On Wed, Mar 05, 2025 at 04:01:48PM +0100, Florian Westphal wrote:
> This uses the wrong length. This must re-use the length of the datatype,
> not the string length.
>
> The added test cases will fail without the fix due to erroneous
> overlap detection, which in itself is due to incorrect sorting of
> the elements.
>
> Example error:
> netlink: Error: interval overlaps with an existing one
> add element inet testifsets simple_wild { "2-1" } failed.
> table inet testifsets {
> ... elements = { "1-1", "abcdef*", "othername", "ppp0" }
>
> ... but clearly "2-1" doesn't overlap with any existing members.
> The false detection is because of the "acvdef*" wildcard getting sorted
> at the beginning of the list which is because its erronously initialised
> as a 64bit number instead of 128 bits (16 bytes / IFNAMSIZ).
One question here.
> Fixes: 5e393ea1fc0a ("segtree: add string "range" reversal support")
> Signed-off-by: Florian Westphal <fw@xxxxxxxxx>
> ---
> src/segtree.c | 2 +-
> tests/shell/testcases/sets/sets_with_ifnames | 62 ++++++++++++++++++++
> 2 files changed, 63 insertions(+), 1 deletion(-)
>
> diff --git a/src/segtree.c b/src/segtree.c
> index 2e32a3291979..11cf27c55dcb 100644
> --- a/src/segtree.c
> +++ b/src/segtree.c
> @@ -471,7 +471,7 @@ static struct expr *interval_to_string(struct expr *low, struct expr *i, const m
>
> expr = constant_expr_alloc(&low->location, low->dtype,
> BYTEORDER_HOST_ENDIAN,
> - (str_len + 1) * BITS_PER_BYTE, data);
> + len * BITS_PER_BYTE, data);
BTW, is this also needed?
diff --git a/src/segtree.c b/src/segtree.c
index 2e32a3291979..b7a89383fae0 100644
--- a/src/segtree.c
+++ b/src/segtree.c
@@ -453,7 +453,7 @@ static struct expr *interval_to_string(struct expr *low, struct expr *i, const m
{
unsigned int len = div_round_up(i->len, BITS_PER_BYTE);
unsigned int prefix_len, str_len;
- char data[len + 2];
+ char data[len + 2] = {};
struct expr *expr;
prefix_len = expr_value(i)->len - mpz_scan0(range, 0);
otherwise uninitialized data could be send to the kernel?
> return __expr_to_set_elem(low, expr);
> }
> diff --git a/tests/shell/testcases/sets/sets_with_ifnames b/tests/shell/testcases/sets/sets_with_ifnames
> index a4bc5072938e..c65499b76bc5 100755
> --- a/tests/shell/testcases/sets/sets_with_ifnames
> +++ b/tests/shell/testcases/sets/sets_with_ifnames
> @@ -105,10 +105,67 @@ check_matching_icmp_ppp()
> fi
> }
>
> +check_add_del_ifnames()
> +{
> + local what="$1"
> + local setname="$2"
> + local prefix="$3"
> + local data="$4"
> + local i=0
> +
> + for i in $(seq 1 5);do
> + local cmd="element inet testifsets $setname { "
> + local to_batch=16
> +
> + for j in $(seq 1 $to_batch);do
> + local name=$(printf '"%x-%d"' $i $j)
> +
> + [ -n "$prefix" ] && cmd="$cmd $prefix . "
> +
> + cmd="$cmd $name"
> +
> + [ -n "$data" ] && cmd="$cmd : $data"
> +
> + if [ $j -lt $to_batch ] ; then
> + cmd="$cmd, "
> + fi
> + done
> +
> + cmd="$cmd }"
> +
> + if ! $NFT "$what" "$cmd"; then
> + echo "$what $cmd failed."
> + $NFT list set inet testifsets $setname
> + exit 1
> + fi
> +
> + if ! ip netns exec "$ns1" $NFT "$what" "$cmd"; then
> + echo "$ns1 $what $cmd failed."
> + ip netns exec "$ns1" $NFT list set inet testifsets $setname
> + exit 1
> + fi
> + done
> +}
> +
> +check_add_ifnames()
> +{
> + check_add_del_ifnames "add" "$1" "$2" "$3"
> +}
> +
> +check_del_ifnames()
> +{
> + check_add_del_ifnames "delete" "$1" "$2" "$3"
> +}
> +
> ip netns add "$ns1" || exit 111
> ip netns add "$ns2" || exit 111
> ip netns exec "$ns1" $NFT -f "$dumpfile" || exit 3
>
> +check_add_ifnames "simple" "" ""
> +check_add_ifnames "simple_wild" "" ""
> +check_add_ifnames "concat" "10.1.2.2" ""
> +check_add_ifnames "map_wild" "" "drop"
> +
> for n in abcdef0 abcdef1 othername;do
> check_elem simple $n
> done
> @@ -150,3 +207,8 @@ ip -net "$ns2" addr add 10.1.2.2/24 dev veth0
> ip -net "$ns2" addr add 10.2.2.2/24 dev veth1
>
> check_matching_icmp_ppp
> +
> +check_del_ifnames "simple" "" ""
> +check_del_ifnames "simple_wild" "" ""
> +check_del_ifnames "concat" "10.1.2.2" ""
> +check_del_ifnames "map_wild" "" "drop"
> --
> 2.48.1
>
>
