[PATCH nft v2] datatype: accept a numeric cgroupsv2 id on input

[PATCH nft v2] datatype: accept a numeric cgroupsv2 id on input

[Avinash Duduskar]

nft prints non-existent cgroup names as the raw id using PRIu64, but
cgroupv2_type_parse() only stats /sys/fs/cgroup/<identifier>, so the
listing does not load back:

  # nft list set ip t s
  table ip t {
          set s {
                  type cgroupsv2
                  elements = { 50834 }
          }
  }
  # nft delete element ip t s { 50834 }
  Error: cgroupv2 path fails: No such file or directory

The element cannot be deleted by key once its cgroup is gone, only
flushed with the set, and a dump does not restore. The json dump has
the same problem, and the stale id has been visible in the wild since
2022 (see Link). tests/shell/testcases/packetpath/cgroupv2 already
works around this in cleanup().

Fall back to integer_type_parse() when the path does not resolve, as
boolean_type_parse() already does. The path lookup stays first, so a
cgroup named as a number still resolves as a path. Improving the
integer parser is left for a follow-up as discussed in v1.

Fixes: 38228087252c ("src: add cgroupsv2 support")
Link: https://lore.kernel.org/netfilter-devel/fabde324-383a-622c-7e69-32c9b2d06191@gmail.com/
Signed-off-by: Avinash Duduskar <avinash.duduskar@gmail.com>
---
Changes since v1 (https://lore.kernel.org/netfilter-devel/20260727082427.740789-1-avinash.duduskar@gmail.com/):
- drop the strict decimal parser, delegate to integer_type_parse()
  (Pablo, Phil)
- json test arm uses a here-string instead of a tempfile (Phil)
- shorter commit message and test

 src/datatype.c                                |  13 +-
 .../shell/testcases/parsing/cgroupv2_stale_id | 129 ++++++++++++++++++
 .../parsing/dumps/cgroupv2_stale_id.json-nft  |  11 ++
 .../parsing/dumps/cgroupv2_stale_id.nft       |   0
 4 files changed, 151 insertions(+), 2 deletions(-)
 create mode 100755 tests/shell/testcases/parsing/cgroupv2_stale_id
 create mode 100644 tests/shell/testcases/parsing/dumps/cgroupv2_stale_id.json-nft
 create mode 100644 tests/shell/testcases/parsing/dumps/cgroupv2_stale_id.nft

diff --git a/src/datatype.c b/src/datatype.c
index 4dbca16e..5b3b350c 100644
--- a/src/datatype.c
+++ b/src/datatype.c
@@ -1665,9 +1665,18 @@ static struct error_record *cgroupv2_type_parse(struct parse_ctx *ctx,
 		 SYSFS_CGROUPSV2_PATH, sym->identifier);
 	cgroupv2_path[sizeof(cgroupv2_path) - 1] = '\0';
 
-	if (stat(cgroupv2_path, &st) < 0)
+	if (stat(cgroupv2_path, &st) < 0) {
+		struct error_record *erec;
+		int stat_errno = errno;
+
+		/* the listing prints a raw id once the path is gone */
+		erec = integer_type_parse(ctx, sym, res);
+		if (!erec)
+			return NULL;
+		erec_destroy(erec);
 		return error(&sym->location, "cgroupv2 path fails: %s",
-			     strerror(errno));
+			     strerror(stat_errno));
+	}
 
 	ino = st.st_ino;
 	*res = constant_expr_alloc(&sym->location, &cgroupv2_type,
diff --git a/tests/shell/testcases/parsing/cgroupv2_stale_id b/tests/shell/testcases/parsing/cgroupv2_stale_id
new file mode 100755
index 00000000..9e77e2b1
--- /dev/null
+++ b/tests/shell/testcases/parsing/cgroupv2_stale_id
@@ -0,0 +1,129 @@
+#!/bin/bash
+
+# A cgroupsv2 element has to stay addressable once its cgroup is gone: the
+# listing prints the raw id, so that id has to be accepted back, and nothing
+# else has to be.
+
+CGROUP="/sys/fs/cgroup/nft-stale-$$"
+# numeric on purpose: a cgroup named as a number must still resolve as a path
+CGNUM="/sys/fs/cgroup/$$"
+
+cleanup()
+{
+	$NFT delete table t 2>/dev/null
+	rmdir "$CGROUP" 2>/dev/null
+	rmdir "$CGNUM" 2>/dev/null
+}
+trap cleanup EXIT
+
+if [ ! -w /sys/fs/cgroup ]; then
+	echo "cgroup filesystem not writable"
+	exit 77
+fi
+
+# -w alone passes on a v1 or hybrid layout, where everything below tests nothing
+if [ "$(stat -f --printf=%T /sys/fs/cgroup)" != "cgroup2fs" ]; then
+	echo "not a cgroupv2 mount"
+	exit 77
+fi
+
+if ! mkdir "$CGROUP" 2>/dev/null; then
+	# unprivileged and left over from a dead run are different facts
+	[ -d "$CGROUP" ] && {
+		echo "E: $CGROUP already exists" >&2
+		exit 1
+	}
+	echo "cannot create a cgroup"
+	exit 77
+fi
+name="${CGROUP##*/}"
+id=$(stat --printf=%i "$CGROUP")
+
+$NFT add table t || exit 1
+$NFT add set t s '{ type cgroupsv2; }' || exit 1
+$NFT add element t s "{ \"$name\" }" || exit 1
+
+rmdir "$CGROUP" || exit 1
+
+# Another cgroup can take this inode between the rmdir and the listing, which
+# resolves it back to a name. Narrow, and not closable from here.
+$NFT list set t s | grep -qw "$id" || {
+	echo "E: listing does not show the stale cgroup id $id" >&2
+	$NFT list set t s >&2
+	exit 1
+}
+
+$NFT delete element t s "{ $id }" || {
+	echo "E: cannot delete a cgroupsv2 element by the id that was listed" >&2
+	exit 1
+}
+
+$NFT list set t s | grep -qw "$id" && {
+	echo "E: element still present after delete" >&2
+	exit 1
+}
+
+$NFT add element t s "{ $id }" || exit 1
+
+# json serialises the id as a string, so it reaches the same parser. The harness
+# round trip at exit sees an empty ruleset here, so do it inline, scoped to this
+# table.
+if [ "$NFT_TEST_HAVE_json" != n ] && $NFT -j list tables >/dev/null 2>&1; then
+	out=$($NFT -j list table t) || exit 1
+	$NFT delete table t || exit 1
+	$NFT -j -f - <<< "$out" || {
+		echo "E: a json dump holding a stale cgroupsv2 id does not reload" >&2
+		exit 1
+	}
+	$NFT list set t s | grep -qw "$id" || {
+		echo "E: the id did not survive the json round trip" >&2
+		exit 1
+	}
+else
+	echo "I: no json support, skipping the json reload check"
+fi
+
+$NFT flush set t s || exit 1
+
+# junk that is neither a path nor a number must still fail, and a value wider
+# than the 64-bit key must be rejected by evaluation
+for bogus in "nft-does-not-exist" "12abc" "18446744073709551616"; do
+	$NFT add element t s "{ \"$bogus\" }" 2>/dev/null && {
+		echo "E: accepted \"$bogus\" as a cgroupsv2 id" >&2
+		$NFT list set t s >&2
+		exit 1
+	}
+done
+
+# each add above also "passes" if it fails for an unrelated reason, so check
+# nothing was stored, without a pipeline that goes vacuous when list fails
+out=$($NFT list set t s) || exit 1
+case "$out" in
+*elements*)
+	echo "E: something was stored despite every add failing" >&2
+	echo "$out" >&2
+	exit 1
+	;;
+esac
+
+# The path has to win for a cgroup named as a number. While the directory
+# exists both readings print the same, so remove it before asserting.
+mkdir "$CGNUM" || exit 1
+numino=$(stat --printf=%i "$CGNUM")
+$NFT add element t s "{ \"$$\" }" || {
+	echo "E: cannot add a cgroup whose name is a number" >&2
+	exit 1
+}
+rmdir "$CGNUM" || exit 1
+if [ "$numino" = "$$" ]; then
+	echo "I: cgroup $$ happens to have inode $$, cannot tell the two apart"
+else
+	$NFT list set t s | grep -qw "$numino" || {
+		echo "E: \"$$\" was taken as an id, not resolved as a path" >&2
+		$NFT list set t s >&2
+		exit 1
+	}
+fi
+$NFT flush set t s || exit 1
+
+exit 0
diff --git a/tests/shell/testcases/parsing/dumps/cgroupv2_stale_id.json-nft b/tests/shell/testcases/parsing/dumps/cgroupv2_stale_id.json-nft
new file mode 100644
index 00000000..546cc597
--- /dev/null
+++ b/tests/shell/testcases/parsing/dumps/cgroupv2_stale_id.json-nft
@@ -0,0 +1,11 @@
+{
+  "nftables": [
+    {
+      "metainfo": {
+        "version": "VERSION",
+        "release_name": "RELEASE_NAME",
+        "json_schema_version": 1
+      }
+    }
+  ]
+}
diff --git a/tests/shell/testcases/parsing/dumps/cgroupv2_stale_id.nft b/tests/shell/testcases/parsing/dumps/cgroupv2_stale_id.nft
new file mode 100644
index 00000000..e69de29b

