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
