On 2/25/2020 9:15 PM, KP Singh wrote: > On 25-Feb 16:30, Casey Schaufler wrote: >> On 2/24/2020 9:41 PM, Alexei Starovoitov wrote: >>> On Mon, Feb 24, 2020 at 01:41:19PM -0800, Kees Cook wrote: >>>> But the LSM subsystem doesn't want special cases (Casey has worked very >>>> hard to generalize everything there for stacking). It is really hard to >>>> accept adding a new special case when there are still special cases yet >>>> to be worked out even in the LSM code itself[2]. >>>> [2] Casey's work to generalize the LSM interfaces continues and it quite >>>> complex: >>>> https://lore.kernel.org/linux-security-module/20200214234203.7086-1-casey@xxxxxxxxxxxxxxxx/ >>> I think the key mistake we made is that we classified KRSI as LSM. >>> LSM stacking, lsmblobs that the above set is trying to do are not necessary for KRSI. >>> I don't see anything in LSM infra that KRSI can reuse. >>> The only thing BPF needs is a function to attach to. >>> It can be a nop function or any other. >>> security_*() functions are interesting from that angle only. >>> Hence I propose to reconsider what I was suggesting earlier. >>> No changes to secruity/ directory. >>> Attach to security_*() funcs via bpf trampoline. >>> The key observation vs what I was saying earlier is KRSI and LSM are wrong names. >>> I think "security" is also loaded word that should be avoided. >> No argument there. >> >>> I'm proposing to rename BPF_PROG_TYPE_LSM into BPF_PROG_TYPE_OVERRIDE_RETURN. >>> >>>> So, unless James is going to take this over Casey's objections, the path >>>> forward I see here is: >>>> >>>> - land a "slow" KRSI (i.e. one that hooks every hook with a stub). >>>> - optimize calling for all LSMs >>> I'm very much surprised how 'slow' KRSI is an option at all. >>> 'slow' KRSI means that CONFIG_SECURITY_KRSI=y adds indirect calls to nop >>> functions for every place in the kernel that calls security_*(). >>> This is not an acceptable overhead. Even w/o retpoline >>> this is not something datacenter servers can use. >> In the universe I live in data centers will disable hyper-threading, >> reducing performance substantially, in the face of hypothetical security >> exploits. That's a massively greater performance impact than the handful >> of instructions required to do indirect calls. Not to mention the impact > Indirect calls have worse performance implications than just a few > instructions and are especially not suitable for hotpaths. > > There have been multiple efforts to reduce their usage e.g.: > > - https://lwn.net/Articles/774743/ > - https://lwn.net/Articles/773985/ > >> of the BPF programs that have been included. Have you ever looked at what > BPF programs are JIT'ed and optimized to native code. Doesn't mean people won't write slow code. >> happens to system performance when polkitd is enabled? > However, let's discuss all this separately when we follow-up with > performance improvements after submitting the initial patch-set. Think performance up front. Don't ignore issues. >>> Another option is to do this: >>> diff --git a/include/linux/security.h b/include/linux/security.h >>> index 64b19f050343..7887ce636fb1 100644 >>> --- a/include/linux/security.h >>> +++ b/include/linux/security.h >>> @@ -240,7 +240,7 @@ static inline const char *kernel_load_data_id_str(enum kernel_load_data_id id) >>> return kernel_load_data_str[id]; >>> } >>> >>> -#ifdef CONFIG_SECURITY >>> +#if defined(CONFIG_SECURITY) || defined(CONFIG_BPF_OVERRIDE_RETURN) >>> >>> Single line change to security.h and new file kernel/bpf/override_security.c >>> that will look like: >>> int security_binder_set_context_mgr(struct task_struct *mgr) >>> { >>> return 0; >>> } >>> >>> int security_binder_transaction(struct task_struct *from, >>> struct task_struct *to) >>> { >>> return 0; >>> } >>> Essentially it will provide BPF side with a set of nop functions. >>> CONFIG_SECURITY is off. It may seem as a downside that it will force a choice >>> on kernel users. Either they build the kernel with CONFIG_SECURITY and their >>> choice of LSMs or build the kernel with CONFIG_BPF_OVERRIDE_RETURN and use >>> BPF_PROG_TYPE_OVERRIDE_RETURN programs to enforce any kind of policy. I think >>> it's a pro not a con. >> Err, no. All distros use an LSM or two. Unless you can re-implement SELinux > The users mentioned here in this context are (I would assume) the more > performance sensitive users who would, potentially, disable > CONFIG_SECURITY because of the current performance characteristics. You assume that the most performance sensitive people would allow a mechanism to arbitrarily add overhead that is out of their control? How does that make sense? > We can also discuss this separately and only if we find that we need > it for the BPF_OVERRIDE_RET type attachment. > > - KP > >> in BPF (good luck with state transitions) you've built a warp drive without >> ever having mined dilithium crystals. >> >>