On Tue, 2022-12-06 at 14:49 +0100, Björn Töpel wrote:
> Ilya Leoshkevich <iii@xxxxxxxxxxxxx> writes:
>
> > On Fri, 2022-12-02 at 11:36 +0100, Björn Töpel wrote:
> > > From: Björn Töpel <bjorn@xxxxxxxxxxxx>
> > >
> > > A BPF call instruction can be, correctly, marked with zext_dst
> > > set to
> > > true. An example of this can be found in the BPF selftests
> > > progs/bpf_cubic.c:
> > >
> > > ...
> > > extern __u32 tcp_reno_undo_cwnd(struct sock *sk) __ksym;
> > >
> > > __u32 BPF_STRUCT_OPS(bpf_cubic_undo_cwnd, struct sock *sk)
> > > {
> > > return tcp_reno_undo_cwnd(sk);
> > > }
> > > ...
> > >
> > > which compiles to:
> > > 0: r1 = *(u64 *)(r1 + 0x0)
> > > 1: call -0x1
> > > 2: exit
> > >
> > > The call will be marked as zext_dst set to true, and for some
> > > backends
> > > (bpf_jit_needs_zext() returns true) expanded to:
> > > 0: r1 = *(u64 *)(r1 + 0x0)
> > > 1: call -0x1
> > > 2: w0 = w0
> > > 3: exit
> >
> > In the verifier, the marking is done by check_kfunc_call() (added
> > in
> > e6ac2450d6de), right? So the problem occurs only for kfuncs?
>
> I've only seen it for kfuncs, yes.
>
> >
> > /* Check return type */
> > t = btf_type_skip_modifiers(desc_btf, func_proto->type,
> > NULL);
> >
> > ...
> >
> > if (btf_type_is_scalar(t)) {
> > mark_reg_unknown(env, regs, BPF_REG_0);
> > mark_btf_func_reg_size(env, BPF_REG_0, t->size);
> >
> > I tried to find some official information whether the eBPF calling
> > convention requires sign- or zero- extending return values and
> > arguments, but unfortunately [1] doesn't mention this.
> >
> > LLVM's lib/Target/BPF/BPFCallingConv.td mentions both R* and W*
> > registers, but since assigning to W* leads to zero-extension, it
> > seems
> > to me that this is the case.
> >
> > If the above is correct, then shouldn't we rather use sizeof(void
> > *) in
> > the mark_btf_func_reg_size() call above?
>
> Hmm, or rather sizeof(u64) if I'm reading you correctly?
Whoops, you are right - that's indeed what I meant here.
