On Tue, 8 Oct 2024 at 12:35, Toke Høiland-Jørgensen <toke@xxxxxxxxxx> wrote:
>
> The verifier contains a cache for looking up module BTF objects when
> calling kfuncs defined in modules. This cache uses a 'struct
> bpf_kfunc_btf_tab', which contains a sorted list of BTF objects that
> were already seen in the current verifier run, and the BTF objects are
> looked up by the offset stored in the relocated call instruction using
> bsearch().
>
> The first time a given offset is seen, the module BTF is loaded from the
> file descriptor passed in by libbpf, and stored into the cache. However,
> there's a bug in the code storing the new entry: it stores a pointer to
> the new cache entry, then calls sort() to keep the cache sorted for the
> next lookup using bsearch(), and then returns the entry that was just
> stored through the stored pointer. However, because sort() modifies the
> list of entries in place *by value*, the stored pointer may no longer
> point to the right entry, in which case the wrong BTF object will be
> returned.
>
> The end result of this is an intermittent bug where, if a BPF program
> calls two functions with the same signature in two different modules,
> the function from the wrong module may sometimes end up being called.
> Whether this happens depends on the order of the calls in the BPF
> program (as that affects whether sort() reorders the array of BTF
> objects), making it especially hard to track down. Simon, credited as
> reporter below, spent significant effort analysing and creating a
> reproducer for this issue. The reproducer is added as a selftest in a
> subsequent patch.
>
> The fix is straight forward: simply don't use the stored pointer after
> calling sort(). Since we already have an on-stack pointer to the BTF
> object itself at the point where the function return, just use that, and
> populate it from the cache entry in the branch where the lookup
> succeeds.
>
> Fixes: 2357672c54c3 ("bpf: Introduce BPF support for kernel module function calls")
> Reported-by: Simon Sundberg <simon.sundberg@xxxxxx>
> Signed-off-by: Toke Høiland-Jørgensen <toke@xxxxxxxxxx>
> ---
Acked-by: Kumar Kartikeya Dwivedi <memxor@xxxxxxxxx>
