ARRAY_ELEM_PTR() is an access macro used to help the BPF verifier not
confused by offseted memory acceeses by yiedling a valid pointer or NULL in
a way that's clear to the verifier. As such, the canonical usage involves
checking NULL return from the macro. Note that in many cases, the NULL
condition can never happen - they're there just to hint the verifier.
In a bpf_loop in scx_central.bpf.c::central_dispatch(), the NULL check was
incorrect in that there was another dereference of the pointer in addition
to the NULL checked access. This worked as the pointer can never be NULL and
the verifier could tell it would never be NULL in this case.
However, this still looks wrong and trips smatch:
./tools/sched_ext/scx_central.bpf.c:205 ____central_dispatch()
error: we previously assumed 'gimme' could be null (see line 201)
./tools/sched_ext/scx_central.bpf.c
195
196 if (!scx_bpf_dispatch_nr_slots())
197 break;
198
199 /* central's gimme is never set */
200 gimme = ARRAY_ELEM_PTR(cpu_gimme_task, cpu, nr_cpu_ids);
201 if (gimme && !*gimme)
^^^^^
If gimme is NULL
202 continue;
203
204 if (dispatch_to_cpu(cpu))
--> 205 *gimme = false;
Fix the NULL check so that there are no derefs if NULL. This doesn't change
actual behavior.
Signed-off-by: Tejun Heo <tj@xxxxxxxxxx>
Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
Link: http://lkml.kernel.org/r/<955e1c3c-ace2-4a1d-b246-15b8196038a3@stanley.mountain>
---
Aplying to sched_ext/for-6.12.
Thanks.
tools/sched_ext/scx_central.bpf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/sched_ext/scx_central.bpf.c b/tools/sched_ext/scx_central.bpf.c
index 1d8fd570eaa7..8dd8eb73b6b8 100644
--- a/tools/sched_ext/scx_central.bpf.c
+++ b/tools/sched_ext/scx_central.bpf.c
@@ -198,7 +198,7 @@ void BPF_STRUCT_OPS(central_dispatch, s32 cpu, struct task_struct *prev)
/* central's gimme is never set */
gimme = ARRAY_ELEM_PTR(cpu_gimme_task, cpu, nr_cpu_ids);
- if (gimme && !*gimme)
+ if (!gimme || !*gimme)
continue;
if (dispatch_to_cpu(cpu))
--
2.46.0
