On 24-Feb 21:41, 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. > I'm proposing to rename BPF_PROG_TYPE_LSM into BPF_PROG_TYPE_OVERRIDE_RETURN. The BPF_PROG_TYPE_OVERRIDE_RETURN seems to be useful in general as well and we have the implementation already figured out as a part of the LSM work. I will split that bit into a separate series. - KP > > > 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. > > 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.