linux-kernel - Re: [RFC PATCH 4/6] ucounts: Allow root to override RLIMIT

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <87a6eza7ws.fsf@email.froward.int.ebiederm.org>
Date:   Wed, 09 Feb 2022 18:21:07 -0600
From:   "Eric W. Biederman" <ebiederm@...ssion.com>
To:     Michal Koutný <mkoutny@...e.com>
Cc:     Alexey Gladkov <legion@...nel.org>,
        Kees Cook <keescook@...omium.org>,
        Shuah Khan <shuah@...nel.org>,
        Christian Brauner <brauner@...nel.org>,
        Solar Designer <solar@...nwall.com>,
        Ran Xiaokai <ran.xiaokai@....com.cn>,
        linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org,
        Linux Containers <containers@...ts.linux-foundation.org>
Subject: Re: [RFC PATCH 4/6] ucounts: Allow root to override RLIMIT_NPROC

Michal Koutný <mkoutny@...e.com> writes:

> Call sites of ucounts_limit_cmp() would allow the global root or capable
> user to bypass RLIMIT_NPROC on the bottom level of user_ns tree by not
> looking at ucounts at all.
>
> As the traversal up the user_ns tree continues, the ucounts to which the
> task is charged may switch the owning user (to the creator of user_ns).
> If the new chargee is root, we don't really care about RLIMIT_NPROC
> observation, so lift the limit to the max.
>
> The result is that an unprivileged user U can globally run more that
> RLIMIT_NPROC (of user_ns) tasks but within each user_ns it is still
> limited to RLIMINT_NPROC (as passed into task->signal->rlim) iff the
> user_nss are created by the privileged user.

My apologies.  When I first looked at this I thought this change was
non-sense.  However I had missed the special logic that happens with
RLIMIT_NPROC to carefully allow the root user to bypass the NPROC
limits.

So yes this does look like something that needs to be addressed as well.

Thank you for reporting all of these issues.

Eric

>
> Signed-off-by: Michal Koutný <mkoutny@...e.com>
> ---
>  kernel/ucount.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/kernel/ucount.c b/kernel/ucount.c
> index 53ccd96387dd..f52b7273a572 100644
> --- a/kernel/ucount.c
> +++ b/kernel/ucount.c
> @@ -356,6 +356,9 @@ long ucounts_limit_cmp(struct ucounts *ucounts, enum ucount_type type, unsigned
>  		if (excess > 0)
>  			return excess;
>  		max = READ_ONCE(iter->ns->ucount_max[type]);
> +		/* Next ucounts owned by root? RLIMIT_NPROC is moot */
> +		if (type == UCOUNT_RLIMIT_NPROC && uid_eq(iter->ns->owner, GLOBAL_ROOT_UID))
> +			max = LONG_MAX;
>  	}
>  	return excess;
>  }