BPF Archive on lore.kernel.org
 help / color / Atom feed
From: Ondrej Mosnacek <omosnace@redhat.com>
To: KP Singh <kpsingh@chromium.org>
Cc: Linux kernel mailing list <linux-kernel@vger.kernel.org>,
	Linux Security Module list 
	Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	James Morris <jmorris@namei.org>,
	Anders Roxell <anders.roxell@linaro.org>,
	Casey Schaufler <casey@schaufler-ca.com>
Subject: Re: [PATCH bpf] security: Fix hook iteration for secid_to_secctx
Date: Fri, 19 Jun 2020 14:49:29 +0200
Message-ID: <CAFqZXNsu8Vs86SKpdnej_=xnQqg=Hh132JqNe1Ybt-bHJB4NeQ@mail.gmail.com> (raw)
In-Reply-To: <20200520125616.193765-1-kpsingh@chromium.org>

On Wed, May 20, 2020 at 2:56 PM KP Singh <kpsingh@chromium.org> wrote:
> From: KP Singh <kpsingh@google.com>
> secid_to_secctx is not stackable, and since the BPF LSM registers this
> hook by default, the call_int_hook logic is not suitable which
> "bails-on-fail" and casues issues when other LSMs register this hook and
> eventually breaks Audit.
> In order to fix this, directly iterate over the security hooks instead
> of using call_int_hook as suggested in:
> https: //lore.kernel.org/bpf/9d0eb6c6-803a-ff3a-5603-9ad6d9edfc00@schaufler-ca.com/#t
> Fixes: 98e828a0650f ("security: Refactor declaration of LSM hooks")
> Fixes: 625236ba3832 ("security: Fix the default value of secid_to_secctx hook"
> Reported-by: Alexei Starovoitov <ast@kernel.org>
> Signed-off-by: KP Singh <kpsingh@google.com>

Sorry for being late to the party, but doesn't this (and the
associated default return value patch) just paper over a bigger
problem? What if I have only the BPF LSM enabled and I attach a BPF
program to this hook that just returns 0? Doesn't that allow anything
privileged enough to do this to force the kernel to try and send
memory from uninitialized pointers to userspace and/or copy such
memory around and/or free uninitialized pointers?

Why on earth does the BPF LSM directly expose *all* of the hooks, even
those that are not being used for any security decisions (and are
"useful" in this context only for borking the kernel...)? Feel free to
prove me wrong, but this lazy approach of "let's just take all the
hooks as they are and stick BPF programs to them" doesn't seem like a
good choice... IMHO you should either limit the set of hooks that can
be attached to only those that aren't used to return back values via
pointers, or (if you really really need to do some state
updates/logging in those hooks) use wrapper functions that will call
the BPF progs via a simplified interface so that they cannot cause
unsafe behavior.

Ondrej Mosnacek
Software Engineer, Platform Security - SELinux kernel
Red Hat, Inc.

  parent reply index

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-20 12:56 KP Singh
2020-05-20 15:15 ` Casey Schaufler
2020-05-21  1:35   ` Alexei Starovoitov
2020-05-21  2:02     ` James Morris
2020-05-21  3:12       ` Alexei Starovoitov
2020-06-19 12:49 ` Ondrej Mosnacek [this message]
2020-06-19 13:13   ` KP Singh
2020-06-19 14:16     ` Ondrej Mosnacek
2020-06-21 21:54       ` KP Singh

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:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAFqZXNsu8Vs86SKpdnej_=xnQqg=Hh132JqNe1Ybt-bHJB4NeQ@mail.gmail.com' \
    --to=omosnace@redhat.com \
    --cc=anders.roxell@linaro.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=casey@schaufler-ca.com \
    --cc=daniel@iogearbox.net \
    --cc=jmorris@namei.org \
    --cc=kpsingh@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \


* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

BPF Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/bpf/0 bpf/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 bpf bpf/ https://lore.kernel.org/bpf \
	public-inbox-index bpf

Example config snippet for mirrors

Newsgroup available over NNTP:

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git