BPF Archive on lore.kernel.org
 help / color / Atom feed
From: Casey Schaufler <casey@schaufler-ca.com>
To: KP Singh <kpsingh@chromium.org>
Cc: LKML <linux-kernel@vger.kernel.org>,
	Linux Security Module list 
	<linux-security-module@vger.kernel.org>,
	bpf <bpf@vger.kernel.org>,
	Casey Schaufler <casey@schaufler-ca.com>
Subject: Re: [PATCH bpf-next v4 4/8] bpf: lsm: Add support for enabling/disabling BPF hooks
Date: Fri, 21 Feb 2020 10:57:39 -0800
Message-ID: <bb32f155-5213-71df-c679-85c614c0ac26@schaufler-ca.com> (raw)
In-Reply-To: <20200220175250.10795-5-kpsingh@chromium.org>

On 2/20/2020 9:52 AM, KP Singh wrote:
> From: KP Singh <kpsingh@google.com>

Again, sorry for trimming the CC list, but thunderbird ...

>
> Each LSM hook defines a static key i.e. bpf_lsm_<name>
> and a bpf_lsm_<name>_set_enabled function to toggle the key
> which enables/disables the branch which executes the BPF programs
> attached to the LSM hook.
>
> Use of static keys was suggested in upstream discussion:
>
>   https://lore.kernel.org/bpf/1cd10710-a81b-8f9b-696d-aa40b0a67225@iogearbox.net/
>
> and results in the following assembly:
>
>   0x0000000000001e31 <+65>:    jmpq   0x1e36 <security_bprm_check+70>
>   0x0000000000001e36 <+70>:    nopl   0x0(%rax,%rax,1)
>   0x0000000000001e3b <+75>:    xor    %eax,%eax
>   0x0000000000001e3d <+77>:    jmp    0x1e25 <security_bprm_check+53>
>
> which avoids an indirect branch and results in lower overhead which is
> especially helpful for LSM hooks in performance hotpaths.
>
> Given the ability to toggle the BPF trampolines, some hooks which do
> not call call_<int, void>_hooks as they have different default return
> values, also gain support for BPF program attachment.
>
> There are some hooks like security_setprocattr and security_getprocattr
> which are not instrumentable as they do not provide any monitoring or
> access control decisions. If required, generation of BTF type
> information for these hooks can be also be blacklisted.
>
> Signed-off-by: KP Singh <kpsingh@google.com>
> ---
>  include/linux/bpf_lsm.h | 30 +++++++++++++++++++++++++++---
>  kernel/bpf/bpf_lsm.c    | 28 ++++++++++++++++++++++++++++
>  security/security.c     | 32 ++++++++++++++++++++++++++++++++
>  3 files changed, 87 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/bpf_lsm.h b/include/linux/bpf_lsm.h
> index f867f72f6aa9..53dcda8ace01 100644
> --- a/include/linux/bpf_lsm.h
> +++ b/include/linux/bpf_lsm.h
> @@ -8,27 +8,51 @@
>  #define _LINUX_BPF_LSM_H
>  
>  #include <linux/bpf.h>
> +#include <linux/jump_label.h>
>  
>  #ifdef CONFIG_BPF_LSM
>  
> +#define LSM_HOOK(RET, NAME, ...)		\
> +DECLARE_STATIC_KEY_FALSE(bpf_lsm_key_##NAME);   \
> +void bpf_lsm_##NAME##_set_enabled(bool value);
> +#include <linux/lsm_hook_names.h>
> +#undef LSM_HOOK

This is an amazing amount of macro magic. You're creating
dependencies that will make changes to the infrastructure
much more difficult. I think. It's really hard to tell.
At the very least you should have a description of what this
accomplishes, as it's far from obvious.

> +
>  #define LSM_HOOK(RET, NAME, ...) RET bpf_lsm_##NAME(__VA_ARGS__);
>  #include <linux/lsm_hook_names.h>
>  #undef LSM_HOOK
>  
> -#define RUN_BPF_LSM_VOID_PROGS(FUNC, ...) bpf_lsm_##FUNC(__VA_ARGS__)
> +#define HAS_BPF_LSM_PROG(FUNC) (static_branch_unlikely(&bpf_lsm_key_##FUNC))
> +
> +#define RUN_BPF_LSM_VOID_PROGS(FUNC, ...)				\
> +	do {								\
> +		if (HAS_BPF_LSM_PROG(FUNC))				\
> +			bpf_lsm_##FUNC(__VA_ARGS__);			\
> +	} while (0)
> +
>  #define RUN_BPF_LSM_INT_PROGS(RC, FUNC, ...) ({				\
>  	do {								\
> -		if (RC == 0)						\
> -			RC = bpf_lsm_##FUNC(__VA_ARGS__);		\
> +		if (HAS_BPF_LSM_PROG(FUNC)) {				\
> +			if (RC == 0)					\
> +				RC = bpf_lsm_##FUNC(__VA_ARGS__);	\
> +		}							\
>  	} while (0);							\
>  	RC;								\
>  })
>  
> +int bpf_lsm_set_enabled(const char *name, bool value);
> +
>  #else /* !CONFIG_BPF_LSM */
>  
> +#define HAS_BPF_LSM_PROG false
>  #define RUN_BPF_LSM_INT_PROGS(RC, FUNC, ...) (RC)
>  #define RUN_BPF_LSM_VOID_PROGS(FUNC, ...)
>  
> +static inline int bpf_lsm_set_enabled(const char *name, bool value)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
>  #endif /* CONFIG_BPF_LSM */
>  
>  #endif /* _LINUX_BPF_LSM_H */
> diff --git a/kernel/bpf/bpf_lsm.c b/kernel/bpf/bpf_lsm.c
> index abc847c9b9a1..d7c44433c003 100644
> --- a/kernel/bpf/bpf_lsm.c
> +++ b/kernel/bpf/bpf_lsm.c
> @@ -8,6 +8,20 @@
>  #include <linux/bpf.h>
>  #include <linux/btf.h>
>  #include <linux/bpf_lsm.h>
> +#include <linux/jump_label.h>
> +#include <linux/kallsyms.h>
> +
> +#define LSM_HOOK(RET, NAME, ...)					\
> +	DEFINE_STATIC_KEY_FALSE(bpf_lsm_key_##NAME);			\
> +	void bpf_lsm_##NAME##_set_enabled(bool value)			\
> +	{								\
> +		if (value)						\
> +			static_branch_enable(&bpf_lsm_key_##NAME);	\
> +		else							\
> +			static_branch_disable(&bpf_lsm_key_##NAME);	\
> +	}
> +#include <linux/lsm_hook_names.h>
> +#undef LSM_HOOK
>  
>  /* For every LSM hook  that allows attachment of BPF programs, declare a NOP
>   * function where a BPF program can be attached as an fexit trampoline.
> @@ -24,6 +38,20 @@
>  #include <linux/lsm_hook_names.h>
>  #undef LSM_HOOK
>  
> +int bpf_lsm_set_enabled(const char *name, bool value)
> +{
> +	char toggle_fn_name[KSYM_NAME_LEN];
> +	void (*toggle_fn)(bool value);
> +
> +	snprintf(toggle_fn_name, KSYM_NAME_LEN, "%s_set_enabled", name);
> +	toggle_fn = (void *)kallsyms_lookup_name(toggle_fn_name);
> +	if (!toggle_fn)
> +		return -ESRCH;
> +
> +	toggle_fn(value);
> +	return 0;
> +}
> +
>  const struct bpf_prog_ops lsm_prog_ops = {
>  };
>  
> diff --git a/security/security.c b/security/security.c
> index aa111392a700..569cc07d5e34 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -804,6 +804,13 @@ int security_vm_enough_memory_mm(struct mm_struct *mm, long pages)
>  			break;
>  		}
>  	}
> +#ifdef CONFIG_BPF_LSM
> +	if (HAS_BPF_LSM_PROG(vm_enough_memory)) {
> +		rc = bpf_lsm_vm_enough_memory(mm, pages);
> +		if (rc <= 0)
> +			cap_sys_admin = 0;
> +	}
> +#endif
>  	return __vm_enough_memory(mm, pages, cap_sys_admin);
>  }
>  
> @@ -1350,6 +1357,13 @@ int security_inode_getsecurity(struct inode *inode, const char *name, void **buf
>  		if (rc != -EOPNOTSUPP)
>  			return rc;
>  	}
> +#ifdef CONFIG_BPF_LSM
> +	if (HAS_BPF_LSM_PROG(inode_getsecurity)) {
> +		rc = bpf_lsm_inode_getsecurity(inode, name, buffer, alloc);
> +		if (rc != -EOPNOTSUPP)
> +			return rc;
> +	}
> +#endif
>  	return -EOPNOTSUPP;
>  }
>  
> @@ -1369,6 +1383,14 @@ int security_inode_setsecurity(struct inode *inode, const char *name, const void
>  		if (rc != -EOPNOTSUPP)
>  			return rc;
>  	}
> +#ifdef CONFIG_BPF_LSM
> +	if (HAS_BPF_LSM_PROG(inode_setsecurity)) {
> +		rc = bpf_lsm_inode_setsecurity(inode, name, value, size,
> +					       flags);
> +		if (rc != -EOPNOTSUPP)
> +			return rc;
> +	}
> +#endif
>  	return -EOPNOTSUPP;
>  }
>  
> @@ -1754,6 +1776,12 @@ int security_task_prctl(int option, unsigned long arg2, unsigned long arg3,
>  				break;
>  		}
>  	}
> +#ifdef CONFIG_BPF_LSM
> +	if (HAS_BPF_LSM_PROG(task_prctl)) {
> +		if (rc == -ENOSYS)
> +			rc = bpf_lsm_task_prctl(option, arg2, arg3, arg4, arg5);
> +	}
> +#endif
>  	return rc;
>  }
>  
> @@ -2334,6 +2362,10 @@ int security_xfrm_state_pol_flow_match(struct xfrm_state *x,
>  		rc = hp->hook.xfrm_state_pol_flow_match(x, xp, fl);
>  		break;
>  	}
> +#ifdef CONFIG_BPF_LSM
> +	if (HAS_BPF_LSM_PROG(xfrm_state_pol_flow_match))
> +		rc = bpf_lsm_xfrm_state_pol_flow_match(x, xp, fl);
> +#endif
>  	return rc;
>  }
>  


  reply index

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-20 17:52 [PATCH bpf-next v4 0/8] MAC and Audit policy using eBPF (KRSI) KP Singh
2020-02-20 17:52 ` [PATCH bpf-next v4 1/8] bpf: Introduce BPF_PROG_TYPE_LSM KP Singh
2020-02-20 17:52 ` [PATCH bpf-next v4 2/8] security: Refactor declaration of LSM hooks KP Singh
2020-02-20 17:52 ` [PATCH bpf-next v4 3/8] bpf: lsm: provide attachment points for BPF LSM programs KP Singh
2020-02-21  2:25   ` Alexei Starovoitov
2020-02-21 11:47     ` KP Singh
     [not found]   ` <0ef26943-9619-3736-4452-fec536a8d169@schaufler-ca.com>
2020-02-21 11:44     ` KP Singh
2020-02-21 18:23       ` Casey Schaufler
     [not found]     ` <202002211946.A23A987@keescook>
2020-02-23 22:08       ` Alexei Starovoitov
2020-02-24 16:32         ` Casey Schaufler
2020-02-24 17:13           ` KP Singh
2020-02-24 18:45             ` Casey Schaufler
2020-02-24 21:41               ` Kees Cook
2020-02-24 22:29                 ` Casey Schaufler
2020-02-25  5:41                 ` Alexei Starovoitov
2020-02-25 15:31                   ` Kees Cook
2020-02-25 19:31                   ` KP Singh
2020-02-26  0:30                   ` Casey Schaufler
2020-02-26  5:15                     ` KP Singh
2020-02-26 15:35                       ` Casey Schaufler
2020-02-25 19:29                 ` KP Singh
2020-02-20 17:52 ` [PATCH bpf-next v4 4/8] bpf: lsm: Add support for enabling/disabling BPF hooks KP Singh
2020-02-21 18:57   ` Casey Schaufler [this message]
2020-02-21 19:11     ` James Morris
2020-02-22  4:26   ` Kees Cook
2020-02-20 17:52 ` [PATCH bpf-next v4 5/8] bpf: lsm: Implement attach, detach and execution KP Singh
2020-02-21  2:17   ` Alexei Starovoitov
2020-02-21 12:02     ` KP Singh
2020-02-20 17:52 ` [PATCH bpf-next v4 6/8] tools/libbpf: Add support for BPF_PROG_TYPE_LSM KP Singh
2020-02-25  6:45   ` Andrii Nakryiko
2020-02-20 17:52 ` [PATCH bpf-next v4 7/8] bpf: lsm: Add selftests " KP Singh
2020-02-20 17:52 ` [PATCH bpf-next v4 8/8] bpf: lsm: Add Documentation KP Singh
2020-02-21 19:19 ` [PATCH bpf-next v4 0/8] MAC and Audit policy using eBPF (KRSI) Casey Schaufler
2020-02-21 19:41   ` KP Singh
2020-02-21 22:31     ` Casey Schaufler
2020-02-21 23:09       ` KP Singh
2020-02-21 23:49         ` Casey Schaufler
2020-02-22  0:22       ` Kees Cook
2020-02-22  1:04         ` Casey Schaufler
2020-02-22  3:36           ` Kees Cook
2020-02-27 18:40 ` Dr. Greg

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=bb32f155-5213-71df-c679-85c614c0ac26@schaufler-ca.com \
    --to=casey@schaufler-ca.com \
    --cc=bpf@vger.kernel.org \
    --cc=kpsingh@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

BPF Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/bpf/0 bpf/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 bpf bpf/ https://lore.kernel.org/bpf \
		bpf@vger.kernel.org
	public-inbox-index bpf

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.bpf


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git