Re: [PATCH sched_ext/for-7.3 03/32] sched_ext: Use READ_ONCE/WRITE_ONCE in cmask word ops and drop _RACY variants

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] Missing READ_ONCE() in cmask_word_op1 leaves scx_cmask_empty() and the boundary checks of scx_cmask_subset() vulnerable to data races.

Pre-existing issues:
- [Critical] The kernel trusts inline metadata (nr_cids, base) from BPF arena cmasks without bounds checking, allowing userspace or BPF to trigger massive out-of-bounds zeroing of kernel memory.
--

commit dff79ba0834e741aea85b2036409e261619d8a3c
Author: Tejun Heo <tj@kernel.org>

sched_ext: Use READ_ONCE/WRITE_ONCE in cmask word ops and drop _RACY variants
    
The cmask ops can operate on BPF-arena cmasks which BPF programs may read
and write concurrently. The _RACY op variants existed to make such lockless
reads explicit but this turned out to be too restrictive. Mark the word
accesses in all the two-cmask ops with READ_ONCE/WRITE_ONCE instead and drop
the _RACY variants.

> diff --git a/kernel/sched/ext/cid.c b/kernel/sched/ext/cid.c
> index af83084ec740a..4b7279ca740f4 100644
> --- a/kernel/sched/ext/cid.c
> +++ b/kernel/sched/ext/cid.c
[ ... ]
> @@ -422,28 +420,22 @@ static __always_inline bool cmask_word_op2(u64 *av, const u64 *bp, u64 mask,
>  {
>  	switch (op) {
>  	case CMASK_OP2_AND:
> -		*av &= ~mask | *bp;
> +		WRITE_ONCE(*av, *av & (~mask | READ_ONCE(*bp)));
>  		return false;
>  	case CMASK_OP2_OR:
> -		*av |= *bp & mask;
> -		return false;
> -	case CMASK_OP2_OR_RACY:
> -		*av |= data_race(*bp) & mask;
> +		WRITE_ONCE(*av, *av | (READ_ONCE(*bp) & mask));
>  		return false;
>  	case CMASK_OP2_COPY:
> -		*av = (*av & ~mask) | (*bp & mask);
> -		return false;
> -	case CMASK_OP2_COPY_RACY:
> -		*av = (*av & ~mask) | (data_race(*bp) & mask);
> +		WRITE_ONCE(*av, (*av & ~mask) | (READ_ONCE(*bp) & mask));
>  		return false;
>  	case CMASK_OP2_ANDNOT:
> -		*av &= ~(*bp & mask);
> +		WRITE_ONCE(*av, *av & ~(READ_ONCE(*bp) & mask));
>  		return false;
>  	case CMASK_OP2_SUBSET:
>  		/* stop on the first bit in @sub not set in @super */
> -		return (*bp & ~*av) & mask;
> +		return (READ_ONCE(*bp) & ~READ_ONCE(*av)) & mask;
>  	case CMASK_OP2_INTERSECTS:
> -		return (*av & *bp) & mask;
> +		return (READ_ONCE(*av) & READ_ONCE(*bp)) & mask;
>  	}
>  	unreachable();
>  }

[Severity: Medium]
Since READ_ONCE() was added to cmask_word_op2() to prevent data races and load
tearing when reading BPF-arena cmasks locklessly, shouldn't it also be added to
cmask_word_op1() in kernel/sched/ext/cid.c?

For example, when scx_cmask_subset() checks the boundaries:

    if (sub->base < super->base &&
        cmask_any_set_in_range(sub, sub->base, min(super->base, sub_end)))
        return false;

This triggers a lockless cmask_word_op1() on sub, which can be an arena cmask
mutated by BPF:

    case CMASK_OP1_ANY_SET:
        return *ap & mask;

Could this unprotected read leave scx_cmask_empty() and the boundary checks of
scx_cmask_subset() vulnerable to data races?

[ ... ]
> diff --git a/kernel/sched/ext/cid.h b/kernel/sched/ext/cid.h
> index 9c4f4b907f12d..54b10df32fd50 100644
> --- a/kernel/sched/ext/cid.h
> +++ b/kernel/sched/ext/cid.h
> @@ -57,9 +57,7 @@ void scx_cmask_clear(struct scx_cmask *m);

[Severity: Critical]
This is a pre-existing issue, but does scx_cmask_clear() blindly trust inline
metadata from BPF arena cmasks without bounds checking?

When the kernel allocates set_cmask_scratch in the BPF arena using
scx_arena_alloc(), and later calls scx_cpumask_to_cmask() in
kernel/sched/ext/ext.c:scx_call_op_set_cpumask():

    struct scx_cmask *kern_va = *this_cpu_ptr(sch->set_cmask_scratch);
    ...
    scx_cpumask_to_cmask(cpumask, kern_va);

This eventually calls scx_cmask_clear(kern_va) in kernel/sched/ext/cid.c, which
calculates the memset size based on the cmask's inline fields:

    nr_words = (m->base + m->nr_cids - 1) / 64 - m->base / 64 + 1;
    memset(m->bits, 0, nr_words * sizeof(u64));

If an untrusted BPF program or userspace process concurrently overwrites
m->nr_cids or m->base with a massive value, could this trigger an out-of-bounds
memset that zeroes out kernel memory adjacent to the arena allocation?

>  void scx_cmask_fill(struct scx_cmask *m);
>  void scx_cmask_and(struct scx_cmask *dst, const struct scx_cmask *src);
>  void scx_cmask_or(struct scx_cmask *dst, const struct scx_cmask *src);
> -void scx_cmask_or_racy(struct scx_cmask *dst, const struct scx_cmask *src);
>  void scx_cmask_copy(struct scx_cmask *dst, const struct scx_cmask *src);
> -void scx_cmask_copy_racy(struct scx_cmask *dst, const struct scx_cmask *src);
>  void scx_cmask_andnot(struct scx_cmask *dst, const struct scx_cmask *src);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260703080159.2314350-1-tj@kernel.org?part=3