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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <bf04019c-b0d0-5aff-be72-32e46b60daea@iogearbox.net>
Date:   Sun, 29 Mar 2020 01:15:21 +0100
From:   Daniel Borkmann <daniel@...earbox.net>
To:     KP Singh <kpsingh@...omium.org>
Cc:     Kees Cook <keescook@...omium.org>,
        open list <linux-kernel@...r.kernel.org>,
        bpf <bpf@...r.kernel.org>,
        Linux Security Module list 
        <linux-security-module@...r.kernel.org>,
        Alexei Starovoitov <ast@...nel.org>,
        James Morris <jmorris@...ei.org>, Paul Turner <pjt@...gle.com>,
        Jann Horn <jannh@...gle.com>,
        Florent Revest <revest@...omium.org>,
        Brendan Jackman <jackmanb@...omium.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: Re: [PATCH bpf-next v8 0/8] MAC and Audit policy using eBPF (KRSI)

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@...omium.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