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

[Avinash Duduskar <avinash.duduskar@gmail.com>]

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 v2 (https://lore.kernel.org/netfilter-devel/20260729173748.1033454-1-avinash.duduskar@gmail.com/):
- return the error from integer_type_parse() instead of discarding it (Pablo)

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                                |   4 +-
 .../shell/testcases/parsing/cgroupv2_stale_id | 128 ++++++++++++++++++
 .../parsing/dumps/cgroupv2_stale_id.json-nft  |  11 ++
 .../parsing/dumps/cgroupv2_stale_id.nft       |   0
 4 files changed, 141 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..5025b276 100644
--- a/src/datatype.c
+++ b/src/datatype.c
@@ -1665,9 +1665,9 @@ static struct error_record *cgroupv2_type_parse(struct parse_ctx *ctx,
 		 SYSFS_CGROUPSV2_PATH, sym->identifier);
 	cgroupv2_path[sizeof(cgroupv2_path) - 1] = '\0';
 
+	/* 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);
 
 	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..7f1833a8
--- /dev/null
+++ b/tests/shell/testcases/parsing/cgroupv2_stale_id
@@ -0,0 +1,128 @@
+#!/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.
+
+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
+
+# a name that is neither a path nor an integer 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