All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Eric W. Biederman" <ebiederm@xmission.com>
To: linux-kernel@vger.kernel.org
Cc: "Alexey Gladkov" <legion@kernel.org>,
	"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>,
	containers@lists.linux-foundation.org,
	"Michal Koutný" <mkoutny@suse.com>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	stable@vger.kernel.org
Subject: [PATCH 4/8] ucounts: Only except the root user in init_user_ns from RLIMIT_NPROC
Date: Thu, 10 Feb 2022 20:13:20 -0600	[thread overview]
Message-ID: <20220211021324.4116773-4-ebiederm@xmission.com> (raw)
In-Reply-To: <87o83e2mbu.fsf@email.froward.int.ebiederm.org>

In [1] Michal Koutný <mkoutny@suse.com> reported that it does not make
sense to unconditionally exempt the INIT_USER during fork and exec
from RLIMIT_NPROC and then to impose a limit on that same user with
is_ucounts_overlimit.  So I looked into why the exeception was added.

commit 909cc4ae86f3 ("[PATCH] Fix two bugs with process limits (RLIMIT_NPROC)") says:
>  If a setuid process swaps it's real and effective uids and then
>  forks, the fork fails if the new realuid has more processes than
>  the original process was limited to.  This is particularly a
>  problem if a user with a process limit (e.g. 256) runs a
>  setuid-root program which does setuid() + fork() (e.g. lprng) while
>  root already has more than 256 process (which is quite possible).
>
>  The root problem here is that a limit which should be a per-user
>  limit is being implemented as a per-process limit with per-process
>  (e.g. CAP_SYS_RESOURCE) controls.  Being a per-user limit, it
>  should be that the root-user can over-ride it, not just some
>  process with CAP_SYS_RESOURCE.
>
>  This patch adds a test to ignore process limits if the real user is root.

With the moving of the limits from per-user to per-user-per-user_ns it
is clear that testing a user_struct is no longer the proper test and
the test should be a test against the ucounts struct to match the
rest of the logic change that was made.

With RLIMIT_NPROC not being enforced for the global root user anywhere
else should it be enforced in is_ucounts_overlimit for a user
namespace created by the global root user?

Since this is limited to just the global root user, and RLIMIT_NPROC
is already ignored for that user I am going to vote no.

This change does imply that it becomes possible to limit all users in
a user namespace but to not enforce the rlimits on the root user or
anyone with CAP_SYS_ADMIN and CAP_SYS_RESOURCE in the user namespace.
It is not clear to me why any of those exceptions exist so I figure
we should until this is actually a problem for someone before
we relax the permission checks here.

Cc: stable@vger.kernel.org
Reported-by: Michal Koutný <mkoutny@suse.com>
[1] https://lkml.kernel.org/r/20220207121800.5079-5-mkoutny@suse.com
History-Tree: https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git
Fixes: 21d1c5e386bc ("Reimplement RLIMIT_NPROC on top of ucounts")
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 fs/exec.c               | 2 +-
 kernel/fork.c           | 2 +-
 kernel/user_namespace.c | 2 ++
 3 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index 1e7f757cbc2c..01c8c7bae9b4 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1881,7 +1881,7 @@ static int do_execveat_common(int fd, struct filename *filename,
 	 */
 	if ((current->flags & PF_NPROC_CHECK) &&
 	    is_ucounts_overlimit(current_ucounts(), UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC)) &&
-	    (current_user() != INIT_USER) &&
+	    (current_ucounts() != &init_ucounts) &&
 	    !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN)) {
 		retval = -EAGAIN;
 		goto out_ret;
diff --git a/kernel/fork.c b/kernel/fork.c
index 2b6a28a86325..6f62d37f3650 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -2027,7 +2027,7 @@ static __latent_entropy struct task_struct *copy_process(
 
 	retval = -EAGAIN;
 	if (is_ucounts_overlimit(task_ucounts(p), UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC))) {
-		if (p->real_cred->user != INIT_USER &&
+		if ((task_ucounts(p) != &init_ucounts) &&
 		    !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN))
 			goto bad_fork_cleanup_count;
 	}
diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
index 6b2e3ca7ee99..f0c04073403d 100644
--- a/kernel/user_namespace.c
+++ b/kernel/user_namespace.c
@@ -123,6 +123,8 @@ int create_user_ns(struct cred *new)
 		ns->ucount_max[i] = INT_MAX;
 	}
 	set_rlimit_ucount_max(ns, UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC));
