From: Daniel Borkmann <daniel@iogearbox.net>
To: KP Singh <kpsingh@chromium.org>
Cc: Kees Cook <keescook@chromium.org>,
open list <linux-kernel@vger.kernel.org>,
bpf <bpf@vger.kernel.org>,
Linux Security Module list
<linux-security-module@vger.kernel.org>,
Alexei Starovoitov <ast@kernel.org>,
James Morris <jmorris@namei.org>, Paul Turner <pjt@google.com>,
Jann Horn <jannh@google.com>,
Florent Revest <revest@chromium.org>,
Brendan Jackman <jackmanb@chromium.org>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: Re: [PATCH bpf-next v8 0/8] MAC and Audit policy using eBPF (KRSI)
Date: Sun, 29 Mar 2020 01:15:21 +0100 [thread overview]
Message-ID: <bf04019c-b0d0-5aff-be72-32e46b60daea@iogearbox.net> (raw)
In-Reply-To: <20200329000738.GA230422@google.com>
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
prev parent reply other threads:[~2020-03-29 0:15 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-27 19:28 [PATCH bpf-next v8 0/8] MAC and Audit policy using eBPF (KRSI) KP Singh
2020-03-27 19:28 ` [PATCH bpf-next v8 1/8] bpf: Introduce BPF_PROG_TYPE_LSM KP Singh
2020-03-27 19:28 ` [PATCH bpf-next v8 2/8] security: Refactor declaration of LSM hooks KP Singh
2020-03-27 19:28 ` [PATCH bpf-next v8 3/8] bpf: lsm: provide attachment points for BPF LSM programs KP Singh
2020-03-27 19:28 ` [PATCH bpf-next v8 4/8] bpf: lsm: Implement attach, detach and execution KP Singh
2020-03-28 1:08 ` James Morris
2020-03-27 19:28 ` [PATCH bpf-next v8 5/8] bpf: lsm: Initialize the BPF LSM hooks KP Singh
2020-03-27 19:28 ` [PATCH bpf-next v8 6/8] tools/libbpf: Add support for BPF_PROG_TYPE_LSM KP Singh
2020-03-27 19:28 ` [PATCH bpf-next v8 7/8] bpf: lsm: Add selftests " KP Singh
2020-03-27 19:28 ` [PATCH bpf-next v8 8/8] bpf: lsm: Add Documentation KP Singh
2020-03-28 17:18 ` [PATCH bpf-next v8 0/8] MAC and Audit policy using eBPF (KRSI) Daniel Borkmann
2020-03-28 19:56 ` KP Singh
2020-03-28 21:50 ` Kees Cook
2020-03-28 22:30 ` KP Singh
2020-03-29 0:07 ` KP Singh
2020-03-29 0:15 ` Daniel Borkmann [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=bf04019c-b0d0-5aff-be72-32e46b60daea@iogearbox.net \
--to=daniel@iogearbox.net \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=jackmanb@chromium.org \
--cc=jannh@google.com \
--cc=jmorris@namei.org \
--cc=keescook@chromium.org \
--cc=kpsingh@chromium.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=pjt@google.com \
--cc=revest@chromium.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).