| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 23 Jan 2020 09:03:47 -0800
From: Casey Schaufler <casey@...aufler-ca.com>
To: KP Singh <kpsingh@...omium.org>,
LKML <linux-kernel@...r.kernel.org>,
Linux Security Module list
<linux-security-module@...r.kernel.org>, bpf@...r.kernel.org
Cc: Casey Schaufler <casey@...aufler-ca.com>
Subject: Re: [PATCH bpf-next v3 04/10] bpf: lsm: Add mutable hooks list for
the BPF LSM
On 1/23/2020 7:24 AM, KP Singh wrote:
> From: KP Singh <kpsingh@...gle.com>
>
> - The list of hooks registered by an LSM is currently immutable as they
> are declared with __lsm_ro_after_init and they are attached to a
> security_hook_heads struct.
> - For the BPF LSM we need to de/register the hooks at runtime. Making
> the existing security_hook_heads mutable broadens an
> attack vector, so a separate security_hook_heads is added for only
> those that ~must~ be mutable.
> - These mutable hooks are run only after all the static hooks have
> successfully executed.
>
> This is based on the ideas discussed in:
>
> https://lore.kernel.org/lkml/20180408065916.GA2832@ircssh-2.c.rugged-nimbus-611.internal
>
> Reviewed-by: Brendan Jackman <jackmanb@...gle.com>
> Reviewed-by: Florent Revest <revest@...gle.com>
> Reviewed-by: Thomas Garnier <thgarnie@...gle.com>
> Signed-off-by: KP Singh <kpsingh@...gle.com>
> ---
> MAINTAINERS | 1 +
> include/linux/bpf_lsm.h | 72 +++++++++++++++++++++++++++++++++++++++++
> security/bpf/Kconfig | 1 +
> security/bpf/Makefile | 2 +-
> security/bpf/hooks.c | 20 ++++++++++++
> security/bpf/lsm.c | 7 ++++
> security/security.c | 25 +++++++-------
> 7 files changed, 116 insertions(+), 12 deletions(-)
> create mode 100644 include/linux/bpf_lsm.h
> create mode 100644 security/bpf/hooks.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index e2b7f76a1a70..c606b3d89992 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -3209,6 +3209,7 @@ L: linux-security-module@...r.kernel.org
> L: bpf@...r.kernel.org
> S: Maintained
> F: security/bpf/
> +F: include/linux/bpf_lsm.h
>
> BROADCOM B44 10/100 ETHERNET DRIVER
> M: Michael Chan <michael.chan@...adcom.com>
> diff --git a/include/linux/bpf_lsm.h b/include/linux/bpf_lsm.h
> new file mode 100644
> index 000000000000..57c20b2cd2f4
> --- /dev/null
> +++ b/include/linux/bpf_lsm.h
> @@ -0,0 +1,72 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +/*
> + * Copyright 2019 Google LLC.
> + */
> +
> +#ifndef _LINUX_BPF_LSM_H
> +#define _LINUX_BPF_LSM_H
> +
> +#include <linux/bpf.h>
> +#include <linux/lsm_hooks.h>
> +
> +#ifdef CONFIG_SECURITY_BPF
> +
> +/* Mutable hooks defined at runtime and executed after all the statically
> + * defined LSM hooks.
> + */
> +extern struct security_hook_heads bpf_lsm_hook_heads;
> +
> +int bpf_lsm_srcu_read_lock(void);
> +void bpf_lsm_srcu_read_unlock(int idx);
> +
> +#define CALL_BPF_LSM_VOID_HOOKS(FUNC, ...) \
> + do { \
> + struct security_hook_list *P; \
> + int _idx; \
> + \
> + if (hlist_empty(&bpf_lsm_hook_heads.FUNC)) \
> + break; \
> + \
> + _idx = bpf_lsm_srcu_read_lock(); \
> + hlist_for_each_entry(P, &bpf_lsm_hook_heads.FUNC, list) \
> + P->hook.FUNC(__VA_ARGS__); \
> + bpf_lsm_srcu_read_unlock(_idx); \
> + } while (0)
> +
> +#define CALL_BPF_LSM_INT_HOOKS(FUNC, ...) ({ \
> + int _ret = 0; \
> + do { \
> + struct security_hook_list *P; \
> + int _idx; \
> + \
> + if (hlist_empty(&bpf_lsm_hook_heads.FUNC)) \
> + break; \
> + \
> + _idx = bpf_lsm_srcu_read_lock(); \
> + \
> + hlist_for_each_entry(P, \
> + &bpf_lsm_hook_heads.FUNC, list) { \
> + _ret = P->hook.FUNC(__VA_ARGS__); \
> + if (_ret && IS_ENABLED(CONFIG_SECURITY_BPF_ENFORCE)) \
> + break; \
> + } \
> + bpf_lsm_srcu_read_unlock(_idx); \
> + } while (0); \
> + IS_ENABLED(CONFIG_SECURITY_BPF_ENFORCE) ? _ret : 0; \
> +})
> +
> +#else /* !CONFIG_SECURITY_BPF */
> +
> +#define CALL_BPF_LSM_INT_HOOKS(FUNC, ...) (0)
> +#define CALL_BPF_LSM_VOID_HOOKS(...)
> +
> +static inline int bpf_lsm_srcu_read_lock(void)
> +{
> + return 0;
> +}
> +static inline void bpf_lsm_srcu_read_unlock(int idx) {}
> +
> +#endif /* CONFIG_SECURITY_BPF */
> +
> +#endif /* _LINUX_BPF_LSM_H */
> diff --git a/security/bpf/Kconfig b/security/bpf/Kconfig
> index a5f6c67ae526..595e4ad597ae 100644
> --- a/security/bpf/Kconfig
> +++ b/security/bpf/Kconfig
> @@ -6,6 +6,7 @@ config SECURITY_BPF
> bool "BPF-based MAC and audit policy"
> depends on SECURITY
> depends on BPF_SYSCALL
> + depends on SRCU
> help
> This enables instrumentation of the security hooks with
> eBPF programs.
> diff --git a/security/bpf/Makefile b/security/bpf/Makefile
> index c78a8a056e7e..c526927c337d 100644
> --- a/security/bpf/Makefile
> +++ b/security/bpf/Makefile
> @@ -2,4 +2,4 @@
> #
> # Copyright 2019 Google LLC.
>
> -obj-$(CONFIG_SECURITY_BPF) := lsm.o ops.o
> +obj-$(CONFIG_SECURITY_BPF) := lsm.o ops.o hooks.o
> diff --git a/security/bpf/hooks.c b/security/bpf/hooks.c
> new file mode 100644
> index 000000000000..b123d9cb4cd4
> --- /dev/null
> +++ b/security/bpf/hooks.c
> @@ -0,0 +1,20 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * Copyright 2019 Google LLC.
> + */
> +
> +#include <linux/bpf_lsm.h>
> +#include <linux/srcu.h>
> +
> +DEFINE_STATIC_SRCU(security_hook_srcu);
> +
> +int bpf_lsm_srcu_read_lock(void)
> +{
> + return srcu_read_lock(&security_hook_srcu);
> +}
> +
> +void bpf_lsm_srcu_read_unlock(int idx)
> +{
> + return srcu_read_unlock(&security_hook_srcu, idx);
> +}
> diff --git a/security/bpf/lsm.c b/security/bpf/lsm.c
> index dc9ac03c7aa0..a25a068e1781 100644
> --- a/security/bpf/lsm.c
> +++ b/security/bpf/lsm.c
> @@ -4,6 +4,7 @@
> * Copyright 2019 Google LLC.
> */
>
> +#include <linux/bpf_lsm.h>
> #include <linux/lsm_hooks.h>
>
> /* This is only for internal hooks, always statically shipped as part of the
> @@ -12,6 +13,12 @@
> */
> static struct security_hook_list bpf_lsm_hooks[] __lsm_ro_after_init = {};
>
> +/* Security hooks registered dynamically by the BPF LSM and must be accessed
> + * by holding bpf_lsm_srcu_read_lock and bpf_lsm_srcu_read_unlock. The mutable
> + * hooks dynamically allocated by the BPF LSM are appeneded here.
> + */
> +struct security_hook_heads bpf_lsm_hook_heads;
> +
> static int __init bpf_lsm_init(void)
> {
> security_add_hooks(bpf_lsm_hooks, ARRAY_SIZE(bpf_lsm_hooks), "bpf");
> diff --git a/security/security.c b/security/security.c
> index 30a8aa700557..95a46ca25dcd 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -27,6 +27,7 @@
> #include <linux/backing-dev.h>
> #include <linux/string.h>
> #include <linux/msg.h>
> +#include <linux/bpf_lsm.h>
> #include <net/flow.h>
>
> #define MAX_LSM_EVM_XATTR 2
> @@ -657,20 +658,22 @@ static void __init lsm_early_task(struct task_struct *task)
> \
> hlist_for_each_entry(P, &security_hook_heads.FUNC, list) \
> P->hook.FUNC(__VA_ARGS__); \
> + CALL_BPF_LSM_VOID_HOOKS(FUNC, __VA_ARGS__); \
I'm sorry if I wasn't clear on the v2 review.
This does not belong in the infrastructure. You should be
doing all the bpf_lsm hook processing in you module.
bpf_lsm_task_alloc() should loop though all the bpf
task_alloc hooks if they have to be handled differently
from "normal" LSM hooks.
> } while (0)
>
> -#define call_int_hook(FUNC, IRC, ...) ({ \
> - int RC = IRC; \
> - do { \
> - struct security_hook_list *P; \
> - \
> +#define call_int_hook(FUNC, IRC, ...) ({ \
> + int RC = IRC; \
> + do { \
> + struct security_hook_list *P; \
> hlist_for_each_entry(P, &security_hook_heads.FUNC, list) { \
> - RC = P->hook.FUNC(__VA_ARGS__); \
> - if (RC != 0) \
> - break; \
> - } \
> - } while (0); \
> - RC; \
> + RC = P->hook.FUNC(__VA_ARGS__); \
> + if (RC != 0) \
> + break; \
> + } \
> + if (RC == 0) \
> + RC = CALL_BPF_LSM_INT_HOOKS(FUNC, __VA_ARGS__); \
> + } while (0); \
> + RC; \
> })
>
> /* Security operations */
Powered by blists - more mailing lists