[PATCH] sched_ext: Reject rq-lock-taking kfuncs in tracing programs

[PATCH] sched_ext: Reject rq-lock-taking kfuncs in tracing programs

[sprooc]

From: Junbin Zhang <sprooc69@gmail.com>

scx_bpf_cpuperf_set() and scx_bpf_cidperf_set() are members of
scx_kfunc_ids_any, which is registered for BPF_PROG_TYPE_TRACING.
A tracing program can run in a context which already holds the target
rq lock, in which case scx_cpuperf_set() attempts to acquire the same
lock again:

  __schedule()
    rq_lock(rq);                          /* rq->__lock is held */
    trace_sched_switch();
      BPF tracing program
        scx_bpf_cpuperf_set(smp_processor_id(), 1024);
          scx_cpuperf_set();
            locked_rq = scx_locked_rq(); /* NULL */
            rq_lock_irqsave(rq, &rf);    /* same lock */

scx_locked_rq() only tracks rq locks held during SCX callback
invocations, so it cannot detect an rq lock held by the surrounding
kernel code.

The deadlock can be reproduced with two BPF programs. The first is a
minimal SCX scheduler, which only needs to be registered and enabled:

  SEC(".struct_ops.link")
  struct sched_ext_ops minimal_ops = {
          .name = "minimal",
  };

The second is a tracing program attached to tp_btf/sched_switch,
which fires from __schedule() with rq->__lock already held:

  SEC("tp_btf/sched_switch")
  int BPF_PROG(on_sched_switch, bool preempt, struct task_struct *prev,
               struct task_struct *next, unsigned int prev_state)
  {
          scx_bpf_cpuperf_set(bpf_get_smp_processor_id(), 1024);
          return 0;
  }

With both programs attached, the next context switch on any CPU
deadlocks that CPU. Lockdep reports:

  WARNING: possible recursive locking detected
  7.3.0-rc7 #3 Not tainted
  swapper/3/0 is trying to acquire lock:
   (&rq->__lock){-.-.}-{2:2}, at: raw_spin_rq_lock_nested+0x1e/0x30

  but task is already holding lock:
   (&rq->__lock){-.-.}-{2:2}, at: __schedule+0xeb/0x1430

   Possible unsafe locking scenario:

         CPU0
         ----
    lock(&rq->__lock);
    lock(&rq->__lock);

    *** DEADLOCK ***

  Call Trace:
   <TASK>
   print_deadlock_bug.cold+0xc0/0xd0
   __lock_acquire+0x12fa/0x1b80
   lock_acquire+0xd1/0x2f0
   _raw_spin_lock_nested+0x32/0x40
   raw_spin_rq_lock_nested+0x1e/0x30
   scx_cpuperf_set+0x1e9/0x310
   scx_bpf_cpuperf_set+0x9b/0x230
   bpf_prog_..._on_sched_switch+0x3d/0x41
   bpf_trace_run4+0x125/0x450
   __traceiter_sched_switch+0x4b/0x70
   __schedule+0x94a/0x1430
   schedule_idle+0x26/0x50
   cpu_startup_entry+0x2d/0x30
   start_secondary+0xfc/0x100
   </TASK>

The CPU remains locked up with interrupts disabled.
scx_bpf_cidperf_set() is similarly affected, as it resolves the CID
through scx_cid_to_cpu() before reaching the same rq-locking path.

There is no reliable way for these kfuncs to determine whether the
surrounding tracing context already holds an rq lock. Reject calls to
both kfuncs from tracing programs at verification time, while
preserving their availability to SCX struct_ops and syscall programs.

Fixes: a11d6784d731 ("sched_ext: Fix missing rq lock in scx_bpf_cpuperf_set()")
Signed-off-by: Junbin Zhang <sprooc69@gmail.com>
---
 kernel/sched/ext/ext.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
index 4b7d2602b4d4..88f4abe944a8 100644
--- a/kernel/sched/ext/ext.c
+++ b/kernel/sched/ext/ext.c
@@ -11822,6 +11822,22 @@ static const struct btf_kfunc_id_set scx_kfunc_set_any = {
 	.filter			= scx_kfunc_context_filter,
 };
 
+/*
+ * Kfuncs which may take the rq lock on their unlocked path.
+ *
+ * A tracing program can execute in any kernel context, including ones which
+ * already hold the rq lock (e.g. sched_switch). scx_locked_rq() only tracks
+ * rq locks taken for SCX callbacks, so from such a context it returns NULL
+ * and these kfuncs would acquire the already held rq lock again and deadlock
+ * the CPU. They cannot be made safe for tracing programs, so keep them off
+ * the tracing hook. Members must also stay in scx_kfunc_ids_any so that
+ * struct_ops and syscall programs keep their access.
+ */
+BTF_KFUNCS_START(scx_kfunc_ids_no_tracing)
+BTF_ID_FLAGS(func, scx_bpf_cpuperf_set, KF_IMPLICIT_ARGS)
+BTF_ID_FLAGS(func, scx_bpf_cidperf_set, KF_IMPLICIT_ARGS)
+BTF_KFUNCS_END(scx_kfunc_ids_no_tracing)
+
 /*
  * cpu-form kfuncs that are forbidden from cid-form schedulers
  * (bpf_sched_ext_ops_cid). Programs targeting the cid struct_ops type must
@@ -11930,6 +11946,7 @@ int scx_kfunc_context_filter(const struct bpf_prog *prog, u32 kfunc_id)
 	bool in_any = btf_id_set8_contains(&scx_kfunc_ids_any, kfunc_id);
 	bool in_cpu_only = btf_id_set8_contains(&scx_kfunc_ids_cpu_only, kfunc_id);
 	bool in_cid = btf_id_set8_contains(&scx_kfunc_ids_cid, kfunc_id);
+	bool in_no_tracing = btf_id_set8_contains(&scx_kfunc_ids_no_tracing, kfunc_id);
 	u32 moff, flags;
 
 	/* Not an SCX kfunc - allow. */
@@ -11942,7 +11959,7 @@ int scx_kfunc_context_filter(const struct bpf_prog *prog, u32 kfunc_id)
 		return (in_unlocked || in_select_cpu || in_idle || in_any || in_cid) ? 0 : -EACCES;
 
 	if (prog->type != BPF_PROG_TYPE_STRUCT_OPS)
-		return (in_any || in_idle || in_cid) ? 0 : -EACCES;
+		return (!in_no_tracing && (in_any || in_idle || in_cid)) ? 0 : -EACCES;
 
 	/*
 	 * add_subprog_and_kfunc() collects all kfunc calls, including dead code
-- 
2.54.0