+	if (new->ucounts == &init_ucounts)
+		set_rlimit_ucount_max(ns, UCOUNT_RLIMIT_NPROC, RLIMIT_INFINITY);
 	set_rlimit_ucount_max(ns, UCOUNT_RLIMIT_MSGQUEUE, rlimit(RLIMIT_MSGQUEUE));
 	set_rlimit_ucount_max(ns, UCOUNT_RLIMIT_SIGPENDING, rlimit(RLIMIT_SIGPENDING));
 	set_rlimit_ucount_max(ns, UCOUNT_RLIMIT_MEMLOCK, rlimit(RLIMIT_MEMLOCK));
-- 
2.29.2


  parent reply	other threads:[~2022-02-11  2:14 UTC|newest]

Thread overview: 78+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-07 12:17 [RFC PATCH 0/6] RLIMIT_NPROC in ucounts fixups Michal Koutný
2022-02-07 12:17 ` [RFC PATCH 1/6] set_user: Perform RLIMIT_NPROC capability check against new user credentials Michal Koutný
2022-02-10  1:14   ` Solar Designer
2022-02-10  1:57     ` Eric W. Biederman
2022-02-11 20:32     ` Eric W. Biederman
2022-02-12 22:14       ` Solar Designer
2022-02-15 11:55     ` Michal Koutný
2022-02-07 12:17 ` [RFC PATCH 2/6] set*uid: Check RLIMIT_PROC against new credentials Michal Koutný
2022-02-07 12:17 ` [RFC PATCH 3/6] cred: Count tasks by their real uid into RLIMIT_NPROC Michal Koutný
2022-02-07 12:17 ` [RFC PATCH 4/6] ucounts: Allow root to override RLIMIT_NPROC Michal Koutný
2022-02-10  0:21   ` Eric W. Biederman
2022-02-07 12:17 ` [RFC PATCH 5/6] selftests: Challenge RLIMIT_NPROC in user namespaces Michal Koutný
2022-02-10  1:22   ` Shuah Khan
2022-02-15  9:45     ` Michal Koutný
2022-02-07 12:18 ` [RFC PATCH 6/6] selftests: Test RLIMIT_NPROC in clone-created " Michal Koutný
2022-02-10  1:25   ` Shuah Khan
2022-02-15  9:34     ` Michal Koutný
2022-02-08 13:54 ` [RFC PATCH 0/6] RLIMIT_NPROC in ucounts fixups Eric W. Biederman
2022-02-11  2:01 ` [PATCH 0/8] ucounts: RLIMIT_NPROC fixes Eric W. Biederman
2022-02-11  2:13   ` [PATCH 1/8] ucounts: Fix RLIMIT_NPROC regression Eric W. Biederman
2022-02-14 18:37     ` Michal Koutný
2022-02-16 15:22       ` Eric W. Biederman
2022-02-11  2:13   ` [PATCH 2/8] ucounts: Fix set_cred_ucounts Eric W. Biederman
2022-02-15 11:10     ` Michal Koutný
2022-02-11  2:13   ` [PATCH 3/8] ucounts: Fix and simplify RLIMIT_NPROC handling during setuid()+execve Eric W. Biederman
2022-02-12 23:17     ` Solar Designer
2022-02-14 15:10       ` Eric W. Biederman
2022-02-14 17:43         ` Eric W. Biederman
2022-02-15 10:25         ` Michal Koutný
2022-02-16 15:35           ` Eric W. Biederman
2022-02-11  2:13   ` Eric W. Biederman [this message]
2022-02-15 10:54     ` [PATCH 4/8] ucounts: Only except the root user in init_user_ns from RLIMIT_NPROC Michal Koutný
2022-02-16 15:41       ` Eric W. Biederman
2022-02-11  2:13   ` [PATCH 5/8] ucounts: Handle wrapping in is_ucounts_overlimit Eric W. Biederman
2022-02-12 22:36     ` Solar Designer
2022-02-14 15:23       ` Eric W. Biederman
2022-02-14 15:23         ` Eric W. Biederman
2022-02-15 11:25         ` Michal Koutný
2022-02-14 17:16       ` David Laight
2022-02-11  2:13   ` [PATCH 6/8] ucounts: Handle inc_rlimit_ucounts wrapping in fork Eric W. Biederman
2022-02-11 11:34     ` Alexey Gladkov
2022-02-11 17:50       ` Eric W. Biederman
2022-02-11 18:32         ` Shuah Khan
2022-02-11 18:40         ` Alexey Gladkov
2022-02-11 19:56           ` Eric W. Biederman
2022-02-11  2:13   ` [PATCH 7/8] rlimit: For RLIMIT_NPROC test the child not the parent for capabilites Eric W. Biederman
2022-02-11  2:13   ` [PATCH 8/8] ucounts: Use the same code to enforce RLIMIT_NPROC in fork and exec Eric W. Biederman
2022-02-11 18:22   ` [PATCH 0/8] ucounts: RLIMIT_NPROC fixes Shuah Khan
2022-02-11 19:23     ` Eric W. Biederman
2022-02-15 11:37     ` Michal Koutný
2022-02-16 15:56   ` [PATCH v2 0/5] " Eric W. Biederman
2022-02-16 15:58     ` [PATCH v2 1/5] rlimit: Fix RLIMIT_NPROC enforcement failure caused by capability calls in set_user Eric W. Biederman
2022-02-16 17:42       ` Solar Designer
2022-02-16 15:58     ` [PATCH v2 2/5] ucounts: Enforce RLIMIT_NPROC not RLIMIT_NPROC+1 Eric W. Biederman
2022-02-16 15:58     ` [PATCH v2 3/5] ucounts: Base set_cred_ucounts changes on the real user Eric W. Biederman
2022-02-16 15:58     ` [PATCH v2 4/5] ucounts: Move RLIMIT_NPROC handling after set_user Eric W. Biederman
2022-02-16 15:58     ` [PATCH v2 5/5] ucounts: Handle wrapping in is_ucounts_overlimit Eric W. Biederman
2022-02-16 17:28       ` Shuah Khan
2022-02-18 15:34     ` [GIT PULL] ucounts: RLIMIT_NPROC fixes for v5.17 Eric W. Biederman
2022-02-20 19:05       ` pr-tracker-bot
2022-03-03  0:12       ` [GIT PULL] ucounts: Regression fix " Eric W. Biederman
2022-03-03  0:30         ` pr-tracker-bot
2022-02-12 15:32 ` [RFC PATCH 0/6] RLIMIT_NPROC in ucounts fixups Etienne Dechamps
2022-02-15 10:11   ` Michal Koutný
2022-02-23  0:57     ` Eric W. Biederman
2022-02-23 18:00       ` How should rlimits, suid exec, and capabilities interact? Eric W. Biederman
2022-02-23 19:44         ` Andy Lutomirski
2022-02-23 21:28           ` Willy Tarreau
2022-02-23 19:50         ` Linus Torvalds
2022-02-24  1:24           ` Eric W. Biederman
2022-02-24  1:41             ` Linus Torvalds
2022-02-24  2:12               ` Eric W. Biederman
2022-02-24 15:41                 ` [PATCH] ucounts: Fix systemd LimigtNPROC with private users regression Eric W. Biederman
2022-02-24 16:28                   ` Kees Cook
2022-02-24 18:53                     ` Michal Koutný
2022-02-25  0:29                     ` Eric W. Biederman
2022-02-24  3:00               ` How should rlimits, suid exec, and capabilities interact? David Laight
2022-02-24  1:32           ` Eric W. Biederman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220211021324.4116773-4-ebiederm@xmission.com \
    --to=ebiederm@xmission.com \
    --cc=brauner@kernel.org \
    --cc=containers@lists.linux-foundation.org \
    --cc=keescook@chromium.org \
    --cc=legion@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mkoutny@suse.com \
    --cc=ran.xiaokai@zte.com.cn \
    --cc=shuah@kernel.org \
    --cc=solar@openwall.com \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.