Michal Koutný mkoutny@suse.com writes:
On Thu, Feb 10, 2022 at 08:13:20PM -0600, "Eric W. Biederman" ebiederm@xmission.com wrote:
@@ -1881,7 +1881,7 @@ static int do_execveat_common(int fd, struct filename *filename,
[...]
(current_user() != INIT_USER) &&
(current_ucounts() != &init_ucounts) &&[...]
@@ -2027,7 +2027,7 @@ static __latent_entropy struct task_struct *copy_process(
[...]
if (p->real_cred->user != INIT_USER &&
if ((task_ucounts(p) != &init_ucounts) &&These substitutions make sense to me.
!capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN)) goto bad_fork_cleanup_count;} diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c index 6b2e3ca7ee99..f0c04073403d 100644 --- a/kernel/user_namespace.c +++ b/kernel/user_namespace.c @@ -123,6 +123,8 @@ int create_user_ns(struct cred *new) ns->ucount_max[i] = INT_MAX; } set_rlimit_ucount_max(ns, UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC));
- if (new->ucounts == &init_ucounts)
set_rlimit_ucount_max(ns, UCOUNT_RLIMIT_NPROC, RLIMIT_INFINITY);First, I wanted to object this double fork_init() but I realized it's relevant for newly created user_ns.
Second, I think new->ucounts would be correct at this point and the check should be
if (ucounts == &init_ucounts)
i.e. before set_cred_ucounts() new->ucounts may not be correct.
I'd suggest also a comment in the create_user_ns() explaining the reason is to exempt global root from RLIMINT_NRPOC also indirectly via descendant user_nss.
Yes.
This one got culled from my next version of the patchset as it is not conservative enough. I think it is probably the right general direction.
On further reflection I am not convinced that it makes sense to test user or ucounts. They are really not fields designed to support permission checks.
I think if we want to exempt the root user's children from the root users rlimit using the second set_rlimit_ucount_max is the way to go.
Someone filed a bug that strongly suggests that we want the second set_rlimit_ucount_max: https://bugzilla.kernel.org/show_bug.cgi?id=215596
I am still trying to understand that case.
Eric