2019-09-10 18:57:44

by KP Singh

[permalink] [raw]
Subject: [RFC v1 00/14] Kernel Runtime Security Instrumentation

From: KP Singh <[email protected]>

# Motivation

Signaling and mitigation are two key aspects of security which go
hand-in-hand. Signals provide the necessary context to narrow down a
malicious actor and are key to creating effective MAC policies to
mitigate/prevent the malicious actor.

One can obtain signals from the kernel by using the audit infrastructure.
System-wide MAC is done separately as a part of the LSMs (eg. SELinux,
AppArmor).

Augmenting the signal information provided by audit requires kernel
changes to audit, its policy language and user-space components.
Furthermore, building a MAC policy based on the newly added signal
requires changes to various LSMs and their own respective policy
languages.

KRSI attempts to solve this problem by providing a common policy API in
the form of security focussed eBPF helpers and a common surface for
creating dynamic (not requiring re-compilation of the kernel) MAC and
Audit policies by attaching eBPF programs to the various LSM hooks.


# Why an LSM?

Linux Security Modules target security behaviours rather than the API. For
example, it's easy to miss out a newly added system call for executing
processes (eg. execve, execveat etc.) while the LSM framework ensures that
all process executions trigger the relevant hooks irrespective of how the
process was executed.

Allowing users to attach eBPF programs to LSM hooks also benefits the LSM
eco-system by enabling feedback from the security community about the kind
of behaviours that the LSM should be targeting.


# How does it work?

NOTE: The cover letter will be assimilated into the documentation
(Documentation/security/) as a part of the patch series.

## SecurityFS Interface

KRSI hooks create a file in securityfs to which eBPF programs can be
attached. This file maps to a single LSM hook but adds a layer of
indirection thus shielding the user-space from any changes to the
underlying hook. This includes changes like renaming of the underlying
hook or replacing the hook with another that maps better to the security
behaviour represented.

For Example:

/sys/kernel/security/krsi/process_execution -> bprm_check_security

## eBPF Program / Helpers

In order to keep things simple for the user, KRSI intends to provide one
eBPF program type. Since, there is only one type of context for a given
eBPF program type, the members of the KRSI context are selectively
populated depending on the hook.

KRSI is conservative about the access into the context and expects users
to use the helpers as an API for getting the required information. This
helps limit the internals of the LSM exposed to the user and maintain
backward compatibility. It also allows the maintainers to update the
structure of the context without worrying about breaking user-space.

The intention is to keep the helpers precise and not depend on kernel
headers or internals strengthening the backward compatibility and
usability across a large and diverse fleet.

Precise helpers also help in tackling seemingly intractable problems. For
example, a helper to dump all environment variables is hard because the
environment variables can be 32 pages long. While a helper to only extract
certain relevant environment variables (e.g. LD_PRELOAD, HISTFILESIZE)
helps in building a more reliable and precise signal with lower overhead.

## Attachment

A privileged (CAP_SYS_ADMIN for loading / attaching eBPF programs)
user-space program opens the securityfs file and the eBPF program
(BPF_PROG_LOAD) and attaches (BPF_PROG_ATTACH) the eBPF program to the
hook.

hook_fd = open("/sys/kernel/security/krsi/process_execution", O_RDWR)
prog_fd = bpf(BPF_PROG_LOAD, ...)
bpf(BPF_PROG_ATTACH, hook_fd, prog_fd)

There can be more than one program attached to a hook and attaching a
program with the same name replaces the existing program. The user can see
the programs attached to the hook as follows:

cat /sys/kernel/security/krsi/process_execution
env_dumper
my_auditor

## Audit / MAC Policy

The userspace controls the policy by installing eBPF programs to the LSM
hook. If any of the attached eBPF programs return an error (-ENOPERM),
the action represented by the hook is denied.

The audit logs are written using a format chosen by the eBPF program to
the perf events buffer (using bpf_perf_event_output) and can be further
processed in user-space.


# Current Status

The RFC provides a proto-type implementation with a few helpers and a
sample program samples/bpf/krsi_kern.c that hooks into process execution
and logs specific environment variables to the perf events buffer.

The user-space program samples/bpf/krsi_user.c loads the eBPF program,
configures the environment variable that needs to be audited and listens
for perf events.


KP Singh (14):
krsi: Add a skeleton and config options for the KRSI LSM
krsi: Introduce types for KRSI eBPF
bpf: krsi: sync BPF UAPI header with tools
krsi: Add support in libbpf for BPF_PROG_TYPE_KRSI
krsi: Initialize KRSI hooks and create files in securityfs
krsi: Implement eBPF operations, attachment and execution
krsi: Check for premissions on eBPF attachment
krsi: Show attached program names in hook read handler.
krsi: Add a helper function for bpf_perf_event_output
krsi: Handle attachment of the same program
krsi: Pin argument pages in bprm_check_security hook
krsi: Add an eBPF helper function to get the value of an env variable
krsi: Provide an example to read and log environment variables
krsi: Pin arg pages only when needed

