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

[Andrew Morton <akpm@linux-foundation.org>]

On Wed, 20 Nov 2024 21:21:56 +0800 Zhen Ni <zhen.ni@easystack.cn> wrote:

> 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.

Let's cc linux-security-module@vger.kernel.org, as we're proposing
altering their locking environment!

> 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.

Seems sane.

Does any code call do_prlimit() frequently enough for this to matter?

> --- 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;