base-commit: 31fb7af1a99d67c87e94b4415e553589b21db2d1
-- 
2.55.0

Re: [PATCH nft v2] datatype: accept a numeric cgroupsv2 id on input

[Pablo Neira Ayuso]

On Wed, Jul 29, 2026 at 11:07:48PM +0530, Avinash Duduskar wrote:
> nft prints non-existent cgroup names as the raw id using PRIu64, but
> cgroupv2_type_parse() only stats /sys/fs/cgroup/<identifier>, so the
> listing does not load back:
> 
>   # nft list set ip t s
>   table ip t {
>           set s {
>                   type cgroupsv2
>                   elements = { 50834 }
>           }
>   }
>   # nft delete element ip t s { 50834 }
>   Error: cgroupv2 path fails: No such file or directory
> 
> The element cannot be deleted by key once its cgroup is gone, only
> flushed with the set, and a dump does not restore. The json dump has
> the same problem, and the stale id has been visible in the wild since
> 2022 (see Link). tests/shell/testcases/packetpath/cgroupv2 already
> works around this in cleanup().
> 
> Fall back to integer_type_parse() when the path does not resolve, as
> boolean_type_parse() already does. The path lookup stays first, so a
> cgroup named as a number still resolves as a path. Improving the
> integer parser is left for a follow-up as discussed in v1.
> 
> Fixes: 38228087252c ("src: add cgroupsv2 support")
> Link: https://lore.kernel.org/netfilter-devel/fabde324-383a-622c-7e69-32c9b2d06191@gmail.com/
> Signed-off-by: Avinash Duduskar <avinash.duduskar@gmail.com>
> ---
> Changes since v1 (https://lore.kernel.org/netfilter-devel/20260727082427.740789-1-avinash.duduskar@gmail.com/):
> - drop the strict decimal parser, delegate to integer_type_parse()
>   (Pablo, Phil)
> - json test arm uses a here-string instead of a tempfile (Phil)
> - shorter commit message and test
> 
>  src/datatype.c                                |  13 +-
>  .../shell/testcases/parsing/cgroupv2_stale_id | 129 ++++++++++++++++++
>  .../parsing/dumps/cgroupv2_stale_id.json-nft  |  11 ++
>  .../parsing/dumps/cgroupv2_stale_id.nft       |   0
>  4 files changed, 151 insertions(+), 2 deletions(-)
>  create mode 100755 tests/shell/testcases/parsing/cgroupv2_stale_id
>  create mode 100644 tests/shell/testcases/parsing/dumps/cgroupv2_stale_id.json-nft
>  create mode 100644 tests/shell/testcases/parsing/dumps/cgroupv2_stale_id.nft
> 
> diff --git a/src/datatype.c b/src/datatype.c
> index 4dbca16e..5b3b350c 100644
> --- a/src/datatype.c
> +++ b/src/datatype.c
> @@ -1665,9 +1665,18 @@ static struct error_record *cgroupv2_type_parse(struct parse_ctx *ctx,
>  		 SYSFS_CGROUPSV2_PATH, sym->identifier);
>  	cgroupv2_path[sizeof(cgroupv2_path) - 1] = '\0';
>  
> -	if (stat(cgroupv2_path, &st) < 0)
> +	if (stat(cgroupv2_path, &st) < 0) {
> +		struct error_record *erec;
> +		int stat_errno = errno;
> +
> +		/* the listing prints a raw id once the path is gone */
> +		erec = integer_type_parse(ctx, sym, res);
> +		if (!erec)
> +			return NULL;
> +		erec_destroy(erec);

Any reason to ignore the error that integer_type_parse() provides?
Instead I would suggest:

                if (erec)
                        return erec;

Thanks.

>  		return error(&sym->location, "cgroupv2 path fails: %s",
> -			     strerror(errno));
> +			     strerror(stat_errno));
> +	}
>  
>  	ino = st.st_ino;
>  	*res = constant_expr_alloc(&sym->location, &cgroupv2_type,

Re: [PATCH nft v2] datatype: accept a numeric cgroupsv2 id on input

[Avinash Duduskar]

On Fri, Jul 31, 2026 at 01:53:52PM +0200, Pablo Neira Ayuso wrote:
> Any reason to ignore the error that integer_type_parse() provides?

No, it was left over from the strict parser that owned its own message.
Nothing follows the call, so it reduces to a direct return and the hunk
goes from ten added lines to two:

+	/* the listing prints a raw id once the path is gone */
 	if (stat(cgroupv2_path, &st) < 0)
-		return error(&sym->location, "cgroupv2 path fails: %s",
-			     strerror(errno));
+		return integer_type_parse(ctx, sym, res);

v3 to follow.

Avi