Le Wed, 27 Sep 2023 18:02:32 +0200, KP Singh <kpsingh@xxxxxxxxxx> a écrit : > On Wed, Sep 27, 2023 at 5:09 PM Tetsuo Handa > <penguin-kernel@xxxxxxxxxxxxxxxxxxx> wrote: > > > > Recently, the LSM community is trying to make drastic changes. > > > > Crispin Cowan has explained > > > > It is Linus' comments that spurred me to want to start this > > undertaking. He observes that there are many different security > > approaches, each with their own advocates. He doesn't want to > > arbitrate which of them should be "the" Linux security approach, > > and would rather that Linux can support any of them. > > > > That is the purpose of this project: to allow Linux to support a > > variety of security models, so that security developers don't have > > to have the "my dog's bigger than your dog" argument, and users can > > choose the security model that suits their needs. > > > > when the LSM project started [1]. > > > > However, Casey Schaufler is trying to make users difficult to > > choose the security model that suits their needs, by requiring LSM > > ID value which is assigned to only LSM modules that succeeded to > > become in-tree [2]. Therefore, I'm asking Casey and Paul Moore to > > change their mind to allow assigning LSM ID value to any LSM > > modules (so that users can choose the security model that suits > > their needs) [3]. > > > > I expect that LSM ID value will be assigned to any publicly > > available LSM modules. Otherwise, it is mostly pointless to propose > > this patch; there will be little LSM modules to built into vmlinux; > > let alone dynamically loading as LKM-based LSMs. > > > > Also, KP Singh is trying to replace the linked list with static > > calls in order to reduce overhead of indirect calls [4]. However, > > this change assumed that any LSM modules are built-in. I don't like > > such assumption because I still consider that LSM modules which are > > not built into vmlinux will be wanted by users [5]. > > > > Then, Casey told me to supply my implementation of loadable security > > modules [6]. Therefore, I post this patch as basic changes needed > > for allowing dynamically appendable LSM modules (and an example of > > appendable LSM modules). This patch was tested on only x86_64. > > > > Question for KP Singh would be how can we allow dynamically > > appendable LSM modules if current linked list is replaced with > > static calls with minimal-sized array... > > As I suggested in the other thread: > > https://lore.kernel.org/bpf/20230918212459.1937798-1-kpsingh@xxxxxxxxxx/T/#md21b9d9cc769f39e451d20364857b693d3fcb587 > > You can add extra static call slots and fallback to a linked list > based implementation if you have more than say N modules [1] and > fallback to a linked list implementation [2]. > > for [1] you can just do MAX_LSM_COUNT you can just do: > > #ifdef CONFIG_MODULAR_LSM > #define MODULAR_LSM_ENABLED "1,1,1,1" > #endif > > and use it in the LSM_COUNT. > > for [2] you can choose to export a better module API than directly > exposing security_hook_heads. > > Now, comes the question of whether we need dynamically loaded LSMs, I > am not in favor of this.Please share your limitations of BPF as you > mentioned and what's missing to implement dynamic LSMs. My question > still remains unanswered. > > Until I hear the real limitations of using BPF, it's a NAK from me. Hi all, I don't understand the reason why you want to enforce implementers to use your BPF? Even if it can do any possible thing that security implementer wants, why enforcing to use it? For experimenting? But then after successful experimentation the implementer must translate to real LSM and rewrite almost every thing. And also why to use faty BPF for a tricky simple stuff? Regards José Bollo > > > > Link: > > https://marc.info/?l=linux-security-module&m=98706471912438&w=2 [1] > > Link: > > https://lkml.kernel.org/r/20230912205658.3432-2-casey@xxxxxxxxxxxxxxxx > > [2] Link: > > https://lkml.kernel.org/r/6e1c25f5-b78c-8b4e-ddc3-484129c4c0ec@xxxxxxxxxxxxxxxxxxx > > [3] Link: > > https://lkml.kernel.org/r/20230918212459.1937798-1-kpsingh@xxxxxxxxxx > > [4] Link: > > https://lkml.kernel.org/r/ed785c86-a1d8-caff-c629-f8a50549e05b@xxxxxxxxxxxxxxxxxxx > > [5] Link: > > https://lkml.kernel.org/r/36c7cf74-508f-1690-f86a-bb18ec686fcf@xxxxxxxxxxxxxxxx > > [6] Signed-off-by: Tetsuo Handa > > <penguin-kernel@xxxxxxxxxxxxxxxxxxx> --- include/linux/lsm_hooks.h > > | 2 + security/security.c | 107 > > ++++++++++++++++++++++++++++++++++++++ 2 files changed, 109 > > insertions(+) > > > > diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h > > index dcb5e5b5eb13..73db3c41df26 100644 > > --- a/include/linux/lsm_hooks.h > > +++ b/include/linux/lsm_hooks.h > > @@ -105,6 +105,8 @@ extern char *lsm_names; > > > > extern void security_add_hooks(struct security_hook_list *hooks, > > int count, const char *lsm); > > +extern int register_loadable_lsm(struct security_hook_list *hooks, > > int count, > > + const char *lsm); > > > > #define LSM_FLAG_LEGACY_MAJOR BIT(0) > > #define LSM_FLAG_EXCLUSIVE BIT(1) > > diff --git a/security/security.c b/security/security.c > > index 23b129d482a7..6c64b7afb251 100644 > > --- a/security/security.c > > +++ b/security/security.c > > @@ -74,6 +74,7 @@ const char *const > > lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX + 1] = { }; > > > > struct security_hook_heads security_hook_heads __ro_after_init; > > +EXPORT_SYMBOL_GPL(security_hook_heads); > > Rather than exposting security_hook_heads, this should actually export > security_hook_module_register. This should internally handle any data > structures used and also not need the special magic that you did for > __ro_after_init. > > - KP > > > static BLOCKING_NOTIFIER_HEAD(blocking_lsm_notifier_chain); > > > > static struct kmem_cache *lsm_file_cache; > > @@ -537,6 +538,112 @@ void __init security_add_hooks(struct > > security_hook_list *hooks, int count, } > > } > > > > +#if defined(CONFIG_STRICT_KERNEL_RWX) > > +#define MAX_RO_PAGES 1024 /* Wild guess. Can be minimized by > > dynamic allocation. */ +static struct page *ro_pages[MAX_RO_PAGES]; > > /* Pages that are marked read-only. */ +static unsigned int > > ro_pages_len; /* Number of pages that are marked read-only. */ + > > +/* Check whether a page containing given address does not have > > _PAGE_BIT_RW bit. */ +static bool lsm_test_page_ro(void *addr) > > +{ > > + unsigned int i; > > + int unused; > > + struct page *page; > > + > > + page = (struct page *) lookup_address((unsigned long) addr, > > &unused); > > + if (!page) > > + return false; > > + if (test_bit(_PAGE_BIT_RW, &(page->flags))) > > + return true; > > + for (i = 0; i < ro_pages_len; i++) > > + if (page == ro_pages[i]) > > + return true; > > + if (ro_pages_len == MAX_RO_PAGES) > > + return false; > > + ro_pages[ro_pages_len++] = page; > > + return true; > > +} > > + > > +/* Find pages which do not have _PAGE_BIT_RW bit. */ > > +static bool check_ro_pages(struct security_hook_list *hooks, int > > count) +{ > > + int i; > > + struct hlist_head *list = &security_hook_heads.capable; > > + > > + if (!copy_to_kernel_nofault(list, list, sizeof(void *))) > > + return true; > > + for (i = 0; i < count; i++) { > > + struct hlist_head *head = hooks[i].head; > > + struct security_hook_list *shp; > > + > > + if (!lsm_test_page_ro(&head->first)) > > + return false; > > + hlist_for_each_entry(shp, head, list) > > + if (!lsm_test_page_ro(&shp->list.next) || > > + !lsm_test_page_ro(&shp->list.pprev)) > > + return false; > > + } > > + return true; > > +} > > +#endif > > + > > +/** > > + * register_loadable_lsm - Add a dynamically appendable module's > > hooks to the hook lists. > > + * @hooks: the hooks to add > > + * @count: the number of hooks to add > > + * @lsm: the name of the security module > > + * > > + * Each dynamically appendable LSM has to register its hooks with > > the infrastructure. > > + * > > + * Assumes that this function is called from module_init() > > function where > > + * call to this function is already serialized by module_mutex > > lock. > > + */ > > +int register_loadable_lsm(struct security_hook_list *hooks, int > > count, > > + const char *lsm) > > +{ > > + int i; > > + char *cp; > > + > > + // TODO: Check whether proposed hooks can co-exist with > > already chained hooks, > > + // and bail out here if one of hooks cannot > > co-exist... + > > +#if defined(CONFIG_STRICT_KERNEL_RWX) > > + // Find pages which needs to make temporarily writable. > > + ro_pages_len = 0; > > + if (!check_ro_pages(hooks, count)) { > > + pr_err("Can't make security_hook_heads again > > writable. Retry with rodata=off kernel command line option > > added.\n"); > > + return -EINVAL; > > + } > > + pr_info("ro_pages_len=%d\n", ro_pages_len); > > +#endif > > + // At least "capability" is already included. > > + cp = kasprintf(GFP_KERNEL, "%s,%s", lsm_names, lsm); > > + if (!cp) { > > + pr_err("%s - Cannot get memory.\n", __func__); > > + return -ENOMEM; > > + } > > +#if defined(CONFIG_STRICT_KERNEL_RWX) > > + // Make security_hook_heads (and hooks chained) temporarily > > writable. > > + for (i = 0; i < ro_pages_len; i++) > > + set_bit(_PAGE_BIT_RW, &(ro_pages[i]->flags)); > > +#endif > > + // Register dynamically appendable module's hooks. > > + for (i = 0; i < count; i++) { > > + hooks[i].lsm = lsm; > > + hlist_add_tail_rcu(&hooks[i].list, hooks[i].head); > > + } > > +#if defined(CONFIG_STRICT_KERNEL_RWX) > > + // Make security_hook_heads (and hooks chained) again > > read-only. > > + for (i = 0; i < ro_pages_len; i++) > > + clear_bit(_PAGE_BIT_RW, &(ro_pages[i]->flags)); > > +#endif > > + // TODO: Wait for reader side before kfree(). > > + kfree(lsm_names); > > + lsm_names = cp; > > + return 0; > > +} > > +EXPORT_SYMBOL_GPL(register_loadable_lsm); > > + > > int call_blocking_lsm_notifier(enum lsm_event event, void *data) > > { > > return > > blocking_notifier_call_chain(&blocking_lsm_notifier_chain, -- > > 2.18.4
