[RFC PATCH 1/2] LSM: Allow dynamically appendable LSM modules.
KP Singh
kpsingh at kernel.org
Wed Sep 27 16:02:32 UTC 2023
On Wed, Sep 27, 2023 at 5:09 PM Tetsuo Handa
<penguin-kernel at i-love.sakura.ne.jp> 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@kernel.org/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.
>
> Link: https://marc.info/?l=linux-security-module&m=98706471912438&w=2 [1]
> Link: https://lkml.kernel.org/r/20230912205658.3432-2-casey@schaufler-ca.com [2]
> Link: https://lkml.kernel.org/r/6e1c25f5-b78c-8b4e-ddc3-484129c4c0ec@I-love.SAKURA.ne.jp [3]
> Link: https://lkml.kernel.org/r/20230918212459.1937798-1-kpsingh@kernel.org [4]
> Link: https://lkml.kernel.org/r/ed785c86-a1d8-caff-c629-f8a50549e05b@I-love.SAKURA.ne.jp [5]
> Link: https://lkml.kernel.org/r/36c7cf74-508f-1690-f86a-bb18ec686fcf@schaufler-ca.com [6]
> Signed-off-by: Tetsuo Handa <penguin-kernel at I-love.SAKURA.ne.jp>
> ---
> 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
More information about the Linux-security-module-archive
mailing list