search

dthaler / ebpf-docs

11 stars 0 forks source link

BPF_CALL for eBPF functions incorrectly refers to offset instead of imm #11

Open Alan-Jowett opened 1 year ago

Alan-Jowett commented 1 year ago

https://github.com/dthaler/ebpf-docs/blob/update/isa/kernel.org/instruction-set.rst#jump-instructions https://github.com/dthaler/ebpf-docs/blob/update/isa/kernel.org/instruction-set.rst#ebpf-functions

BPF call instruction with src == 1 should pass the offset to the BPF function via the immediate field and not the offset field.

Alan-Jowett commented 1 year ago

Found while fixing https://github.com/Alan-Jowett/bpf_conformance/issues/60

yesh0 commented 1 year ago

By the way, one may verify the statement against the Linux implementation in its verifier, which uses insn->imm as the offset:

https://github.com/torvalds/linux/blob/ef4d3ea40565a781c25847e9cb96c1bd9f462bc6/kernel/bpf/verifier.c#L2198-L2199

if (bpf_pseudo_func(insn) || bpf_pseudo_call(insn))
    ret = add_subprog(env, i + insn->imm + 1);
Alan-Jowett commented 1 year ago

Thanks @yesh0 I appreciate the feedback. Unfortunately, due to license conflicts, I can't look at the Linux eBPF code (I work on a MIT licensed BPF runtime). The approach I have been taking is to use a set of tests in bpf_conformance to measure the behavior of Linux (and verify that other BPF implementations match the Linux behavior).

While implementing the corresponding BPF assembler (using Intel style assembly) for this test, I noticed that setting the offset in the offset field didn't work, hence filing this issue to get the doc updated.

We are also using the same test suites to verify that the behavior of other BPF implementations is correct, including uBPF, rbpf, prevail verifier, and bpf2c.