Re: [PATCH bpf-next v8 0/8] MAC and Audit policy using eBPF (KRSI)

On 28-Mar 23:30, KP Singh wrote:

quoted

On Sat, Mar 28, 2020 at 10:50 PM Kees Cook [off-list ref] wrote:

quoted

On Sat, Mar 28, 2020 at 08:56:36PM +0100, KP Singh wrote:

quoted

Since the attachment succeeds and the hook does not get called, it
seems like "bpf" LSM is not being initialized and the hook, although
present, does not get called.

This indicates that "bpf" is not in CONFIG_LSM. It should, however, be
there by default as we added it to default value of CONFIG_LSM and
also for other DEFAULT_SECURITY_* options.

Let me know if that's the case and it fixes it.

Is the selftest expected to at least fail cleanly (i.e. not segfault)

I am not sure where the crash comes from, it does not look like it's test_lsm,
it seems to happen in test_overhead. Both seem to run fine for me.

So I was able to reproduce the crash:

* Remove "bpf" from CONFIG_LSM

./test_progs -n 66,67
test_test_lsm:PASS:skel_load 0 nsec
test_test_lsm:PASS:attach 0 nsec
test_test_lsm:PASS:exec_cmd 0 nsec
test_test_lsm:FAIL:bprm_count bprm_count = 0
test_test_lsm:FAIL:heap_mprotect want errno=EPERM, got 0
#66 test_lsm:FAIL
Caught signal #11!
Stack trace:
./test_progs(crash_handler+0x1f)[0x55b7f9867acf]
/lib/x86_64-linux-gnu/libpthread.so.0(+0x13520)[0x7fcf1467e520]
/lib/x86_64-linux-gnu/libc.so.6(+0x15f73d)[0x7fcf1460a73d]
/lib/x86_64-linux-gnu/libc.so.6(__libc_calloc+0x2ca)[0x7fcf1453286a]
/usr/lib/x86_64-linux-gnu/libelf.so.1(+0x37

[snip]

* The crash went away when I removed the heap_mprotect call, now the BPF
   hook attached did not allow this operation, so it had no side-effects.
   Which lead me to believe the crash could be a side-effect of this
   operation. So I did:

--- a/tools/testing/selftests/bpf/prog_tests/test_lsm.c
+++ b/tools/testing/selftests/bpf/prog_tests/test_lsm.c

@@ -29,7 +29,7 @@ int heap_mprotect(void)
         if (buf == NULL)
                 return -ENOMEM;

-       ret = mprotect(buf, sz, PROT_READ | PROT_EXEC);
+       ret = mprotect(buf, sz, PROT_READ | PROT_WRITE | PROT_EXEC);
         free(buf);
         return ret;
  }

and the crash went away. Which made me realize that the free
operation does not like memory without PROT_WRITE, So I did this:

diff --git a/tools/testing/selftests/bpf/prog_tests/test_lsm.c b/tools/testing/selftests/bpf/prog_tests/test_lsm.c
index fcd839e88540..78f125cc09b3 100644
--- a/tools/testing/selftests/bpf/prog_tests/test_lsm.c
+++ b/tools/testing/selftests/bpf/prog_tests/test_lsm.c

@@ -30,7 +30,7 @@ int heap_mprotect(void)
                 return -ENOMEM;

         ret = mprotect(buf, sz, PROT_READ | PROT_EXEC);
-       free(buf);
+       // free(buf);
         return ret;
  }

and the crash went away as well. So it indeed was a combination of:

* CONFIG_LSM not enabling the hook
* mprotect marking the memory as non-writeable
* free being called on the memory.

I will send a v9 which has the PROT_WRITE on the mprotect. Thanks
for noticing this!