Re: [PATCH v2 1/5] userns: record how far up CAP_SETFCAP reaches

["Serge E. Hallyn" <serge@hallyn.com>]

On Wed, Oct 14, 2026 at 08:47:46PM +0000, Josef Bacik wrote:
> create_user_ns() records in ns->parent_could_setfcap whether the creator
> had CAP_SETFCAP in its effective set, and verify_root_map() trusts that
> when a task in the new namespace maps the parent's uid 0.  A bool only
> describes the parent, though.  File capabilities belong to the kuid of a
> root user, and a namespace that maps the parent's uid 0 shares that kuid
> with the parent and possibly with ancestors further up, so what matters
> is how far up the creator's CAP_SETFCAP counted.
> 
> Replace the bool with ns->setfcap_level: CAP_SETFCAP held in the
> namespace counts there and in its ancestors up to the one at that
> ->level.  cap_setfcap_level() computes it from the creator's cred.  A
> creator that holds CAP_SETFCAP passes on the setfcap_level of its own
> namespace, one that doesn't gets the new namespace's own level, and the
> initial namespace has 0.
> 
> verify_root_map() keeps checking the same condition: the bool was false
> exactly when the level is the namespace's own.
> 
> No functional change.
> 
> Assisted-by: LLM
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> ---
>  include/linux/capability.h     |  3 +++
>  include/linux/user_namespace.h |  7 ++++---
>  kernel/user_namespace.c        |  4 ++--
>  security/commoncap.c           | 26 ++++++++++++++++++++++++++
>  4 files changed, 35 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/capability.h b/include/linux/capability.h
> index 622137f66f09..026974a5111b 100644
> --- a/include/linux/capability.h
> +++ b/include/linux/capability.h
> @@ -34,6 +34,7 @@ struct cpu_vfs_cap_data {
>  #define _USER_CAP_HEADER_SIZE  (sizeof(struct __user_cap_header_struct))
>  #define _KERNEL_CAP_T_SIZE     (sizeof(kernel_cap_t))
>  
> +struct cred;
>  struct file;
>  struct inode;
>  struct dentry;
> @@ -222,4 +223,6 @@ int get_vfs_caps_from_disk(const struct mnt_idmap *idmap,
>  int cap_convert_nscap(const struct mnt_idmap *idmap, struct dentry *dentry,
>  		      const void **ivalue, size_t size);
>  
> +int cap_setfcap_level(const struct cred *cred, struct user_namespace *ns);
> +
>  #endif /* !_LINUX_CAPABILITY_H */
> diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
> index 91232053775d..f3753b6b6d29 100644
> --- a/include/linux/user_namespace.h
> +++ b/include/linux/user_namespace.h
> @@ -86,9 +86,10 @@ struct user_namespace {
>  	kgid_t			group;
>  	struct ns_common	ns;
>  	unsigned long		flags;
> -	/* parent_could_setfcap: true if the creator if this ns had CAP_SETFCAP
> -	 * in its effective capability set at the child ns creation time. */
> -	bool			parent_could_setfcap;
> +	/* setfcap_level: CAP_SETFCAP held in this ns counts here and in the
> +	 * ancestors up to the one at this ->level, see cap_setfcap_level().
> +	 */
> +	int			setfcap_level;
>  
>  #ifdef CONFIG_KEYS
>  	/* List of joinable keyrings in this namespace.  Modification access of
> diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
> index 50c374ed4687..a5a94367600d 100644
> --- a/kernel/user_namespace.c
> +++ b/kernel/user_namespace.c
> @@ -125,7 +125,7 @@ int create_user_ns(struct cred *new)
>  	if (!ns)
>  		goto fail_dec;
>  
> -	ns->parent_could_setfcap = cap_raised(new->cap_effective, CAP_SETFCAP);
> +	ns->setfcap_level = cap_setfcap_level(new, parent_ns);
>  
>  	ret = ns_common_init(ns);
>  	if (ret)
> @@ -921,7 +921,7 @@ static bool verify_root_map(const struct file *file,
>  		 * the new namespace.  Verify that the parent had CAP_SETFCAP
>  		 * when it unshared.
>  		 * */
> -		if (!file_ns->parent_could_setfcap)
> +		if (file_ns->setfcap_level >= file_ns->level)
>  			return false;
>  	} else {
>  		/* Process p1 is writing to uid_map of p2, who is in a child
> diff --git a/security/commoncap.c b/security/commoncap.c
> index d47ab3022343..a51b9d51d96d 100644
> --- a/security/commoncap.c
> +++ b/security/commoncap.c
> @@ -131,6 +131,32 @@ int cap_capable(const struct cred *cred, struct user_namespace *target_ns,
>  	return ret;
>  }
>  
> +/**
> + * cap_setfcap_level - Determine how far up CAP_SETFCAP of a cred reaches
> + * @cred: The credentials to use
> + * @ns: The user namespace of @cred or one of its descendants

Is @ns really always @cred or one of its descendants?

Should there be something like a BUG_ON(ns->level < cred->user_ns->level)
to make sure no user of this function passes in a wrong ns?

I think this function should not be directly used when doing checks in
the later patches.  Please use a clearly named helper, something like
cred_may_setfcap_userns(cred, ns).

> + * File capabilities belong to the kuid of a namespace's root user, and the
> + * same kuid can be the root user of ancestors of that namespace.  Every task
> + * gets CAP_SETFCAP when it enters a user namespace, so having it there says
> + * nothing about those ancestors.  user_ns->setfcap_level does: it is taken
> + * from the creator when the namespace is created.
> + *
> + * Return: the ->level of the topmost namespace, from @ns upwards, for which
> + * CAP_SETFCAP of @cred counts; @ns->level + 1 if it doesn't even over @ns.
> + */
> +int cap_setfcap_level(const struct cred *cred, struct user_namespace *ns)
> +{
> +	if (cap_capable(cred, ns, CAP_SETFCAP, CAP_OPT_NOAUDIT))

I'd far prefer this to be

	if (cap_capable(...) != 0)

especially since the next line checks cap_raised.  I sometimes get the
urge to fix the semantics of these to be consistent...

> +		return ns->level + 1;
> +
> +	if (cap_raised(cred->cap_effective, CAP_SETFCAP))
> +		return cred->user_ns->setfcap_level;
> +
> +	/* All we have is that we own a child of our namespace. */
> +	return cred->user_ns->level + 1;
> +}
> +
>  /**
>   * cap_settime - Determine whether the current process may set the system clock
>   * @ts: The time to set
> 
> -- 
> 2.55.0