linux-kernel - Re: [PATCH 3/4] cgroup: Use __counted

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <87cc0370-1924-4d33-bbf1-7fc2b03149e3@huaweicloud.com>
Date: Thu, 18 Dec 2025 15:09:32 +0800
From: Chen Ridong <chenridong@...weicloud.com>
To: Michal Koutný <mkoutny@...e.com>,
 cgroups@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-hardening@...r.kernel.org
Cc: "Gustavo A. R. Silva" <gustavo@...eddedor.com>, Tejun Heo
 <tj@...nel.org>, Johannes Weiner <hannes@...xchg.org>,
 Kees Cook <kees@...nel.org>, "Gustavo A. R. Silva" <gustavoars@...nel.org>
Subject: Re: [PATCH 3/4] cgroup: Use __counted_by for cgroup::ancestors



On 2025/12/18 0:27, Michal Koutný wrote:
> cgroup::ancestors includes self, i.e. root cgroups have one ancestor but
> their level is 0. Change the value that we store inside struct cgroup
> and use an inlined helper where we need to know the level. This way we
> preserve the concept of 0-based levels and we can utilize __counted_by
> constraint to guard ancestors access. (We could've used level value as a
> counter for _low_ancestors but that would have no benefit since we never
> access data through this flexible array alias.)
> 
> Cc: "Gustavo A. R. Silva" <gustavo@...eddedor.com>
> Signed-off-by: Michal Koutný <mkoutny@...e.com>
> ---
>  include/linux/cgroup-defs.h | 19 ++++++++-----------
>  include/linux/cgroup.h      |  2 +-
>  kernel/cgroup/cgroup.c      |  3 ++-
>  3 files changed, 11 insertions(+), 13 deletions(-)
> 
> diff --git a/include/linux/cgroup-defs.h b/include/linux/cgroup-defs.h
> index 9247e437da5ce..8ce1ae9bea909 100644
> --- a/include/linux/cgroup-defs.h
> +++ b/include/linux/cgroup-defs.h
> @@ -475,14 +475,6 @@ struct cgroup {
>  
>  	unsigned long flags;		/* "unsigned long" so bitops work */
>  
> -	/*
> -	 * The depth this cgroup is at.  The root is at depth zero and each
> -	 * step down the hierarchy increments the level.  This along with
> -	 * ancestors[] can determine whether a given cgroup is a
> -	 * descendant of another without traversing the hierarchy.
> -	 */
> -	int level;
> -

Note that this level may already be used in existing BPF programs (e.g.,
tools/testing/selftests/bpf/progs/task_ls_uptr.c). Do we need to consider compatibility here?

>  	/* Maximum allowed descent tree depth */
>  	int max_depth;
>  
> @@ -625,13 +617,18 @@ struct cgroup {
>  	struct bpf_local_storage __rcu  *bpf_cgrp_storage;
>  #endif
>  
> -	/* All ancestors including self */
>  	union {
>  		struct {
> -			void *_sentinel[0]; /* XXX to avoid 'flexible array member in a struct with no named members' */
> -			struct cgroup *ancestors[];
> +			int nr_ancestors;	/* do not use directly but via cgroup_level() */
> +			/*
> +			 * All ancestors including self.
> +			 * ancestors[] can determine whether a given cgroup is a
> +			 * descendant of another without traversing the hierarchy.
> +			 */
> +			struct cgroup *ancestors[] __counted_by(nr_ancestors);
>  		};
>  		struct {
> +			int _nr_ancestors;	/* auxiliary padding, see nr_ancestors above */
>  			struct cgroup *_root_ancestor;
>  			struct cgroup *_low_ancestors[];
>  		};
> diff --git a/include/linux/cgroup.h b/include/linux/cgroup.h
> index 0290878ebad26..45f720b9ecedd 100644
> --- a/include/linux/cgroup.h
> +++ b/include/linux/cgroup.h
> @@ -534,7 +534,7 @@ static inline struct cgroup *cgroup_parent(struct cgroup *cgrp)
>   */
>  static inline int cgroup_level(struct cgroup *cgrp)
>  {
> -	return cgrp->level;
> +	return cgrp->nr_ancestors - 1;
>  }
>  
>  /**
> diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
> index e011f1dd6d87f..5110d3e13d125 100644
> --- a/kernel/cgroup/cgroup.c
> +++ b/kernel/cgroup/cgroup.c
> @@ -2197,6 +2197,7 @@ int cgroup_setup_root(struct cgroup_root *root, u16 ss_mask)
>  	}
>  	root_cgrp->kn = kernfs_root_to_node(root->kf_root);
>  	WARN_ON_ONCE(cgroup_ino(root_cgrp) != 1);
> +	root_cgrp->nr_ancestors = 1; /* stored in _root_ancestor */
>  	root_cgrp->ancestors[0] = root_cgrp;
>  
>  	ret = css_populate_dir(&root_cgrp->self);
> @@ -5869,7 +5870,7 @@ static struct cgroup *cgroup_create(struct cgroup *parent, const char *name,
>  
>  	cgrp->self.parent = &parent->self;
>  	cgrp->root = root;
> -	cgrp->level = level;
> +	cgrp->nr_ancestors = parent->nr_ancestors + 1;
>  
>  	/*
>  	 * Now that init_cgroup_housekeeping() has been called and cgrp->self

-- 
Best regards,
Ridong