[PATCH sched_ext/for-6.12] sched_ext: Fix unsafe list iteration in process_ddsp_deferred_locals()

[PATCH sched_ext/for-6.12] sched_ext: Fix unsafe list iteration in process_ddsp_deferred_locals()

[Tejun Heo]

process_ddsp_deferred_locals() executes deferred direct dispatches to the
local DSQs of remote CPUs. It iterates the tasks on
rq->scx.ddsp_deferred_locals list, removing and calling
dispatch_to_local_dsq() on each. However, the list is protected by the rq
lock that can be dropped by dispatch_to_local_dsq() temporarily, so the list
can be modified during the iteration, which can lead to oopses and other
failures.

Fix it by popping from the head of the list instead of iterating the list.

Signed-off-by: Tejun Heo <tj@kernel.org>
Fixes: 5b26f7b920f7 ("sched_ext: Allow SCX_DSQ_LOCAL_ON for direct dispatches")
---
 kernel/sched/ext.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

--- a/kernel/sched/ext.c
+++ b/kernel/sched/ext.c
@@ -2726,17 +2726,19 @@ static void set_next_task_scx(struct rq
 
 static void process_ddsp_deferred_locals(struct rq *rq)
 {
-	struct task_struct *p, *tmp;
+	struct task_struct *p;
 
 	lockdep_assert_rq_held(rq);
 
 	/*
 	 * Now that @rq can be unlocked, execute the deferred enqueueing of
 	 * tasks directly dispatched to the local DSQs of other CPUs. See
-	 * direct_dispatch().
+	 * direct_dispatch(). Keep popping from the head instead of using
+	 * list_for_each_entry_safe() as dispatch_local_dsq() may unlock @rq
+	 * temporarily.
 	 */
-	list_for_each_entry_safe(p, tmp, &rq->scx.ddsp_deferred_locals,
-				 scx.dsq_list.node) {
+	while ((p = list_first_entry_or_null(&rq->scx.ddsp_deferred_locals,
+				struct task_struct, scx.dsq_list.node))) {
 		s32 ret;
 
 		list_del_init(&p->scx.dsq_list.node);

Re: [PATCH sched_ext/for-6.12] sched_ext: Fix unsafe list iteration in process_ddsp_deferred_locals()

[David Vernet]

[-- Attachment #1: Type: text/plain, Size: 757 bytes --]

On Wed, Aug 07, 2024 at 10:17:38AM -1000, Tejun Heo wrote:
> process_ddsp_deferred_locals() executes deferred direct dispatches to the
> local DSQs of remote CPUs. It iterates the tasks on
> rq->scx.ddsp_deferred_locals list, removing and calling
> dispatch_to_local_dsq() on each. However, the list is protected by the rq
> lock that can be dropped by dispatch_to_local_dsq() temporarily, so the list
> can be modified during the iteration, which can lead to oopses and other
> failures.
> 
> Fix it by popping from the head of the list instead of iterating the list.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Fixes: 5b26f7b920f7 ("sched_ext: Allow SCX_DSQ_LOCAL_ON for direct dispatches")

Acked-by: David Vernet <void@manifault.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH sched_ext/for-6.12] sched_ext: Fix unsafe list iteration in process_ddsp_deferred_locals()

[Tejun Heo]

On Wed, Aug 07, 2024 at 10:17:38AM -1000, Tejun Heo wrote:
> process_ddsp_deferred_locals() executes deferred direct dispatches to the
> local DSQs of remote CPUs. It iterates the tasks on
> rq->scx.ddsp_deferred_locals list, removing and calling
> dispatch_to_local_dsq() on each. However, the list is protected by the rq
> lock that can be dropped by dispatch_to_local_dsq() temporarily, so the list
> can be modified during the iteration, which can lead to oopses and other
> failures.
> 
> Fix it by popping from the head of the list instead of iterating the list.
> 
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Fixes: 5b26f7b920f7 ("sched_ext: Allow SCX_DSQ_LOCAL_ON for direct dispatches")

Applied to sched_ext/for-6.12.

Thanks.

-- 
tejun