|
Message-Id: <54b0cf752c2c275a164aa980e2d8e4e464797ca1.1610299858.git.gladkov.alexey@gmail.com> Date: Sun, 10 Jan 2021 18:33:47 +0100 From: Alexey Gladkov <gladkov.alexey@...il.com> To: LKML <linux-kernel@...r.kernel.org>, Linux Containers <containers@...ts.linux-foundation.org>, Kernel Hardening <kernel-hardening@...ts.openwall.com> Cc: Alexey Gladkov <legion@...nel.org>, "Eric W . Biederman" <ebiederm@...ssion.com>, Kees Cook <keescook@...omium.org>, Christian Brauner <christian@...uner.io>, Linus Torvalds <torvalds@...ux-foundation.org> Subject: [RFC PATCH v2 8/8] Move RLIMIT_NPROC check to the place where we increment the counter After calling set_user(), we always have to call commit_creds() to apply new credentials upon the current task. There is no need to separate limit check and counter incrementing. Signed-off-by: Alexey Gladkov <gladkov.alexey@...il.com> --- kernel/cred.c | 22 +++++++++++++++++----- kernel/sys.c | 13 ------------- 2 files changed, 17 insertions(+), 18 deletions(-) diff --git a/kernel/cred.c b/kernel/cred.c index 89a945571533..770447b4f4de 100644 --- a/kernel/cred.c +++ b/kernel/cred.c @@ -488,14 +488,26 @@ int commit_creds(struct cred *new) if (!gid_eq(new->fsgid, old->fsgid)) key_fsgid_changed(new); - /* do it - * RLIMIT_NPROC limits on user->processes have already been checked - * in set_user(). - */ alter_cred_subscribers(new, 2); if (new->user != old->user || new->user_ns != old->user_ns) { + bool overlimit; + set_cred_ucounts(new, new->user_ns, new->euid); - inc_rlimit_ucounts(new->ucounts, UCOUNT_RLIMIT_NPROC, 1); + + overlimit = inc_rlimit_ucounts_and_test(new->ucounts, UCOUNT_RLIMIT_NPROC, + 1, rlimit(RLIMIT_NPROC)); + + /* + * We don't fail in case of NPROC limit excess here because too many + * poorly written programs don't check set*uid() return code, assuming + * it never fails if called by root. We may still enforce NPROC limit + * for programs doing set*uid()+execve() by harmlessly deferring the + * failure to the execve() stage. + */ + if (overlimit && new->user != INIT_USER) + current->flags |= PF_NPROC_EXCEEDED; + else + current->flags &= ~PF_NPROC_EXCEEDED; } rcu_assign_pointer(task->real_cred, new); rcu_assign_pointer(task->cred, new); diff --git a/kernel/sys.c b/kernel/sys.c index c2734ab9474e..180c4e06064f 100644 --- a/kernel/sys.c +++ b/kernel/sys.c @@ -467,19 +467,6 @@ static int set_user(struct cred *new) if (!new_user) return -EAGAIN; - /* - * We don't fail in case of NPROC limit excess here because too many - * poorly written programs don't check set*uid() return code, assuming - * it never fails if called by root. We may still enforce NPROC limit - * for programs doing set*uid()+execve() by harmlessly deferring the - * failure to the execve() stage. - */ - if (is_ucounts_overlimit(new->ucounts, UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC)) && - new_user != INIT_USER) - current->flags |= PF_NPROC_EXCEEDED; - else - current->flags &= ~PF_NPROC_EXCEEDED; - free_uid(new->user); new->user = new_user; return 0; -- 2.29.2
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.