Re: [PATCH nft v2] datatype: accept a numeric cgroupsv2 id on input
Pablo Neira Ayuso <[email protected]> Fri, 31 Jul 2026 13:53:52 +0200
| Newsgroups | gmane.comp.security.firewalls.netfilter.devel |
|---|---|
| Message-ID | <amyM0DqEImxfc8gK@chamomile> |
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/[email protected]/
> Signed-off-by: Avinash Duduskar <[email protected]>
> ---
> Changes since v1 (https://lore.kernel.org/netfilter-devel/[email protected]/):
> - 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,