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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] cred: use-after-put of user_ns in userns_install()
--

commit 4646fadaf7eaab13c361fe3e2861493a87f5e140
Author: Josef Bacik <josef@toxicpanda.com>

cred: record how far up CAP_SETFCAP reaches

This patch introduces cred->setfcap_level to track how far up the user
namespace hierarchy a CAP_SETFCAP capability extends. It sets this value
in set_cred_user_ns() when entering a new user namespace.

> diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
> index 1b23d819d3983..6b45df3a8d826 100644
> --- a/kernel/user_namespace.c
> +++ b/kernel/user_namespace.c
> @@ -44,6 +44,9 @@ static void dec_user_namespaces(struct ucounts *ucounts)
>  
>  static void set_cred_user_ns(struct cred *cred, struct user_namespace *user_ns)
>  {
> +	/* The last chance to see what we can do outside of the new namespace. */
> +	cred->setfcap_level = cap_setfcap_level(cred, user_ns->parent);
> +

[Severity: Low]
Could this result in a logical use-after-put of cred->user_ns when called
from userns_install()?

Looking at userns_install() in kernel/user_namespace.c, the reference to
cred->user_ns is relinquished right before calling set_cred_user_ns():

userns_install() {
    ...
	put_user_ns(cred->user_ns);
	set_cred_user_ns(cred, get_user_ns(user_ns));
    ...
}

Inside the newly added call to cap_setfcap_level(), the provided cred
object is dereferenced. Specifically, cap_setfcap_level() checks
cap_capable(), which inspects cred->user_ns, and later uses
cred->user_ns->level as a fallback.

While the object is independently kept alive by current->real_cred->user_ns
during the setns system call, does this pattern violate object lifecycle
semantics by relying on a logically dropped pointer?

Should the reference to cred->user_ns be saved to a temporary variable and
put only after set_cred_user_ns returns?

>  	/* Start with the same capabilities as init but useless for doing
>  	 * anything as the capabilities are bound to the new user namespace.
>  	 */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261006-b4-setfcap-userns-v1-0-f47e7ed66072@toxicpanda.com?part=1