On Tue, Feb 27, 2024 at 5:36 PM Eduard Zingerman eddyz87@gmail.com wrote:
On Tue, 2024-02-27 at 17:18 +0100, Benjamin Tissoires wrote: [...]
Hmm, I must still be missing a piece of the puzzle: if I declare bpf_timer_set_sleepable_cb() to take a third "aux" argument, given that it is declared as kfunc, I also must declare it in my bpf program, or I get the following:
# libbpf: extern (func ksym) 'bpf_timer_set_sleepable_cb': func_proto [264] incompatible with vmlinux [18151]
And if I declare it, then I don't know what to pass, given that this is purely added by the verifier:
43: (85) call bpf_timer_set_sleepable_cb#18152 arg#2 pointer type STRUCT bpf_prog_aux must point to scalar, or struct with scalar
Right, something has to be done about number of arguments and we don't have a convenient mechanism for this afaik.
The simplest way would be to have two kfuncs:
- one with 2 arguments, used form bpf program;
- another with 3 arguments, used at runtime;
- replace former by latter during rewrite.
It's hacky but seems interesting enough to be tested :)
Maybe I should teach the verifier that this kfunc only takes 2 arguments, and the third one is virtual, but that also means that when the kfunc definitions are to be included in vmlinux.h, they would also have this special case.
It might be a somewhat generic mechanism, e.g. btf_decl_tag("hidden") for kfunc parameter.
We also could use the suffix (like __uninit, __k, etc...), but it might introduce more headaches than the 2 kfuncs you are proposing.
imho, having two kfuncs is less hacky.
(I just tried with a blank u64 instead of the struct bpf_prog_aux*, but it crashes with KASAN complaining).
For my understanding:
- you added a 3rd param (void *) to kfunc;
it was struct bpf_prog_aux *, but yes
- passed it as zero in BPF program;
- applied the above rewrite, so that r3 equals to prog->aux;
- and now KASAN complains, right?
yep, but see below
Could you please provide more details on what exactly it complains about?
Well, there is a simple reason: that code is never reached because, in that function, there is a `if (insn->src_reg == BPF_PSEUDO_KFUNC_CALL)` above that unconditionally terminates with a `continue`. So basically this part of the code is never hit.
I'll include that new third argument and the dual kfunc call in fixup_kfunc_call() and report if it works from here.
Cheers, Benjamin