[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