MAINTAINERS | 8 +
include/linux/bpf_types.h | 3 +
include/linux/krsi.h | 19 ++
include/uapi/linux/bpf.h | 44 ++-
kernel/bpf/syscall.c | 7 +
samples/bpf/.gitignore | 1 +
samples/bpf/Makefile | 3 +
samples/bpf/krsi_helpers.h | 31 ++
samples/bpf/krsi_kern.c | 52 ++++
samples/bpf/krsi_user.c | 202 +++++++++++++
security/Kconfig | 1 +
security/Makefile | 2 +
security/krsi/Kconfig | 22 ++
security/krsi/Makefile | 3 +
security/krsi/include/hooks.h | 22 ++
security/krsi/include/krsi_fs.h | 19 ++
security/krsi/include/krsi_init.h | 106 +++++++
security/krsi/krsi.c | 157 ++++++++++
security/krsi/krsi_fs.c | 190 ++++++++++++
security/krsi/ops.c | 342 ++++++++++++++++++++++
tools/include/uapi/linux/bpf.h | 44 ++-
tools/lib/bpf/libbpf.c | 4 +
tools/lib/bpf/libbpf.h | 2 +
tools/lib/bpf/libbpf.map | 2 +
tools/lib/bpf/libbpf_probes.c | 1 +
tools/testing/selftests/bpf/bpf_helpers.h | 3 +
26 files changed, 1288 insertions(+), 2 deletions(-)
create mode 100644 include/linux/krsi.h
create mode 100644 samples/bpf/krsi_helpers.h
create mode 100644 samples/bpf/krsi_kern.c
create mode 100644 samples/bpf/krsi_user.c
create mode 100644 security/krsi/Kconfig
create mode 100644 security/krsi/Makefile
create mode 100644 security/krsi/include/hooks.h
create mode 100644 security/krsi/include/krsi_fs.h
create mode 100644 security/krsi/include/krsi_init.h
create mode 100644 security/krsi/krsi.c
create mode 100644 security/krsi/krsi_fs.c
create mode 100644 security/krsi/ops.c

--
2.20.1


2019-09-10 18:57:47

by KP Singh

[permalink] [raw]
Subject: [RFC v1 03/14] bpf: krsi: sync BPF UAPI header with tools

From: KP Singh <[email protected]>

Signed-off-by: KP Singh <[email protected]>
---
tools/include/uapi/linux/bpf.h | 2 ++
1 file changed, 2 insertions(+)

diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index a5aa7d3ac6a1..32ab38f1a2fe 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -171,6 +171,7 @@ enum bpf_prog_type {
BPF_PROG_TYPE_CGROUP_SYSCTL,
BPF_PROG_TYPE_RAW_TRACEPOINT_WRITABLE,
BPF_PROG_TYPE_CGROUP_SOCKOPT,
+ BPF_PROG_TYPE_KRSI,
};

