Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   =?UTF-8?q?Michal=20Koutn=C3=BD?= <mkoutny@suse.com>
To:     Eric Biederman <ebiederm@xmission.com>,
        Alexey Gladkov <legion@kernel.org>
Cc:     Kees Cook <keescook@chromium.org>, Shuah Khan <shuah@kernel.org>,
        Christian Brauner <brauner@kernel.org>,
        Solar Designer <solar@openwall.com>,
        Ran Xiaokai <ran.xiaokai@zte.com.cn>,
        linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
        Linux Containers <containers@lists.linux-foundation.org>
Subject: [RFC PATCH 3/6] cred: Count tasks by their real uid into RLIMIT_NPROC
Date:   Mon,  7 Feb 2022 13:17:57 +0100
Message-Id: <20220207121800.5079-4-mkoutny@suse.com>
In-Reply-To: <20220207121800.5079-1-mkoutny@suse.com>
References: <20220207121800.5079-1-mkoutny@suse.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

Tasks are associated to multiple users at once. Historically and as per
setrlimit(2) RLIMIT_NPROC is enforce based on real user ID.

The commit 21d1c5e386bc ("Reimplement RLIMIT_NPROC on top of ucounts")
made the accounting structure "indexed" by euid and hence potentially
account tasks differently.

The effective user ID may be different e.g. for setuid programs but
those are exec'd into already existing task (i.e. below limit), so
different accounting is moot.

Some special setresuid(2) users may notice the difference, justifying
this fix.
(This is just illustrative, it piggy-backs onto nproc_flags and should
be implemented properly.)

Fixes: 21d1c5e386bc ("Reimplement RLIMIT_NPROC on top of ucounts")
Signed-off-by: Michal Koutný <mkoutny@suse.com>
---
 kernel/cred.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/kernel/cred.c b/kernel/cred.c
index 791cab70b764..ed247daa1f67 100644
--- a/kernel/cred.c
+++ b/kernel/cred.c
@@ -668,6 +668,7 @@ int set_cred_ucounts(struct cred *new, unsigned int *nproc_flags)
 	struct task_struct *task = current;
 	const struct cred *old = task->real_cred;
 	struct ucounts *new_ucounts, *old_ucounts = new->ucounts;
+	kuid_t new_uid = nproc_flags ? new->uid : new->euid;
 
 	if (new->user == old->user && new->user_ns == old->user_ns)
 		return 0;
@@ -676,10 +677,10 @@ int set_cred_ucounts(struct cred *new, unsigned int *nproc_flags)
 	 * This optimization is needed because alloc_ucounts() uses locks
 	 * for table lookups.
 	 */
-	if (old_ucounts->ns == new->user_ns && uid_eq(old_ucounts->uid, new->euid))
+	if (old_ucounts->ns == new->user_ns && uid_eq(old_ucounts->uid, new_uid))
 		return 0;
 
-	if (!(new_ucounts = alloc_ucounts(new->user_ns, new->euid)))
+	if (!(new_ucounts = alloc_ucounts(new->user_ns, new_uid)))
 		return -EAGAIN;
 
 	new->ucounts = new_ucounts;
-- 
2.34.1