From: Casey Schaufler <email@example.com> To: Alexei Starovoitov <firstname.lastname@example.org>, Kees Cook <email@example.com> Cc: KP Singh <firstname.lastname@example.org>, LKML <email@example.com>, Linux Security Module list <firstname.lastname@example.org>, Alexei Starovoitov <email@example.com>, James Morris <firstname.lastname@example.org>, email@example.com, firstname.lastname@example.org, Casey Schaufler <email@example.com> Subject: Re: [PATCH bpf-next v4 3/8] bpf: lsm: provide attachment points for BPF LSM programs Date: Mon, 24 Feb 2020 08:32:10 -0800 Message-ID: <firstname.lastname@example.org> (raw) In-Reply-To: <20200223220833.wdhonzvven7payaw@ast-mbp> On 2/23/2020 2:08 PM, Alexei Starovoitov wrote: > 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 (?) > no. > 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. Then why the insistence that it be last? >> 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. Um, then why not use the infrastructure as is? >> 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. Sorry, I haven't gotten that memo. > Not only for security, but for any new infra. The LSM infrastructure ain't new. > 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. Sorry, but I don't see how BPF is so overwhelmingly special. >> 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. Err, no, it doesn't. It does an hlish_for_each_entry(), which may be the equivalent on an empty list, but let's not go around spreading misinformation. > Some of those hooks are in critical path. This load+cmp > can be avoided with static_key optimization. I think it's worth doing. I admit to being unfamiliar with the static_key implementation, but if it would work for a list of hooks rather than a singe hook, I'm all ears. >> If static keys are justified for KRSI > I really like that KRSI costs absolutely zero when it's not enabled. And I dislike that there's security module specific code in security.c, security.h and/or lsm_hooks.h. KRSI *is not that special*. > 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. I'm not objecting to the good it does for KRSI. I am *strongly* objecting to special casing KRSI. > 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.
next prev 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] ` <email@example.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 2020-02-24 16:32 ` Casey Schaufler [this message] 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: 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 \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.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
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 \ email@example.com public-inbox-index bpf Example config snippet for mirrors Newsgroup available over NNTP: nntp://nntp.lore.kernel.org/org.kernel.vger.bpf AGPL code for this site: git clone https://public-inbox.org/public-inbox.git