Michal Koutný mkoutny@suse.com wrote:
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.
I looked at the cred->ucount is only used for rlimit operations that were previously stored in cred->user. Making the fact cred->ucount can refer to a different user from cred->user a bug working will all rlimits not just RLIMIT_NPROC.
So fix set_cred_ucounts to always use the real uid not the effective uid.
Further simplify set_cred_ucounts by noticing that set_cred_ucounts somehow retained a draft version of the check to see if alloc_ucounts was needed that checks the new->user and new->user_ns against the current_real_cred(), when nothing matters for setting the ucounts field of a struct cred except the other fields in that same struct cred.
So delete the confusing and wrong check against the current_real_cred(), and all of it's intermediate variables.
Cc: stable@vger.kernel.org Link: https://lkml.kernel.org/r/20220207121800.5079-4-mkoutny@suse.com Reported-by: Michal Koutný mkoutny@suse.com Fixes: 21d1c5e386bc ("Reimplement RLIMIT_NPROC on top of ucounts") Signed-off-by: "Eric W. Biederman" ebiederm@xmission.com --- kernel/cred.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/kernel/cred.c b/kernel/cred.c index 473d17c431f3..933155c96922 100644 --- a/kernel/cred.c +++ b/kernel/cred.c @@ -665,21 +665,16 @@ EXPORT_SYMBOL(cred_fscmp);
int set_cred_ucounts(struct cred *new) { - struct task_struct *task = current; - const struct cred *old = task->real_cred; struct ucounts *new_ucounts, *old_ucounts = new->ucounts;
- if (new->user == old->user && new->user_ns == old->user_ns) - return 0; - /* * 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;