Michal Koutný mkoutny@suse.com wrote:
It was reported that v5.14 behaves differently when enforcing RLIMIT_NPROC limit, namely, it allows one more task than previously. This is consequence of the commit 21d1c5e386bc ("Reimplement RLIMIT_NPROC on top of ucounts") that missed the sharpness of equality in the forking path.
This can be fixed either by fixing the test or by moving the increment to be before the test. Fix it my moving copy_creds which contains the increment before is_ucounts_overlimit.
This is necessary so that in the case of CLONE_NEWUSER the new credential with the new ucounts is available of is_ucounts_overlimit to test.
Both the test in fork and the test in set_user were semantically changed when the code moved to ucounts. The change of the test in fork was bad because it was before the increment. The test in set_user was wrong and the change to ucounts fixed it. So this fix is only restore the old behavior in one lcatio not two.
Link: https://lkml.kernel.org/r/20220204181144.24462-1-mkoutny@suse.com Cc: stable@vger.kernel.org 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/fork.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/kernel/fork.c b/kernel/fork.c index d75a528f7b21..17d8a8c85e3b 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -2021,18 +2021,18 @@ static __latent_entropy struct task_struct *copy_process( #ifdef CONFIG_PROVE_LOCKING DEBUG_LOCKS_WARN_ON(!p->softirqs_enabled); #endif + retval = copy_creds(p, clone_flags); + if (retval < 0) + goto bad_fork_free; + retval = -EAGAIN; if (is_ucounts_overlimit(task_ucounts(p), UCOUNT_RLIMIT_NPROC, rlimit(RLIMIT_NPROC))) { if (p->real_cred->user != INIT_USER && !capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN)) - goto bad_fork_free; + goto bad_fork_cleanup_count; } current->flags &= ~PF_NPROC_EXCEEDED;
- retval = copy_creds(p, clone_flags); - if (retval < 0) - goto bad_fork_free; - /* * If multiple threads are within copy_process(), then this check * triggers too late. This doesn't hurt, the check is only there
On Thu, Feb 10, 2022 at 08:13:17PM -0600, "Eric W. Biederman" ebiederm@xmission.com wrote:
This can be fixed either by fixing the test or by moving the increment to be before the test. Fix it my moving copy_creds which contains the increment before is_ucounts_overlimit.
This is simpler than my approach and I find it correct too.
Both the test in fork and the test in set_user were semantically changed when the code moved to ucounts. The change of the test in fork was bad because it was before the increment.
The test in set_user was wrong and the change to ucounts fixed it. So this fix is only restore the old behavior in one lcatio not two.
Whom should be the task accounted to in the case of set*uid? (The change to ucounts made the check against the pre-switch user's ucounts.)
kernel/fork.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
Reviewed-by: Michal Koutný mkoutny@suse.com
Michal Koutný mkoutny@suse.com writes:
On Thu, Feb 10, 2022 at 08:13:17PM -0600, "Eric W. Biederman" ebiederm@xmission.com wrote:
This can be fixed either by fixing the test or by moving the increment to be before the test. Fix it my moving copy_creds which contains the increment before is_ucounts_overlimit.
This is simpler than my approach and I find it correct too.
Both the test in fork and the test in set_user were semantically changed when the code moved to ucounts. The change of the test in fork was bad because it was before the increment.
The test in set_user was wrong and the change to ucounts fixed it. So this fix is only restore the old behavior in one lcatio not two.
Whom should be the task accounted to in the case of set*uid? (The change to ucounts made the check against the pre-switch user's ucounts.)
It needs to be post-switch in the case of set*id().
I have that fixed in the next version of my patchset.
kernel/fork.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
Reviewed-by: Michal Koutný mkoutny@suse.com
linux-stable-mirror@lists.linaro.org
-
Eric W. Biederman -
Michal Koutný