enum bpf_attach_type {
@@ -197,6 +198,7 @@ enum bpf_attach_type {
BPF_CGROUP_UDP6_RECVMSG,
BPF_CGROUP_GETSOCKOPT,
BPF_CGROUP_SETSOCKOPT,
+ BPF_KRSI,
__MAX_BPF_ATTACH_TYPE
};

--
2.20.1

2019-09-10 18:57:55

by KP Singh

[permalink] [raw]
Subject: [RFC v1 06/14] krsi: Implement eBPF operations, attachment and execution

From: KP Singh <[email protected]>

A user space program can attach an eBPF program by:

hook_fd = open("/sys/kernel/security/krsi/process_execution", O_RDWR)
prog_fd = bpf(BPF_PROG_LOAD, ...)
bpf(BPF_PROG_ATTACH, hook_fd, prog_fd)

When such an attach call is received, the attachment logic looks up the
dentry and appends the program to the bpf_prog_array.

The BPF programs are stored in a bpf_prog_array and writes to the array
are guarded by a mutex. The eBPF programs are executed as a part of the
LSM hook they are attached to. If any of the eBPF programs return
an error (-ENOPERM) the action represented by the hook is denied.

Signed-off-by: KP Singh <[email protected]>
---
include/linux/krsi.h | 18 ++++++
kernel/bpf/syscall.c | 3 +-
security/krsi/include/krsi_init.h | 51 +++++++++++++++
security/krsi/krsi.c | 13 +++-
security/krsi/krsi_fs.c | 28 ++++++++
security/krsi/ops.c | 102 ++++++++++++++++++++++++++++++
6 files changed, 213 insertions(+), 2 deletions(-)
create mode 100644 include/linux/krsi.h

diff --git a/include/linux/krsi.h b/include/linux/krsi.h
new file mode 100644
index 000000000000..c7d1790d0c1f
--- /dev/null
+++ b/include/linux/krsi.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _KRSI_H
+#define _KRSI_H
+
+#include <linux/bpf.h>
+
+#ifdef CONFIG_SECURITY_KRSI
+int krsi_prog_attach(const union bpf_attr *attr, struct bpf_prog *prog);
+#else
+static inline int krsi_prog_attach(const union bpf_attr *attr,
+ struct bpf_prog *prog)
+{
+ return -EINVAL;
+}
+#endif /* CONFIG_SECURITY_KRSI */
+
+#endif /* _KRSI_H */
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index f38a539f7e67..ab063ed84258 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -4,6 +4,7 @@
#include <linux/bpf.h>
#include <linux/bpf_trace.h>
#include <linux/bpf_lirc.h>
+#include <linux/krsi.h>
#include <linux/btf.h>
#include <linux/syscalls.h>
#include <linux/slab.h>
@@ -1950,7 +1951,7 @@ static int bpf_prog_attach(const union bpf_attr *attr)
ret = lirc_prog_attach(attr, prog);
break;
case BPF_PROG_TYPE_KRSI:
- ret = -EINVAL;
+ ret = krsi_prog_attach(attr, prog);
break;
case BPF_PROG_TYPE_FLOW_DISSECTOR:
ret = skb_flow_dissector_bpf_prog_attach(attr, prog);
diff --git a/security/krsi/include/krsi_init.h b/security/krsi/include/krsi_init.h
index 68755182a031..4e17ecacd4ed 100644
--- a/security/krsi/include/krsi_init.h
+++ b/security/krsi/include/krsi_init.h
@@ -5,12 +5,29 @@

#include "krsi_fs.h"

+#include <linux/binfmts.h>
+
enum krsi_hook_type {
PROCESS_EXECUTION,
__MAX_KRSI_HOOK_TYPE, /* delimiter */
};

extern int krsi_fs_initialized;
+
+struct krsi_bprm_ctx {
+ struct linux_binprm *bprm;
+};
+
+/*
+ * krsi_ctx is the context that is passed to all KRSI eBPF
+ * programs.
+ */
+struct krsi_ctx {
+ union {
+ struct krsi_bprm_ctx bprm_ctx;
+ };
+};
+
/*
* The LSM creates one file per hook.
*
@@ -33,10 +50,44 @@ struct krsi_hook {
* The dentry of the file created in securityfs.
*/
struct dentry *h_dentry;
+ /*
+ * The mutex must be held when updating the progs attached to the hook.
+ */
+ struct mutex mutex;
+ /*
+ * The eBPF programs that are attached to this hook.
+ */
+ struct bpf_prog_array __rcu *progs;
};

extern struct krsi_hook krsi_hooks_list[];

