Messages in this thread | | | Subject | Re: [PATCH bpf-next v8 0/8] MAC and Audit policy using eBPF (KRSI) | From | Daniel Borkmann <> | Date | Sun, 29 Mar 2020 01:15:21 +0100 |
| |
On 3/29/20 1:07 AM, KP Singh wrote: > On 28-Mar 23:30, KP Singh wrote: >> On Sat, Mar 28, 2020 at 10:50 PM Kees Cook <keescook@chromium.org> wrote: >>> >>> On Sat, Mar 28, 2020 at 08:56:36PM +0100, KP Singh wrote: >>>> 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!
And also explains the stack trace for __libc_calloc() where it's trying to zero the area later on.
Thanks for the quick debugging, Daniel
| |