KP Singh [off-list ref] writes:

[...]

quoted

The above code gets generated per-program and exists out-of-tree in a
very unreadable format in it's final form. I have general objections to
being forced to "trust" out-of-tree code, when it's demostrably trivial

This is not out of tree. It's very much within the kernel tree.

No, it's not.

Running something like

bpftool gen skeleton -S -k <private_key> -i <identity_cert>
fentry_test.bpf.o

will yield a header file fentery_test.h or whatever. That header file
contains a customized and one-off version of the templated code in this
patch. That header file and the resultant loader it gets compiled into
exists out-of-tree.

quoted

to perform this check in-kernel, without impeding any of the other
stated use cases. There is no possible audit log nor LSM hook for these
operations. There is no way to know that this check was ever performed.

Further, this check ends up happeing in an entirely different syscall,
the LSM layer and the end user may both see invalid programs successfully
being loaded into the kernel, that may fail mysteriously later.

Also, this patch seems to rely on hacking into struct internals and
magic binary layouts.

These magical binary layouts are BPF programs, as I mentioned, if you
don't like this you (i.e an advanced user like Microsoft) can
implement your own trusted loader in whatever format you like. We are
not forcing you.

If you really want to do it in the kernel, you can do it out of tree
and maintain these patches (that's what "out of tree" actually means),
this is not a direction the BPF maintainers are interested in as it
does not meet the broader community's use-cases. We don’t want an
unnecessary extension to the UAPI when some BPF programs do have
stable instructions already (e.g. network) and some that can
potentially have someday.

Yes, you are forcing us. Saying we are only allowed to use "trusted"
loaders, and that no one is allowed to have any in-kernel, in-tree code
that inspects user inputs or target programs directly is very
non-consentual on my end. This is a design mandate, being forced upon
other people, by you, with no concrete reasons, other than vague statements
around UAPI design, need or necessity.

-blaise

RE The struct internals will be replaced by calling BPF_OBJ_GET_INFO
directly from the loader program as I mentioned in the commit.”


- KP

quoted

-blaise

quoted

 void bpf_gen__record_attach_target(struct bpf_gen *gen, const char *attach_name,
                                 enum bpf_attach_type type)
 {

diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index b6ee9870523a..084372fa54f4 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h

@@ -1803,9 +1803,10 @@ struct gen_loader_opts {
      const char *insns;
      __u32 data_sz;
      __u32 insns_sz;
+     bool gen_hash;
 };

-#define gen_loader_opts__last_field insns_sz
+#define gen_loader_opts__last_field gen_hash
 LIBBPF_API int bpf_object__gen_loader(struct bpf_object *obj,
                                    struct gen_loader_opts *opts);

--

2.43.0