On 16-Jan 11:10, Andrii Nakryiko wrote:

On Thu, Jan 16, 2020 at 4:49 AM KP Singh [off-list ref] wrote:

quoted

Thanks for the review Andrii!

I will incorporate the fixes in the next revision.

On 15-Jan 13:19, Andrii Nakryiko wrote:

quoted

On Wed, Jan 15, 2020 at 9:13 AM KP Singh [off-list ref] wrote:

quoted

From: KP Singh <redacted>

* Add functionality in libbpf to attach eBPF program to LSM hooks
* Lookup the index of the LSM hook in security_hook_heads and pass it in
  attr->lsm_hook_index

Signed-off-by: KP Singh <redacted>
---
 tools/lib/bpf/bpf.c      |   6 +-
 tools/lib/bpf/bpf.h      |   1 +
 tools/lib/bpf/libbpf.c   | 143 ++++++++++++++++++++++++++++++++++-----
 tools/lib/bpf/libbpf.h   |   4 ++
 tools/lib/bpf/libbpf.map |   3 +
 5 files changed, 138 insertions(+), 19 deletions(-)

[...]

quoted

quoted

quoted

+{
+       struct btf *btf = bpf_find_kernel_btf();

ok, it's probably time to do this right. Let's ensure we load kernel
BTF just once, keep it inside bpf_object while we need it and then
release it after successful load. We are at the point where all the
new types of program is loading/releasing kernel BTF for every section
and it starts to feel very wasteful.

Sure, will give it a shot in v3.

thanks!

[...]

quoted

quoted

quoted

+               if (!strcmp(btf__name_by_offset(btf, m->name_off), name))
+                       return j + 1;

I looked briefly through kernel-side patch introducing lsm_hook_index,
but it didn't seem to explain why this index needs to be (unnaturally)
1-based. So asking here first as I'm looking through libbpf changes?

The lsm_hook_idx is one-based as it makes it easy to validate the
input. If we make it zero-based it's hard to check if the user
intended to attach to the LSM hook at index 0 or did not set it.

Think about providing FDs. 0 is a valid, though rarely
intended/correct value. Yet we don't make all FD arguments
artificially 1-based, right? This extra +1/-1 translation just makes
for more confusing interface, IMO. If user "accidentally" guessed type
signature of very first hook, well, so be it... If not, BPF verifier
will politely refuse. Seems like enough protection.

Thanks! I see your point and will update to using the
more-conventional 0-based indexing for the next revision.

- KP

quoted

We are then up to the verifier to reject the loaded program which
may or may not match the signature of the hook at lsm_hook_idx = 0.

I will clarify this in the commit log as well.

[...]