Re: [RFC PATCH 3/5] ebpf: Added declaration/initialization routines.

[Jason Wang]

On Thu, Mar 30, 2023 at 4:34 PM Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> On Thu, Mar 30, 2023 at 02:54:32PM +0800, Jason Wang wrote:
> > On Thu, Mar 30, 2023 at 8:33 AM Andrew Melnychenko <andrew@daynix.com> 
> > wrote:
> > >
> > > Now, the binary objects may be retrieved by id/name.
> > > It would require for future qmp commands that may require specific
> > > eBPF blob.
> > >
> > > Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
> > > ---
> > >  ebpf/ebpf.c      | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
> > >  ebpf/ebpf.h      | 25 +++++++++++++++++++++++++
> > >  ebpf/ebpf_rss.c  |  4 ++++
> > >  ebpf/meson.build |  1 +
> > >  4 files changed, 78 insertions(+)
> > >  create mode 100644 ebpf/ebpf.c
> > >  create mode 100644 ebpf/ebpf.h
> > >
> > > diff --git a/ebpf/ebpf.c b/ebpf/ebpf.c
> > > new file mode 100644
> > > index 0000000000..86320d72f5
> > > --- /dev/null
> > > +++ b/ebpf/ebpf.c
> > > @@ -0,0 +1,48 @@
> > > +/*
> > > + * QEMU eBPF binary declaration routine.
> > > + *
> > > + * Developed by Daynix Computing LTD (http://www.daynix.com)
> > > + *
> > > + * Authors:
> > > + *  Andrew Melnychenko <andrew@daynix.com>
> > > + *
> > > + * This work is licensed under the terms of the GNU GPL, version 2 or
> > > + * later.  See the COPYING file in the top-level directory.
> > > + */
> > > +
> > > +#include "qemu/osdep.h"
> > > +#include "qemu/queue.h"
> > > +#include "ebpf/ebpf.h"
> > > +
> > > +struct ElfBinaryDataEntry {
> > > +    const char *id;
> > > +    const void * (*fn)(size_t *);
> > > +
> > > +    QSLIST_ENTRY(ElfBinaryDataEntry) node;
> > > +};
> > > +
> > > +static QSLIST_HEAD(, ElfBinaryDataEntry) ebpf_elf_obj_list =
> > > +                                            QSLIST_HEAD_INITIALIZER();
> > > +
> > > +void ebpf_register_binary_data(const char *id, const void * (*fn)(size_t 
> > > *))
> > > +{
> > > +    struct ElfBinaryDataEntry *data = NULL;
> > > +
> > > +    data = g_malloc0(sizeof(*data));
> > > +    data->fn = fn;
> > > +    data->id = id;
> > > +
> > > +    QSLIST_INSERT_HEAD(&ebpf_elf_obj_list, data, node);
> > > +}
> > > +
> > > +const void *ebpf_find_binary_by_id(const char *id, size_t *sz)
> > > +{
> > > +    struct ElfBinaryDataEntry *it = NULL;
> > > +    QSLIST_FOREACH(it, &ebpf_elf_obj_list, node) {
> > > +        if (strcmp(id, it->id) == 0) {
> > > +            return it->fn(sz);
> > > +        }
> > > +    }
> > > +
> > > +    return NULL;
> > > +}
> > > diff --git a/ebpf/ebpf.h b/ebpf/ebpf.h
> > > new file mode 100644
> > > index 0000000000..fd705cb73e
> > > --- /dev/null
> > > +++ b/ebpf/ebpf.h
> > > @@ -0,0 +1,25 @@
> > > +/*
> > > + * QEMU eBPF binary declaration routine.
> > > + *
> > > + * Developed by Daynix Computing LTD (http://www.daynix.com)
> > > + *
> > > + * Authors:
> > > + *  Andrew Melnychenko <andrew@daynix.com>
> > > + *
> > > + * This work is licensed under the terms of the GNU GPL, version 2 or
> > > + * later.  See the COPYING file in the top-level directory.
> > > + */
> > > +
> > > +#ifndef EBPF_H
> > > +#define EBPF_H
> > > +
> > > +void ebpf_register_binary_data(const char *id, const void * (*fn)(size_t 
> > > *));
> > > +const void *ebpf_find_binary_by_id(const char *id, size_t *sz);
> > > +
> > > +#define ebpf_binary_init(id, fn)                                         
> > >   \
> > > +static void __attribute__((constructor)) ebpf_binary_init_ ## fn(void)   
> > >   \
> > > +{                                                                        
> > >   \
> > > +    ebpf_register_binary_data(id, fn);                                   
> > >   \
> > > +}
> > > +
> > > +#endif /* EBPF_H */
> > > diff --git a/ebpf/ebpf_rss.c b/ebpf/ebpf_rss.c
> > > index 08015fecb1..b4038725f2 100644
> > > --- a/ebpf/ebpf_rss.c
> > > +++ b/ebpf/ebpf_rss.c
> > > @@ -21,6 +21,8 @@
> > >
> > >  #include "ebpf/ebpf_rss.h"
> > >  #include "ebpf/rss.bpf.skeleton.h"
> > > +#include "ebpf/ebpf.h"
> > > +
> > >  #include "trace.h"
> > >
> > >  void ebpf_rss_init(struct EBPFRSSContext *ctx)
> > > @@ -237,3 +239,5 @@ void ebpf_rss_unload(struct EBPFRSSContext *ctx)
> > >      ctx->obj = NULL;
> > >      ctx->program_fd = -1;
> > >  }
> > > +
> > > +ebpf_binary_init("rss", rss_bpf__elf_bytes)
> >
> > Who or how the ABI compatibility is preserved between libvirt and Qemu?
>
> There's no real problem with binary compatibility to solve any more.
>
> When libvirt first launches a QEMU VM, it will fetch the eBPF programs
> it needs from that running QEMU using QMP. WHen it later needs to
> enable features that use eBPF, it already has the program data that
> matches the running QEMU

Ok, then who will validate the eBPF program? I don't think libvirt can
trust what is received from Qemu otherwise arbitrary eBPF programs
could be executed by Qemu in this way. One example is that when guests
escape to Qemu it can modify the rss_bpf__elf_bytes. Though
BPF_PROG_TYPE_SOCKET_FILTER gives some of the restrictions, we still
need to evaluate side effects of this. Or we need to find other ways
like using the binary in libvirt or use rx filter events.

Thanks

>
>
> With regards,
> Daniel
> --
> |: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-            https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
>