select_cpu_from_kfunc() skipped pi_lock for @p when called from ops.select_cpu() or another rq-locked SCX op, assuming the held lock protects @p. scx_bpf_select_cpu_dfl() / __scx_bpf_select_cpu_and() accept an arbitrary KF_RCU task_struct, so a caller in e.g. ops.select_cpu(p1) or ops.enqueue(p1) can pass some other p2 - the held pi_lock / rq lock is p1's, not p2's - and reading p2->cpus_ptr / nr_cpus_allowed races with set_cpus_allowed_ptr() and migrate_disable_switch() on another CPU. Abort the scheduler on cross-task calls in both branches: for ops.select_cpu() use scx_kf_arg_task_ok() to verify @p is the wake-up task recorded in current->scx.kf_tasks[] by SCX_CALL_OP_TASK_RET(); for other rq-locked SCX ops compare task_rq(p) against scx_locked_rq(). v2: Switch the in_select_cpu cross-task check from direct_dispatch_task comparison to scx_kf_arg_task_ok(). The former spuriously rejects when ops.select_cpu() calls scx_bpf_dsq_insert() first, then calls scx_bpf_select_cpu_*() on the same task. (Andrea Righi)
Showing the changes to kernel (1 of 1 file). Show all
Unified · Split
| @@ -927,14 +927,24 @@ static s32 select_cpu_from_kfunc(struct scx_sched *sch, struct task_struct *p, · 926 unmodified lines | |||
| 927 | * Accessing p->cpus_ptr / p->nr_cpus_allowed needs either @p's rq | 927 | * Accessing p->cpus_ptr / p->nr_cpus_allowed needs either @p's rq |
| 928 | * lock or @p's pi_lock. Three cases: | 928 | * lock or @p's pi_lock. Three cases: |
| 929 | * | 929 | * |
| 930 | * - inside ops.select_cpu(): try_to_wake_up() holds @p's pi_lock. | 930 | * - inside ops.select_cpu(): try_to_wake_up() holds the wake-up |
| 931 | * task's pi_lock; the wake-up task is recorded in kf_tasks[0] | ||
| 932 | * by SCX_CALL_OP_TASK_RET(). | ||
| 931 | * - other rq-locked SCX op: scx_locked_rq() points at the held rq. | 933 | * - other rq-locked SCX op: scx_locked_rq() points at the held rq. |
| 932 | * - truly unlocked (UNLOCKED ops, SYSCALL, non-SCX struct_ops): | 934 | * - truly unlocked (UNLOCKED ops, SYSCALL, non-SCX struct_ops): |
| 933 | * nothing held, take pi_lock ourselves. | 935 | * nothing held, take pi_lock ourselves. |
| 936 | * | ||
| 937 | * In the first two cases, BPF schedulers may pass an arbitrary task | ||
| 938 | * that the held lock doesn't cover. Refuse those. | ||
| 934 | */ | 939 | */ |
| 935 | if (this_rq()->scx.in_select_cpu) { | 940 | if (this_rq()->scx.in_select_cpu) { |
| 941 | if (!scx_kf_arg_task_ok(sch, p)) | ||
| 942 | return -EINVAL; | ||
| 936 | lockdep_assert_held(&p->pi_lock); | 943 | lockdep_assert_held(&p->pi_lock); |
| 937 | } else if (!scx_locked_rq()) { | 944 | } else if (scx_locked_rq()) { |
| 945 | if (task_rq(p) != scx_locked_rq()) | ||
| 946 | goto cross_task; | ||
| 947 | } else { | ||
| 938 | raw_spin_lock_irqsave(&p->pi_lock, irq_flags); | 948 | raw_spin_lock_irqsave(&p->pi_lock, irq_flags); |
| 939 | we_locked = true; | 949 | we_locked = true; |
| 940 | } | 950 | } |
| @@ -960,6 +970,11 @@ static s32 select_cpu_from_kfunc(struct scx_sched *sch, struct task_struct *p, · 19 unmodified lines | |||
| 960 | raw_spin_unlock_irqrestore(&p->pi_lock, irq_flags); | 970 | raw_spin_unlock_irqrestore(&p->pi_lock, irq_flags); |
| 961 | 971 | ||
| 962 | return cpu; | 972 | return cpu; |
| 973 | |||
| 974 | cross_task: | ||
| 975 | scx_error(sch, "select_cpu kfunc called cross-task on %s[%d]", | ||
| 976 | p->comm, p->pid); | ||
| 977 | return -EINVAL; | ||
| 963 | } | 978 | } |
| 964 | 979 | ||
| 965 | /** | 980 | /** |