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 cred->ucount and it 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, affecting all uses of cred->ulimit not just RLIMIT_NPROC.
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(). Remove that draft version of the check.
All that matters for setting the cred->ucounts are the user_ns and uid fields in the cred.
Cc: stable@vger.kernel.org Link: https://lkml.kernel.org/r/20220207121800.5079-4-mkoutny@suse.com Reported-by: Michal Koutný mkoutny@suse.com Reviewed-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;