Re: [PATCH v4 sched_ext/for-7.3 08/40] sched_ext: Reject direct slice and dsq_vtime writes for cid-form schedulers

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

Pre-existing issues:
- [Critical] bpf_scx_btf_struct_access_common permits BPF_WRITE to PTR_TO_BTF_ID | PTR_UNTRUSTED pointers, allowing arbitrary kernel memory writes.
--

--- Patch [8]: [PATCH v4 sched_ext/for-7.3 08/40] sched_ext: Reject direct slice and dsq_vtime writes for cid-form schedulers ---
commit b5bf42bcb4f4546f3eb981870b33d4470cf01c02
Author: Tejun Heo <[email protected]>

sched_ext: Reject direct slice and dsq_vtime writes for cid-form schedulers

This commit introduces a separate verifier ops struct for cid-form schedulers
to reject direct writes to p->scx.slice and p->scx.dsq_vtime. The cpu-form
schedulers retain direct writes and drop the previous deprecation warning.

> diff --git a/kernel/sched/ext/ext.c b/kernel/sched/ext/ext.c
> index be75bdc9ada23..bd3cb59b1dbf9 100644
> --- a/kernel/sched/ext/ext.c
> +++ b/kernel/sched/ext/ext.c
> @@ -6995,6 +6995,21 @@ static bool bpf_scx_is_valid_access(int off, int size,
>  	return btf_ctx_access(off, size, type, prog, info);
>  }
>  
> +/* common to both forms: only scx.disallow is writable */
> +static int bpf_scx_btf_struct_access_common(const struct bpf_reg_state *reg,
> +					    int off, int size)
> +{
> +	const struct btf_type *t;
> +
> +	t = btf_type_by_id(reg->btf, reg->btf_id);
> +	if (t == task_struct_type &&
> +	    off >= offsetof(struct task_struct, scx.disallow) &&
> +	    off + size <= offsetofend(struct task_struct, scx.disallow))
> +		return SCALAR_VALUE;

[Severity: Critical]
This is a pre-existing issue, but does this function permit arbitrary kernel
memory writes if the register has the PTR_UNTRUSTED flag set?

If an attacker loads a BPF STRUCT_OPS program and forges an arbitrary address
into a PTR_TO_BTF_ID | PTR_UNTRUSTED pointer, they can attempt to write to
the scx.disallow field of this pointer. The BPF verifier's
check_ptr_to_btf_access unconditionally calls this btf_struct_access callback
for BPF_WRITE if the pointer is not program-allocated.

Since this callback returns SCALAR_VALUE without verifying the PTR_UNTRUSTED
flag on reg->type, the write is approved. The JIT will then emit a direct
BPF_MEM store instruction that silently overwrites the target address because
fixups skip BPF_PROBE_MEM conversion for BPF_WRITE on untrusted pointers.

Should we explicitly check for the PTR_UNTRUSTED flag in reg->type here?

	if (type_flag(reg->type) & PTR_UNTRUSTED)
		return -EACCES;

> +
> +	return -EACCES;
> +}
> +
>  static int bpf_scx_btf_struct_access(struct bpf_verifier_log *log,
>  				     const struct bpf_reg_state *reg, int off,
>  				     int size)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=8