On Wed, Aug 2, 2023 at 10:06 AM David Vernet <void@xxxxxxxxxxxxx> wrote:
>
> On Wed, Aug 02, 2023 at 09:33:18AM -0700, Alexei Starovoitov wrote:
> > On Tue, Aug 1, 2023 at 8:30 PM David Vernet <void@xxxxxxxxxxxxx> wrote:
> > > I agree that this is the correct way to generalize this. The only thing
> > > that we'll have to figure out is how to generalize treating const struct
> > > cpumask * objects as kptrs. In sched_ext [0] we export
> > > scx_bpf_get_idle_cpumask() and scx_bpf_get_idle_smtmask() kfuncs to
> > > return trusted global cpumask kptrs that can then be "released" in
> > > scx_bpf_put_idle_cpumask(). scx_bpf_put_idle_cpumask() is empty and
> > > exists only to appease the verifier that the trusted cpumask kptrs
> > > aren't being leaked and are having their references "dropped".
> >
> > why is it KF_ACQUIRE ?
> > I think it can just return a trusted pointer without acquire.
>
> I don't think there's a way to do this yet without hard-coding the kfuncs as
> special, right? That's why we do stuff like this:
>
>
> 11479 } else if (meta.func_id == special_kfunc_list[KF_bpf_cast_to_kern_ctx]) {
> 11480 mark_reg_known_zero(env, regs, BPF_REG_0);
> 11481 regs[BPF_REG_0].type = PTR_TO_BTF_ID | PTR_TRUSTED;
> 11482 regs[BPF_REG_0].btf = desc_btf;
> 11483 regs[BPF_REG_0].btf_id = meta.ret_btf_id;
>
> We could continue to do that, but I wonder if it would be useful to add
> a kfunc flag that allowed a kfunc to specify that? Something like
> KF_ALWAYS_TRUSTED? In general though, yes, we can teach the verifier to
> not require KF_ACQUIRE if we want. It's just that what we have now
> doesn't really scale to the kernel for any global cpumask.
We should probably just do this:
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index e7b1af016841..f0c3f69ee5a8 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -11567,7 +11567,7 @@ static int check_kfunc_call(struct
bpf_verifier_env *env, struct bpf_insn *insn,
} else {
mark_reg_known_zero(env, regs, BPF_REG_0);
regs[BPF_REG_0].btf = desc_btf;
- regs[BPF_REG_0].type = PTR_TO_BTF_ID;
+ regs[BPF_REG_0].type = PTR_TO_BTF_ID | PTR_TRUSTED;
regs[BPF_REG_0].btf_id = ptr_type_id;
A quick audit for kfuncs shows that this code is never used.
Everywhere where ptr_to_btf is returned the kfunc is maked with KF_ACQUIRE.
We'd need to have careful code review to make sure future kfuncs
return trusted pointers.
That would simplify scx_bpf_get_idle_cpumask(). No need for get and nop put.
