BPF Archive on lore.kernel.org
 help / color / Atom feed
From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
To: Kees Cook <keescook@chromium.org>
Cc: Casey Schaufler <casey@schaufler-ca.com>,
	KP Singh <kpsingh@chromium.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Linux Security Module list 
	Alexei Starovoitov <ast@kernel.org>,
	James Morris <jmorris@namei.org>,
	bpf@vger.kernel.org, netdev@vger.kernel.org
Subject: Re: [PATCH bpf-next v4 3/8] bpf: lsm: provide attachment points for BPF LSM programs
Date: Sun, 23 Feb 2020 14:08:34 -0800
Message-ID: <20200223220833.wdhonzvven7payaw@ast-mbp> (raw)
In-Reply-To: <202002211946.A23A987@keescook>

On Fri, Feb 21, 2020 at 08:22:59PM -0800, Kees Cook wrote:
> If I'm understanding this correctly, there are two issues:
> 1- BPF needs to be run last due to fexit trampolines (?)

The placement of nop call can be anywhere.
BPF trampoline is automagically converting nop call into a sequence
of directly invoked BPF programs.
No link list traversals and no indirect calls in run-time.

> 2- BPF hooks don't know what may be attached at any given time, so
>    ALL LSM hooks need to be universally hooked. THIS turns out to create
>    a measurable performance problem in that the cost of the indirect call
>    on the (mostly/usually) empty BPF policy is too high.

also no.

> So, trying to avoid the indirect calls is, as you say, an optimization,
> but it might be a needed one due to the other limitations.

I'm convinced that avoiding the cost of retpoline in critical path is a
requirement for any new infrastructure in the kernel.
Not only for security, but for any new infra.
Networking stack converted all such places to conditional calls.
In BPF land we converted indirect calls to direct jumps and direct calls.
It took two years to do so. Adding new indirect calls is not an option.
I'm eagerly waiting for Peter's static_call patches to land to convert
a lot more indirect calls. May be existing LSMs will take advantage
of static_call patches too, but static_call is not an option for BPF.
That's why we introduced BPF trampoline in the last kernel release.

> b) Would there actually be a global benefit to using the static keys
>    optimization for other LSMs?

Yes. Just compiling with CONFIG_SECURITY adds "if (hlist_empty)" check
for every hook. Some of those hooks are in critical path. This load+cmp
can be avoided with static_key optimization. I think it's worth doing.

> If static keys are justified for KRSI

I really like that KRSI costs absolutely zero when it's not enabled.
Attaching BPF prog to one hook preserves zero cost for all other hooks.
And when one hook is BPF powered it's using direct call instead of
super expensive retpoline.

Overall this patch set looks good to me. There was a minor issue with prog
accounting. I expect only that bit to be fixed in v5.

  parent reply index

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-20 17:52 [PATCH bpf-next v4 0/8] MAC and Audit policy using eBPF (KRSI) KP Singh
2020-02-20 17:52 ` [PATCH bpf-next v4 1/8] bpf: Introduce BPF_PROG_TYPE_LSM KP Singh
2020-02-20 17:52 ` [PATCH bpf-next v4 2/8] security: Refactor declaration of LSM hooks KP Singh
2020-02-20 17:52 ` [PATCH bpf-next v4 3/8] bpf: lsm: provide attachment points for BPF LSM programs KP Singh
2020-02-21  2:25   ` Alexei Starovoitov
2020-02-21 11:47     ` KP Singh
     [not found]   ` <0ef26943-9619-3736-4452-fec536a8d169@schaufler-ca.com>
2020-02-21 11:44     ` KP Singh
2020-02-21 18:23       ` Casey Schaufler
     [not found]     ` <202002211946.A23A987@keescook>
2020-02-23 22:08       ` Alexei Starovoitov [this message]
2020-02-24 16:32         ` Casey Schaufler
2020-02-24 17:13           ` KP Singh
2020-02-24 18:45             ` Casey Schaufler
2020-02-24 21:41               ` Kees Cook
2020-02-24 22:29                 ` Casey Schaufler
2020-02-25  5:41                 ` Alexei Starovoitov
2020-02-25 15:31                   ` Kees Cook
2020-02-25 19:31                   ` KP Singh
2020-02-26  0:30                   ` Casey Schaufler
2020-02-26  5:15                     ` KP Singh
2020-02-26 15:35                       ` Casey Schaufler
2020-02-25 19:29                 ` KP Singh
2020-02-20 17:52 ` [PATCH bpf-next v4 4/8] bpf: lsm: Add support for enabling/disabling BPF hooks KP Singh
2020-02-21 18:57   ` Casey Schaufler
2020-02-21 19:11     ` James Morris
2020-02-22  4:26   ` Kees Cook
2020-02-20 17:52 ` [PATCH bpf-next v4 5/8] bpf: lsm: Implement attach, detach and execution KP Singh
2020-02-21  2:17   ` Alexei Starovoitov
2020-02-21 12:02     ` KP Singh
2020-02-20 17:52 ` [PATCH bpf-next v4 6/8] tools/libbpf: Add support for BPF_PROG_TYPE_LSM KP Singh
2020-02-25  6:45   ` Andrii Nakryiko
2020-02-20 17:52 ` [PATCH bpf-next v4 7/8] bpf: lsm: Add selftests " KP Singh
2020-02-20 17:52 ` [PATCH bpf-next v4 8/8] bpf: lsm: Add Documentation KP Singh
2020-02-21 19:19 ` [PATCH bpf-next v4 0/8] MAC and Audit policy using eBPF (KRSI) Casey Schaufler
2020-02-21 19:41   ` KP Singh
2020-02-21 22:31     ` Casey Schaufler
2020-02-21 23:09       ` KP Singh
2020-02-21 23:49         ` Casey Schaufler
2020-02-22  0:22       ` Kees Cook
2020-02-22  1:04         ` Casey Schaufler
2020-02-22  3:36           ` Kees Cook
2020-02-27 18:40 ` Dr. Greg

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=20200223220833.wdhonzvven7payaw@ast-mbp \
    --to=alexei.starovoitov@gmail.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=casey@schaufler-ca.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=netdev@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