Re: [PATCH cgroup/for-7.2-fixes] cgroup: Create the psimon kthread outside of cgroup_mutex

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

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the psi rtpoll_timer can be re-armed after it is deleted and subsequently execute after the psi_group is freed, causing a Use-After-Free.
--

--- Patch [1]: [PATCH cgroup/for-7.2-fixes] cgroup: Create the psimon kthread outside of cgroup_mutex ---
commit 8790bcf0b23cb2132be9400b975c7f18fd1147b7
Author: Tejun Heo <[email protected]>

cgroup: Create the psimon kthread outside of cgroup_mutex

This commit splits the trigger creation process so the rtpoll worker thread
is forked without holding cgroup_mutex. This resolves a deadlock between
cgroup_mutex and the fork path during sched_ext enablement.

Link: https://lore.kernel.org/all/[email protected]/

> diff --git a/kernel/sched/psi.c b/kernel/sched/psi.c
> index 848955f8893dd..b9f207c920edb 100644
> --- a/kernel/sched/psi.c
> +++ b/kernel/sched/psi.c

[ ... ]

> @@ -1565,12 +1592,22 @@ static ssize_t psi_write(struct file *file, const char __user *user_buf,
>  		return -EBUSY;
>  	}
>  
> -	new = psi_trigger_create(&psi_system, buf, res, file, NULL);
> +	new = psi_trigger_create(&psi_system, buf, res, file, NULL,
> +				 &need_rtpoll_worker);
>  	if (IS_ERR(new)) {
>  		mutex_unlock(&seq->lock);
>  		return PTR_ERR(new);
>  	}
>  
> +	if (need_rtpoll_worker) {
> +		ret = psi_trigger_create_rtpoll_worker(&psi_system);
> +		if (ret) {
> +			psi_trigger_destroy(new);

[Severity: High]
This isn't a bug introduced by this patch, but can the psi rtpoll_timer be
re-armed after it is deleted and subsequently execute after the psi_group is
freed, causing a use-after-free?

In psi_trigger_destroy(), the timer is deleted before synchronize_rcu():

psi_trigger_destroy() {
    rcu_assign_pointer(group->rtpoll_task, NULL);
    timer_delete(&group->rtpoll_timer);
    ...
    synchronize_rcu();
}

If Thread A calls psi_schedule_rtpoll_work() and gets delayed or preempted
after reading rtpoll_task inside the RCU read-side critical section:

psi_schedule_rtpoll_work() {
    rcu_read_lock();
    task = rcu_dereference(group->rtpoll_task);
    ...
    // Thread A delayed here
}

Thread B could then run psi_trigger_destroy(), delete the timer, and block in
synchronize_rcu() waiting for Thread A. When Thread A resumes, it sees the
task is non-NULL and calls mod_timer():

    if (likely(task))
        mod_timer(&group->rtpoll_timer, jiffies + delay);

This re-arms the timer after it was already deleted by Thread B. 

Later, when the cgroup is removed and psi_cgroup_free() is called, the
psi_group is freed without deleting this leaked timer. When the timer finally
expires, poll_timer_fn() will attempt to access the freed group memory.

> +			mutex_unlock(&seq->lock);
> +			return ret;
> +		}
> +	}
> +
>  	smp_store_release(&seq->private, new);
>  	mutex_unlock(&seq->lock);
>

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