+static inline int krsi_run_progs(enum krsi_hook_type t, struct krsi_ctx *ctx)
+{
+ struct bpf_prog_array_item *item;
+ struct bpf_prog *prog;
+ struct krsi_hook *h = &krsi_hooks_list[t];
+ int ret, retval = 0;
+
+ preempt_disable();
+ rcu_read_lock();
+
+ item = rcu_dereference(h->progs)->items;
+ while ((prog = READ_ONCE(item->prog))) {
+ ret = BPF_PROG_RUN(prog, ctx);
+ if (ret < 0) {
+ retval = ret;
+ goto out;
+ }
+ item++;
+ }
+
+out:
+ rcu_read_unlock();
+ preempt_enable();
+ return IS_ENABLED(CONFIG_SECURITY_KRSI_ENFORCE) ? retval : 0;
+}
+
#define krsi_for_each_hook(hook) \
for ((hook) = &krsi_hooks_list[0]; \
(hook) < &krsi_hooks_list[__MAX_KRSI_HOOK_TYPE]; \
diff --git a/security/krsi/krsi.c b/security/krsi/krsi.c
index 77d7e2f91172..d3a4a361c192 100644
--- a/security/krsi/krsi.c
+++ b/security/krsi/krsi.c
@@ -1,6 +1,9 @@
// SPDX-License-Identifier: GPL-2.0

#include <linux/lsm_hooks.h>
+#include <linux/filter.h>
+#include <linux/bpf.h>
+#include <linux/binfmts.h>

#include "krsi_init.h"

@@ -16,7 +19,15 @@ struct krsi_hook krsi_hooks_list[] = {

static int krsi_process_execution(struct linux_binprm *bprm)
{
- return 0;
+ int ret;
+ struct krsi_ctx ctx;
+
+ ctx.bprm_ctx = (struct krsi_bprm_ctx) {
+ .bprm = bprm,
+ };
+
+ ret = krsi_run_progs(PROCESS_EXECUTION, &ctx);
+ return ret;
}

static struct security_hook_list krsi_hooks[] __lsm_ro_after_init = {
diff --git a/security/krsi/krsi_fs.c b/security/krsi/krsi_fs.c
index 604f826cee5c..3ba18b52ce85 100644
--- a/security/krsi/krsi_fs.c
+++ b/security/krsi/krsi_fs.c
@@ -5,6 +5,8 @@
#include <linux/file.h>
#include <linux/fs.h>
#include <linux/types.h>
+#include <linux/filter.h>
+#include <linux/bpf.h>
#include <linux/security.h>

#include "krsi_fs.h"
@@ -27,12 +29,29 @@ bool is_krsi_hook_file(struct file *f)

static void __init krsi_free_hook(struct krsi_hook *h)
{
+ struct bpf_prog_array_item *item;
+ /*
+ * This function is __init so we are guarranteed that there will be
+ * no concurrent access.
+ */
+ struct bpf_prog_array *progs = rcu_dereference_raw(h->progs);
+
+ if (progs) {
+ item = progs->items;
+ while (item->prog) {
+ bpf_prog_put(item->prog);
+ item++;
+ }
+ bpf_prog_array_free(progs);
+ }
+
securityfs_remove(h->h_dentry);
h->h_dentry = NULL;
}

static int __init krsi_init_hook(struct krsi_hook *h, struct dentry *parent)
{
+ struct bpf_prog_array __rcu *progs;
struct dentry *h_dentry;
int ret;

@@ -41,6 +60,15 @@ static int __init krsi_init_hook(struct krsi_hook *h, struct dentry *parent)

if (IS_ERR(h_dentry))
return PTR_ERR(h_dentry);
+
+ mutex_init(&h->mutex);
+ progs = bpf_prog_array_alloc(0, GFP_KERNEL);
+ if (!progs) {
+ ret = -ENOMEM;
+ goto error;
+ }
+
+ RCU_INIT_POINTER(h->progs, progs);
h_dentry->d_fsdata = h;
h->h_dentry = h_dentry;
return 0;
diff --git a/security/krsi/ops.c b/security/krsi/ops.c
index f2de3bd9621e..cf4d06189aa1 100644
--- a/security/krsi/ops.c
+++ b/security/krsi/ops.c
@@ -1,10 +1,112 @@
// SPDX-License-Identifier: GPL-2.0

+#include <linux/err.h>
+#include <linux/types.h>
#include <linux/filter.h>
#include <linux/bpf.h>
+#include <linux/security.h>
+#include <linux/krsi.h>
+
+#include "krsi_init.h"
+#include "krsi_fs.h"
+
+extern struct krsi_hook krsi_hooks_list[];
+
+static struct krsi_hook *get_hook_from_fd(int fd)
+{
+ struct fd f = fdget(fd);
+ struct krsi_hook *h;
+ int ret;
+
+ if (!f.file) {
+ ret = -EBADF;
+ goto error;
+ }
+
+ if (!is_krsi_hook_file(f.file)) {
+ ret = -EINVAL;
+ goto error;
+ }
+
+ /*
+ * The securityfs dentry never disappears, so we don't need to take a
+ * reference to it.
+ */
+ h = file_dentry(f.file)->d_fsdata;
+ if (WARN_ON(!h)) {
+ ret = -EINVAL;
+ goto error;
+ }
+ fdput(f);
+ return h;
+
+error:
+ fdput(f);
+ return ERR_PTR(ret);
+}
+
+int krsi_prog_attach(const union bpf_attr *attr, struct bpf_prog *prog)
+{
+ struct bpf_prog_array *old_array;
+ struct bpf_prog_array *new_array;
+ struct krsi_hook *h;
+ int ret = 0;
+
+ h = get_hook_from_fd(attr->target_fd);
+ if (IS_ERR(h))
+ return PTR_ERR(h);
+
+ mutex_lock(&h->mutex);
+ old_array = rcu_dereference_protected(h->progs,
+ lockdep_is_held(&h->mutex));
+
+ ret = bpf_prog_array_copy(old_array, NULL, prog, &new_array);
+ if (ret < 0) {
+ ret = -ENOMEM;
+ goto unlock;
+ }
+
+ rcu_assign_pointer(h->progs, new_array);
+ bpf_prog_array_free(old_array);
+
+unlock:
+ mutex_unlock(&h->mutex);
+ return ret;
+}

const struct bpf_prog_ops krsi_prog_ops = {
};

+static bool krsi_prog_is_valid_access(int off, int size,
+ enum bpf_access_type type,
+ const struct bpf_prog *prog,
+ struct bpf_insn_access_aux *info)
+{
+ /*
+ * KRSI is conservative about any direct access in eBPF to
+ * prevent the users from depending on the internals of the kernel and
+ * aims at providing a rich eco-system of safe eBPF helpers as an API
+ * for accessing relevant information from the context.
+ */
+ return false;
+}
+
+static const struct bpf_func_proto *krsi_prog_func_proto(enum bpf_func_id
+ func_id,
+ const struct bpf_prog
+ *prog)
+{
+ switch (func_id) {
+ case BPF_FUNC_map_lookup_elem:
+ return &bpf_map_lookup_elem_proto;
+ case BPF_FUNC_get_current_pid_tgid:
+ return &bpf_get_current_pid_tgid_proto;
+ default:
+ return NULL;
+ }
+}
+
const struct bpf_verifier_ops krsi_verifier_ops = {
+ .get_func_proto = krsi_prog_func_proto,
+ .is_valid_access = krsi_prog_is_valid_access,
};
--
2.20.1

2019-09-10 18:58:07

by KP Singh

[permalink] [raw]
Subject: [RFC v1 08/14] krsi: Show attached program names in hook read handler.

From: KP Singh <[email protected]>

For inspectability the system administrator should be able to view the
list of active KRSI programs:

bash # cat /sys/kernel/security/krsi/process_execution
bpf_prog1

Signed-off-by: KP Singh <[email protected]>
---
security/krsi/krsi_fs.c | 76 ++++++++++++++++++++++++++++++++++++++++-
1 file changed, 75 insertions(+), 1 deletion(-)

diff --git a/security/krsi/krsi_fs.c b/security/krsi/krsi_fs.c
index 3ba18b52ce85..0ebf4fabe935 100644
--- a/security/krsi/krsi_fs.c
+++ b/security/krsi/krsi_fs.c
@@ -6,6 +6,7 @@
#include <linux/fs.h>
#include <linux/types.h>
#include <linux/filter.h>
+#include <linux/seq_file.h>
#include <linux/bpf.h>
#include <linux/security.h>

@@ -16,8 +17,81 @@ extern struct krsi_hook krsi_hooks_list[];

static struct dentry *krsi_dir;

+static void *seq_start(struct seq_file *m, loff_t *pos)
+ __acquires(rcu)
+{
+ struct krsi_hook *h;
+ struct dentry *dentry;
+ struct bpf_prog_array *progs;
+ struct bpf_prog_array_item *item;
+
+ /*
+ * rcu_read_lock() must be held before any return statement
+ * because the stop() will always be called and thus call
+ * rcu_read_unlock()
+ */
+ rcu_read_lock();
+
+ dentry = file_dentry(m->file);
+ h = dentry->d_fsdata;
+ if (WARN_ON(!h))
+ return ERR_PTR(-EFAULT);
+
+ progs = rcu_dereference(h->progs);
+ if ((*pos) >= bpf_prog_array_length(progs))
+ return NULL;
+
+ item = progs->items + *pos;
+ if (!item->prog)
+ return NULL;
+
+ return item;
+}
+
+static void *seq_next(struct seq_file *m, void *v, loff_t *pos)
+{
+ struct bpf_prog_array_item *item = v;
+
+ item++;
+ ++*pos;
+
+ if (!item->prog)
+ return NULL;
+
+ return item;
+}
+
+static void seq_stop(struct seq_file *m, void *v)
+ __releases(rcu)
+{
+ rcu_read_unlock();
+}
+
+static int show_prog(struct seq_file *m, void *v)
+{
+ struct bpf_prog_array_item *item = v;
+
+ seq_printf(m, "%s\n", item->prog->aux->name);
+ return 0;
+}
+
+static const struct seq_operations seq_ops = {
+ .show = show_prog,
+ .start = seq_start,
+ .next = seq_next,
+ .stop = seq_stop,
+};
+
+static int hook_open(struct inode *inode, struct file *file)
+{
+ return seq_open(file, &seq_ops);
+}
+
static const struct file_operations krsi_hook_ops = {
- .llseek = generic_file_llseek,
+ .open = hook_open,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = seq_release,
};

int krsi_fs_initialized;
--
2.20.1

2019-09-14 21:41:35

by Yonghong Song

[permalink] [raw]
Subject: Re: [RFC v1 06/14] krsi: Implement eBPF operations, attachment and execution



On 9/10/19 12:55 PM, KP Singh wrote:
> From: KP Singh <[email protected]>
>
> A user space program can attach an eBPF program by:
>
> hook_fd = open("/sys/kernel/security/krsi/process_execution", O_RDWR)
> prog_fd = bpf(BPF_PROG_LOAD, ...)
> bpf(BPF_PROG_ATTACH, hook_fd, prog_fd)
>
> When such an attach call is received, the attachment logic looks up the
> dentry and appends the program to the bpf_prog_array.
>
> The BPF programs are stored in a bpf_prog_array and writes to the array
> are guarded by a mutex. The eBPF programs are executed as a part of the
> LSM hook they are attached to. If any of the eBPF programs return
> an error (-ENOPERM) the action represented by the hook is denied.
>
> Signed-off-by: KP Singh <[email protected]>
> ---
> include/linux/krsi.h | 18 ++++++
> kernel/bpf/syscall.c | 3 +-
> security/krsi/include/krsi_init.h | 51 +++++++++++++++
> security/krsi/krsi.c | 13 +++-
> security/krsi/krsi_fs.c | 28 ++++++++
> security/krsi/ops.c | 102 ++++++++++++++++++++++++++++++
> 6 files changed, 213 insertions(+), 2 deletions(-)
> create mode 100644 include/linux/krsi.h
>
> diff --git a/include/linux/krsi.h b/include/linux/krsi.h
> new file mode 100644
> index 000000000000..c7d1790d0c1f
> --- /dev/null
> +++ b/include/linux/krsi.h
> @@ -0,0 +1,18 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef _KRSI_H
> +#define _KRSI_H
> +
> +#include <linux/bpf.h>
> +
> +#ifdef CONFIG_SECURITY_KRSI
> +int krsi_prog_attach(const union bpf_attr *attr, struct bpf_prog *prog);
> +#else
> +static inline int krsi_prog_attach(const union bpf_attr *attr,
> + struct bpf_prog *prog)
> +{
> + return -EINVAL;
> +}
> +#endif /* CONFIG_SECURITY_KRSI */
> +
> +#endif /* _KRSI_H */
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index f38a539f7e67..ab063ed84258 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -4,6 +4,7 @@
> #include <linux/bpf.h>
> #include <linux/bpf_trace.h>
> #include <linux/bpf_lirc.h>
> +#include <linux/krsi.h>
> #include <linux/btf.h>
> #include <linux/syscalls.h>
> #include <linux/slab.h>
> @@ -1950,7 +1951,7 @@ static int bpf_prog_attach(const union bpf_attr *attr)
> ret = lirc_prog_attach(attr, prog);
> break;
> case BPF_PROG_TYPE_KRSI:
> - ret = -EINVAL;
> + ret = krsi_prog_attach(attr, prog);
> break;
> case BPF_PROG_TYPE_FLOW_DISSECTOR:
> ret = skb_flow_dissector_bpf_prog_attach(attr, prog);
> diff --git a/security/krsi/include/krsi_init.h b/security/krsi/include/krsi_init.h
> index 68755182a031..4e17ecacd4ed 100644
> --- a/security/krsi/include/krsi_init.h
> +++ b/security/krsi/include/krsi_init.h
> @@ -5,12 +5,29 @@
>
> #include "krsi_fs.h"
>
> +#include <linux/binfmts.h>
> +
> enum krsi_hook_type {
> PROCESS_EXECUTION,
> __MAX_KRSI_HOOK_TYPE, /* delimiter */
> };
>
> extern int krsi_fs_initialized;
> +
> +struct krsi_bprm_ctx {
> + struct linux_binprm *bprm;
> +};
> +
> +/*
> + * krsi_ctx is the context that is passed to all KRSI eBPF
> + * programs.
> + */
> +struct krsi_ctx {
> + union {
> + struct krsi_bprm_ctx bprm_ctx;
> + };
> +};
> +
> /*
> * The LSM creates one file per hook.
> *
> @@ -33,10 +50,44 @@ struct krsi_hook {
> * The dentry of the file created in securityfs.
> */
> struct dentry *h_dentry;
> + /*
> + * The mutex must be held when updating the progs attached to the hook.
> + */
> + struct mutex mutex;
> + /*
> + * The eBPF programs that are attached to this hook.
> + */
> + struct bpf_prog_array __rcu *progs;
> };
>
> extern struct krsi_hook krsi_hooks_list[];
>
> +static inline int krsi_run_progs(enum krsi_hook_type t, struct krsi_ctx *ctx)
> +{
> + struct bpf_prog_array_item *item;
> + struct bpf_prog *prog;
> + struct krsi_hook *h = &krsi_hooks_list[t];
> + int ret, retval = 0;

Reverse christmas tree style?

> +
> + preempt_disable();

Do we need preempt_disable() here?

> + rcu_read_lock();
> +
> + item = rcu_dereference(h->progs)->items;
> + while ((prog = READ_ONCE(item->prog))) {
> + ret = BPF_PROG_RUN(prog, ctx);
> + if (ret < 0) {
> + retval = ret;
> + goto out;
> + }
> + item++;
> + }
> +
> +out:
> + rcu_read_unlock();
> + preempt_enable();
> + return IS_ENABLED(CONFIG_SECURITY_KRSI_ENFORCE) ? retval : 0;
> +}
> +
> #define krsi_for_each_hook(hook) \
> for ((hook) = &krsi_hooks_list[0]; \
> (hook) < &krsi_hooks_list[__MAX_KRSI_HOOK_TYPE]; \
> diff --git a/security/krsi/krsi.c b/security/krsi/krsi.c
> index 77d7e2f91172..d3a4a361c192 100644
> --- a/security/krsi/krsi.c
> +++ b/security/krsi/krsi.c
> @@ -1,6 +1,9 @@
> // SPDX-License-Identifier: GPL-2.0
>
> #include <linux/lsm_hooks.h>
> +#include <linux/filter.h>
> +#include <linux/bpf.h>
> +#include <linux/binfmts.h>
>
> #include "krsi_init.h"
>
> @@ -16,7 +19,15 @@ struct krsi_hook krsi_hooks_list[] = {
>
> static int krsi_process_execution(struct linux_binprm *bprm)
> {
> - return 0;
> + int ret;
> + struct krsi_ctx ctx;
> +
> + ctx.bprm_ctx = (struct krsi_bprm_ctx) {
> + .bprm = bprm,
> + };
> +
> + ret = krsi_run_progs(PROCESS_EXECUTION, &ctx);
> + return ret;
> }
>
> static struct security_hook_list krsi_hooks[] __lsm_ro_after_init = {
> diff --git a/security/krsi/krsi_fs.c b/security/krsi/krsi_fs.c
> index 604f826cee5c..3ba18b52ce85 100644
> --- a/security/krsi/krsi_fs.c
> +++ b/security/krsi/krsi_fs.c
> @@ -5,6 +5,8 @@
> #include <linux/file.h>
> #include <linux/fs.h>
> #include <linux/types.h>
> +#include <linux/filter.h>
> +#include <linux/bpf.h>
> #include <linux/security.h>
>
> #include "krsi_fs.h"
> @@ -27,12 +29,29 @@ bool is_krsi_hook_file(struct file *f)
>
> static void __init krsi_free_hook(struct krsi_hook *h)
> {
> + struct bpf_prog_array_item *item;
> + /*
> + * This function is __init so we are guarranteed that there will be
> + * no concurrent access.
> + */
> + struct bpf_prog_array *progs = rcu_dereference_raw(h->progs);
> +
> + if (progs) {

bpf_prog_array itself should never be null?

> + item = progs->items;
> + while (item->prog) {
> + bpf_prog_put(item->prog);
> + item++;
> + }
> + bpf_prog_array_free(progs);
> + }
> +
> securityfs_remove(h->h_dentry);
> h->h_dentry = NULL;
> }
>
> static int __init krsi_init_hook(struct krsi_hook *h, struct dentry *parent)
> {
> + struct bpf_prog_array __rcu *progs;
> struct dentry *h_dentry;
> int ret;
>
> @@ -41,6 +60,15 @@ static int __init krsi_init_hook(struct krsi_hook *h, struct dentry *parent)
>
> if (IS_ERR(h_dentry))
> return PTR_ERR(h_dentry);
> +
> + mutex_init(&h->mutex);
> + progs = bpf_prog_array_alloc(0, GFP_KERNEL);
> + if (!progs) {
> + ret = -ENOMEM;
> + goto error;
> + }
> +
> + RCU_INIT_POINTER(h->progs, progs);
> h_dentry->d_fsdata = h;
> h->h_dentry = h_dentry;
> return 0;
> diff --git a/security/krsi/ops.c b/security/krsi/ops.c
> index f2de3bd9621e..cf4d06189aa1 100644
> --- a/security/krsi/ops.c
> +++ b/security/krsi/ops.c
> @@ -1,10 +1,112 @@
> // SPDX-License-Identifier: GPL-2.0
>
> +#include <linux/err.h>
> +#include <linux/types.h>
> #include <linux/filter.h>
> #include <linux/bpf.h>
> +#include <linux/security.h>
> +#include <linux/krsi.h>
> +
> +#include "krsi_init.h"
> +#include "krsi_fs.h"
> +
> +extern struct krsi_hook krsi_hooks_list[];
> +
> +static struct krsi_hook *get_hook_from_fd(int fd)
> +{
> + struct fd f = fdget(fd);
> + struct krsi_hook *h;
> + int ret;
> +
> + if (!f.file) {
> + ret = -EBADF;
> + goto error;
> + }
> +
> + if (!is_krsi_hook_file(f.file)) {
> + ret = -EINVAL;
> + goto error;
> + }
> +
> + /*
> + * The securityfs dentry never disappears, so we don't need to take a
> + * reference to it.
> + */
> + h = file_dentry(f.file)->d_fsdata;
> + if (WARN_ON(!h)) {
> + ret = -EINVAL;
> + goto error;
> + }
> + fdput(f);
> + return h;
> +
> +error:
> + fdput(f);
> + return ERR_PTR(ret);
> +}
> +
> +int krsi_prog_attach(const union bpf_attr *attr, struct bpf_prog *prog)
> +{
> + struct bpf_prog_array *old_array;
> + struct bpf_prog_array *new_array;
> + struct krsi_hook *h;
> + int ret = 0;
> +
> + h = get_hook_from_fd(attr->target_fd);
> + if (IS_ERR(h))
> + return PTR_ERR(h);
> +
> + mutex_lock(&h->mutex);
> + old_array = rcu_dereference_protected(h->progs,
> + lockdep_is_held(&h->mutex));
> +
> + ret = bpf_prog_array_copy(old_array, NULL, prog, &new_array);
> + if (ret < 0) {
> + ret = -ENOMEM;
> + goto unlock;
> + }
> +
> + rcu_assign_pointer(h->progs, new_array);
> + bpf_prog_array_free(old_array);
> +
> +unlock:
> + mutex_unlock(&h->mutex);
> + return ret;
> +}
>
> const struct bpf_prog_ops krsi_prog_ops = {
> };
>
> +static bool krsi_prog_is_valid_access(int off, int size,
> + enum bpf_access_type type,
> + const struct bpf_prog *prog,
> + struct bpf_insn_access_aux *info)
> +{
> + /*
> + * KRSI is conservative about any direct access in eBPF to
> + * prevent the users from depending on the internals of the kernel and
> + * aims at providing a rich eco-system of safe eBPF helpers as an API
> + * for accessing relevant information from the context.
> + */
> + return false;
> +}
> +
> +static const struct bpf_func_proto *krsi_prog_func_proto(enum bpf_func_id
> + func_id,
> + const struct bpf_prog
> + *prog)
> +{
> + switch (func_id) {
> + case BPF_FUNC_map_lookup_elem:
> + return &bpf_map_lookup_elem_proto;
> + case BPF_FUNC_get_current_pid_tgid:
> + return &bpf_get_current_pid_tgid_proto;
> + default:
> + return NULL;
> + }
> +}
> +
> const struct bpf_verifier_ops krsi_verifier_ops = {
> + .get_func_proto = krsi_prog_func_proto,
> + .is_valid_access = krsi_prog_is_valid_access,
> };
>

2019-09-15 02:20:19

by Yonghong Song

[permalink] [raw]
Subject: Re: Re: [RFC v1 06/14] krsi: Implement eBPF operations, attachment and execution



On 9/14/19 5:56 PM, Yonghong Song wrote:
>
>
> On 9/10/19 12:55 PM, KP Singh wrote:
>> From: KP Singh <[email protected]>
>>
>> A user space program can attach an eBPF program by:
>>
>> hook_fd = open("/sys/kernel/security/krsi/process_execution", O_RDWR)
>> prog_fd = bpf(BPF_PROG_LOAD, ...)
>> bpf(BPF_PROG_ATTACH, hook_fd, prog_fd)
>>
>> When such an attach call is received, the attachment logic looks up the
>> dentry and appends the program to the bpf_prog_array.
>>
>> The BPF programs are stored in a bpf_prog_array and writes to the array
>> are guarded by a mutex. The eBPF programs are executed as a part of the
>> LSM hook they are attached to. If any of the eBPF programs return
>> an error (-ENOPERM) the action represented by the hook is denied.
>>
>> Signed-off-by: KP Singh <[email protected]>
>> ---
>> include/linux/krsi.h | 18 ++++++
>> kernel/bpf/syscall.c | 3 +-
>> security/krsi/include/krsi_init.h | 51 +++++++++++++++
>> security/krsi/krsi.c | 13 +++-
>> security/krsi/krsi_fs.c | 28 ++++++++
>> security/krsi/ops.c | 102 ++++++++++++++++++++++++++++++
>> 6 files changed, 213 insertions(+), 2 deletions(-)
>> create mode 100644 include/linux/krsi.h
>>
[...]
>>
>> +static inline int krsi_run_progs(enum krsi_hook_type t, struct krsi_ctx *ctx)
>> +{
>> + struct bpf_prog_array_item *item;
>> + struct bpf_prog *prog;
>> + struct krsi_hook *h = &krsi_hooks_list[t];
>> + int ret, retval = 0;
>
> Reverse christmas tree style?
>
>> +
>> + preempt_disable();
>
> Do we need preempt_disable() here?

From the following patches, I see perf_event_output() helper
and per-cpu array usage. So, indeed preempt_disable() is needed.

>
>> + rcu_read_lock();
>> +
>> + item = rcu_dereference(h->progs)->items;
>> + while ((prog = READ_ONCE(item->prog))) {
>> + ret = BPF_PROG_RUN(prog, ctx);
>> + if (ret < 0) {
>> + retval = ret;
>> + goto out;
>> + }
>> + item++;
>> + }
>> +
>> +out:
>> + rcu_read_unlock();
>> + preempt_enable();
>> + return IS_ENABLED(CONFIG_SECURITY_KRSI_ENFORCE) ? retval : 0;
>> +}
>> +
[...]