Re: [PATCH RFC 5/5] sched: use kfree_nolock() instead of kfree_rcu()

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Replacing `kfree_rcu()` with `kfree_nolock()` introduces an 'Invalid wait context' lockdep warning and RT deadlock risk.

Pre-existing issues:
- [Critical] Pre-existing Use-After-Free (UAF) due to lockless access of `user_cpus_ptr` in `relax_compatible_cpus_allowed_ptr()` racing with concurrent `sched_setaffinity()`.
--

--- Patch [5]: [PATCH RFC 5/5] sched: use kfree_nolock() instead of kfree_rcu() ---
commit 59f1ba4e68974041e778f52147c67a01b2854ec6
Author: Vlastimil Babka (SUSE) <[email protected]>

sched: use kfree_nolock() instead of kfree_rcu()

This commit replaces kfree_rcu() with kfree_nolock() in set_cpus_allowed_force()
to avoid unnecessary RCU grace period delays. It also updates
alloc_user_cpus_ptr() to no longer allocate extra space for the RCU head.

> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 96226707c2f61..d2929e4e23f13 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -2807,20 +2807,15 @@ void set_cpus_allowed_force(struct task_struct *p, const struct cpumask *new_mas
>  		.user_mask = NULL,
>  		.flags     = SCA_USER,	/* clear the user requested mask */
>  	};
[ ... ]
>  	/*
>  	 * Because this is called with p->pi_lock held, it is not possible
> -	 * to use kfree() here (when PREEMPT_RT=y), therefore punt to using
> -	 * kfree_rcu().
> +	 * to use kfree() here (when PREEMPT_RT=y), thus use kfree_nolock()
>  	 */
> -	kfree_rcu((union cpumask_rcuhead *)ac.user_mask, rcu);
> +	kfree_nolock(ac.user_mask);

[Severity: High]
Does this change trigger an "Invalid wait context" lockdep warning and RT
deadlock risk?

set_cpus_allowed_force() is called with the target task's pi_lock held, which
is a raw_spinlock_t.

kfree_nolock() forwards the call to free_to_pcs():

mm/slub.c:kfree_nolock() {
    ...
	if (likely(can_free_to_pcs(slab)) && likely(free_to_pcs(s, x, false)))
    ...
}

free_to_pcs() then unconditionally acquires a local_trylock_t:

mm/slub.c:free_to_pcs() {
    ...
	if (!local_trylock(&s->cpu_sheaves->lock))
    ...
}

On PREEMPT_RT, local_trylock_t maps to a sleeping spinlock. Acquiring a
sleeping lock (even via a trylock) while holding a raw spinlock violates the
wait-context rules (WAIT_SLEEP under WAIT_RAW).

Could this cause a lockdep splat since CONFIG_PROVE_RAW_LOCK_NESTING checks
declared wait-types rather than runtime behavior?


[Severity: Critical]
This is a pre-existing issue, but can the lockless access of user_cpus_ptr in
relax_compatible_cpus_allowed_ptr() race with a concurrent sched_setaffinity()?

relax_compatible_cpus_allowed_ptr() performs a lockless read of
p->user_cpus_ptr and passes it as ctx->new_mask:

kernel/sched/core.c:relax_compatible_cpus_allowed_ptr() {
    ...
	.new_mask  = task_user_cpus(p),
    ...
}

__sched_setaffinity() then allocates temporary masks with GFP_KERNEL:

kernel/sched/syscalls.c:__sched_setaffinity() {
    ...
	if (!alloc_cpumask_var(&cpus_allowed, GFP_KERNEL))
    ...
}

Since GFP_KERNEL can sleep or be preempted, a concurrent thread calling the
sched_setaffinity() syscall on the same task can update user_cpus_ptr and free
the old mask.

When __sched_setaffinity() resumes, it dereferences the potentially freed
memory:

kernel/sched/syscalls.c:__sched_setaffinity() {
    ...
	cpumask_and(new_mask, ctx->new_mask, cpus_allowed);
    ...
}

Can this use-after-free lead to memory corruption or a kernel crash if
triggered by unprivileged userspace executing a compat binary?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5