linux-kernel - [PATCH] kernel/sys: Optimize do_prlimit lock scope to reduce contention

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20241120132156.207250-1-zhen.ni@easystack.cn>
Date: Wed, 20 Nov 2024 21:21:56 +0800
From: Zhen Ni <zhen.ni@...ystack.cn>
To: akpm@...ux-foundation.org,
	viro@...iv.linux.org.uk,
	oleg@...hat.com,
	catalin.marinas@....com,
	brauner@...nel.org,
	zev@...ilderbeest.net
Cc: linux-kernel@...r.kernel.org,
	Zhen Ni <zhen.ni@...ystack.cn>
Subject: [PATCH] kernel/sys: Optimize do_prlimit lock scope to reduce contention

Refines the lock scope in the do_prlimit function to reduce
contention on task_lock(tsk->group_leader). The lock now protects only
sections that access or modify shared resources (rlim). Permission
checks (capable) and security validations (security_task_setrlimit)
are placed outside the lock, as they do not modify rlim and are
independent of shared data protection.

The security_task_setrlimit function is a Linux Security Module (LSM)
hook that evaluates resource limit changes based on security policies.
It does not alter the rlim data structure, as confirmed by existing
LSM implementations (e.g., SELinux and AppArmor). Thus, this function
does not require locking, ensuring correctness while improving
concurrency.

Signed-off-by: Zhen Ni <zhen.ni@...ystack.cn>
---
 kernel/sys.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/kernel/sys.c b/kernel/sys.c
index c4c701c6f0b4..ef99b654e8d8 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -1481,18 +1481,20 @@ static int do_prlimit(struct task_struct *tsk, unsigned int resource,

 	/* Holding a refcount on tsk protects tsk->signal from disappearing. */
 	rlim = tsk->signal->rlim + resource;
-	task_lock(tsk->group_leader);
 	if (new_rlim) {
 		/*
 		 * Keep the capable check against init_user_ns until cgroups can
 		 * contain all limits.
 		 */
 		if (new_rlim->rlim_max > rlim->rlim_max &&
-				!capable(CAP_SYS_RESOURCE))
-			retval = -EPERM;
-		if (!retval)
-			retval = security_task_setrlimit(tsk, resource, new_rlim);
+		    !capable(CAP_SYS_RESOURCE))
+			return -EPERM;
+		retval = security_task_setrlimit(tsk, resource, new_rlim);
+		if (retval)
+			return retval;
 	}
+
+	task_lock(tsk->group_leader);
 	if (!retval) {
 		if (old_rlim)
 			*old_rlim = *rlim;
-- 
2.20.1