On Wed, Apr 20, 2022 at 4:40 AM Jakub Sitnicki <jakub@xxxxxxxxxxxxxx> wrote:
>
> Hi Alexei,
>
> On Wed, Dec 01, 2021 at 10:10 AM -08, Alexei Starovoitov wrote:
> > From: Alexei Starovoitov <ast@xxxxxxxxxx>
> >
> > struct bpf_core_relo is generated by llvm and processed by libbpf.
> > It's a de-facto uapi.
> > With CO-RE in the kernel the struct bpf_core_relo becomes uapi de-jure.
> > Add an ability to pass a set of 'struct bpf_core_relo' to prog_load command
> > and let the kernel perform CO-RE relocations.
> >
> > Note the struct bpf_line_info and struct bpf_func_info have the same
> > layout when passed from LLVM to libbpf and from libbpf to the kernel
> > except "insn_off" fields means "byte offset" when LLVM generates it.
> > Then libbpf converts it to "insn index" to pass to the kernel.
> > The struct bpf_core_relo's "insn_off" field is always "byte offset".
> >
> > Acked-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
> > Signed-off-by: Alexei Starovoitov <ast@xxxxxxxxxx>
> > ---
> > include/linux/bpf.h | 8 ++++
> > include/uapi/linux/bpf.h | 59 +++++++++++++++++++++++++-
> > kernel/bpf/btf.c | 6 +++
> > kernel/bpf/syscall.c | 2 +-
> > kernel/bpf/verifier.c | 76 ++++++++++++++++++++++++++++++++++
> > tools/include/uapi/linux/bpf.h | 59 +++++++++++++++++++++++++-
> > tools/lib/bpf/relo_core.h | 53 ------------------------
> > 7 files changed, 207 insertions(+), 56 deletions(-)
> >
> > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > index cad0829710be..8bbf08fbab66 100644
> > --- a/include/linux/bpf.h
> > +++ b/include/linux/bpf.h
> > @@ -1732,6 +1732,14 @@ bool bpf_prog_has_kfunc_call(const struct bpf_prog *prog);
> > const struct btf_func_model *
> > bpf_jit_find_kfunc_model(const struct bpf_prog *prog,
> > const struct bpf_insn *insn);
> > +struct bpf_core_ctx {
> > + struct bpf_verifier_log *log;
> > + const struct btf *btf;
> > +};
> > +
> > +int bpf_core_apply(struct bpf_core_ctx *ctx, const struct bpf_core_relo *relo,
> > + int relo_idx, void *insn);
> > +
> > #else /* !CONFIG_BPF_SYSCALL */
> > static inline struct bpf_prog *bpf_prog_get(u32 ufd)
> > {
> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > index 9e66b1880020..c26871263f1f 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -1342,8 +1342,10 @@ union bpf_attr {
> > /* or valid module BTF object fd or 0 to attach to vmlinux */
> > __u32 attach_btf_obj_fd;
> > };
> > - __u32 :32; /* pad */
> > + __u32 core_relo_cnt; /* number of bpf_core_relo */
> > __aligned_u64 fd_array; /* array of FDs */
> > + __aligned_u64 core_relos;
> > + __u32 core_relo_rec_size; /* sizeof(struct bpf_core_relo) */
> > };
> >
> > struct { /* anonymous struct used by BPF_OBJ_* commands */
>
> I think I've spotted a breakage.
>
> Plugging the 4 byte hole with core_relo_cnt means that programs built
> against < v5.17 headers pass garbage as core_relo_cnt value.
>
> That in turn makes check_core_relo() fail with -EINVAL, which fails
> PROG_LOAD.
bpf_attr is supposed to be zeroed out with memset(), so that hole
should have zero even before core_relo_cnt was added
>
> [...]
