diff mbox series

[3/5] powerpc/spufs: take a reference on contexts pulled off the runqueue

Message ID 20260812-spufs-fixes-v1-3-449e201d9f64@outlook.com (mailing list archive)
State New
Headers show
Series powerpc/spufs: fix a leak, an OOB read and three context lifetime/locking bugs | expand

Commit Message

Junrui Luo via B4 Relay Aug. 12, 2026, 5:48 a.m. UTC 
From: Junrui Luo <moonafterrain@outlook.com>

grab_runnable_context() unlinks the chosen context with
__spu_del_from_rq() and returns it after dropping spu_prio->runq_lock.
The runqueue holds no reference of its own — __spu_add_to_rq() only does
a list_add_tail() and __spu_del_from_rq() only a list_del_init() — so the
caller is left with a bare pointer. Both callers dereference it after the
lock is gone: __spu_deactivate() and spusched_tick() call spu_schedule(),
which starts with mutex_lock(&ctx->state_mutex).

The only thing synchronizing the two sides is spu_run_fini() ->
spu_del_from_rq(), which takes runq_lock. Once grab_runnable_context()
has unlinked the context, that call finds ctx->rq already empty and
becomes a no-op, so the owner is free to leave spu_run() and close the
context directory; the resulting put_spu_context() can reach
destroy_spu_context() and kfree() while the scheduler still holds the
pointer. The window spans a full spu_unschedule() -> spu_unbind_context()
SPU context save, and the BUG_ON(!list_empty(&ctx->rq)) in
destroy_spu_context() cannot catch it because list_del_init() has already
emptied ctx->rq.

Fix by taking a reference in grab_runnable_context() while runq_lock is
still held, where a queued context is guaranteed to be alive, and
dropping it in both callers once they are done with it. This matches what
find_victim() already does around its state_mutex trylock.

Fixes: e65c2f6fcebb ("[POWERPC] spufs: decouple spu scheduler from spufs_spu_run (asynchronous scheduling)")
Reported-by: Yuhao Jiang <danisjiang@gmail.com>
Assisted-by: Claude:claude-opus-5
Cc: stable@vger.kernel.org
Signed-off-by: Junrui Luo <moonafterrain@outlook.com>
---
Found by inspection; I have no Cell/PS3 hardware, so this is
compile-tested only.
---
 arch/powerpc/platforms/cell/spufs/sched.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
diff mbox series

Patch

diff --git a/arch/powerpc/platforms/cell/spufs/sched.c b/arch/powerpc/platforms/cell/spufs/sched.c
index c52af883e01c..1b50b28cab11 100644
--- a/arch/powerpc/platforms/cell/spufs/sched.c
+++ b/arch/powerpc/platforms/cell/spufs/sched.c
@@ -815,6 +815,9 @@  int spu_activate(struct spu_context *ctx, unsigned long flags)
  *
  * Remove the highest priority context on the runqueue and return it
  * to the caller.  Returns %NULL if no runnable context was found.
+ *
+ * The context is returned with a reference held on behalf of the caller,
+ * which has to drop it using put_spu_context() once it is done with it.
  */
 static struct spu_context *grab_runnable_context(int prio, int node)
 {
@@ -830,6 +833,7 @@  static struct spu_context *grab_runnable_context(int prio, int node)
 			/* XXX(hch): check for affinity here as well */
 			if (__node_allowed(ctx, node)) {
 				__spu_del_from_rq(ctx);
+				get_spu_context(ctx);
 				goto found;
 			}
 		}
@@ -860,6 +864,7 @@  static int __spu_deactivate(struct spu_context *ctx, int force, int max_prio)
 					   interruptible */
 					mutex_lock(&ctx->state_mutex);
 				}
+				put_spu_context(new);
 			}
 		}
 	}
@@ -933,8 +938,10 @@  static noinline void spusched_tick(struct spu_context *ctx)
 out:
 	spu_release(ctx);
 
-	if (new)
+	if (new) {
 		spu_schedule(spu, new);
+		put_spu_context(new);
+	}
 }
 
 /**