diff mbox series

[bpf-next,v1,04/13] bpf: lsm: Allow btf_id based attachment for LSM hooks

Message ID 20191220154208.15895-5-kpsingh@chromium.org (mailing list archive)
State New, archived
Headers show
Series MAC and Audit policy using eBPF (KRSI) | expand

Commit Message

KP Singh Dec. 20, 2019, 3:41 p.m. UTC 
From: KP Singh <kpsingh@google.com>

Refactor and re-use most of the logic for BPF_PROG_TYPE_TRACING with a few
changes.

- The LSM hook BTF types are prefixed with "lsm_btf_"
- These types do not need the first (void *) pointer argument. The verifier
  only looks for this argument if prod->aux->attach_btf_trace is set.

Signed-off-by: KP Singh <kpsingh@google.com>
---
 kernel/bpf/syscall.c  |  1 +
 kernel/bpf/verifier.c | 83 ++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 80 insertions(+), 4 deletions(-)

Comments

Andrii Nakryiko Dec. 23, 2019, 11:54 p.m. UTC | #1 
On Fri, Dec 20, 2019 at 7:42 AM KP Singh <kpsingh@chromium.org> wrote:
>
> From: KP Singh <kpsingh@google.com>
>
> Refactor and re-use most of the logic for BPF_PROG_TYPE_TRACING with a few
> changes.
>
> - The LSM hook BTF types are prefixed with "lsm_btf_"

btf_trace_ and btf_struct_ops all have btf_ first, let's keep this consistent.

> - These types do not need the first (void *) pointer argument. The verifier
>   only looks for this argument if prod->aux->attach_btf_trace is set.
>
> Signed-off-by: KP Singh <kpsingh@google.com>
> ---
>  kernel/bpf/syscall.c  |  1 +
>  kernel/bpf/verifier.c | 83 ++++++++++++++++++++++++++++++++++++++++---
>  2 files changed, 80 insertions(+), 4 deletions(-)
>

[...]

> +
> +       t = btf_type_by_id(btf_vmlinux, btf_id);
> +       if (!t) {
> +               verbose(env, "attach_btf_id %u is invalid\n", btf_id);
> +               return -EINVAL;
> +       }
> +
> +       tname = btf_name_by_offset(btf_vmlinux, t->name_off);
> +       if (!tname) {

it can be empty, so better: !tname || !tname[0]

> +               verbose(env, "attach_btf_id %u doesn't have a name\n", btf_id);
> +               return -EINVAL;
> +       }
> +

[...]
KP Singh Dec. 30, 2019, 7:22 p.m. UTC | #2 
On 23-Dez 15:54, Andrii Nakryiko wrote:
> On Fri, Dec 20, 2019 at 7:42 AM KP Singh <kpsingh@chromium.org> wrote:
> >
> > From: KP Singh <kpsingh@google.com>
> >
> > Refactor and re-use most of the logic for BPF_PROG_TYPE_TRACING with a few
> > changes.
> >
> > - The LSM hook BTF types are prefixed with "lsm_btf_"

Got rid of this for v2 as we are using trampoline. Will keep this in
mind if we ever need to generate type information. Thanks!

> 
> btf_trace_ and btf_struct_ops all have btf_ first, let's keep this consistent.
> 
> > - These types do not need the first (void *) pointer argument. The verifier
> >   only looks for this argument if prod->aux->attach_btf_trace is set.
> >
> > Signed-off-by: KP Singh <kpsingh@google.com>
> > ---
> >  kernel/bpf/syscall.c  |  1 +
> >  kernel/bpf/verifier.c | 83 ++++++++++++++++++++++++++++++++++++++++---
> >  2 files changed, 80 insertions(+), 4 deletions(-)
> >
> 
> [...]
> 
> > +
> > +       t = btf_type_by_id(btf_vmlinux, btf_id);
> > +       if (!t) {
> > +               verbose(env, "attach_btf_id %u is invalid\n", btf_id);
> > +               return -EINVAL;
> > +       }
> > +
> > +       tname = btf_name_by_offset(btf_vmlinux, t->name_off);
> > +       if (!tname) {
> 
> it can be empty, so better: !tname || !tname[0]

Will fix the usages in v2. 

- KP
> 
> > +               verbose(env, "attach_btf_id %u doesn't have a name\n", btf_id);
> > +               return -EINVAL;
> > +       }
> > +
> 
> [...]
diff mbox series

Patch

diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 5a773fc6f9f5..4fcaf6042c07 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1642,6 +1642,7 @@  bpf_prog_load_check_attach(enum bpf_prog_type prog_type,
 {
 	switch (prog_type) {
 	case BPF_PROG_TYPE_TRACING:
+	case BPF_PROG_TYPE_LSM:
 		if (btf_id > BTF_MAX_TYPE)
 			return -EINVAL;
 		break;
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index a0482e1c4a77..0d1231d9c1ef 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -9504,7 +9504,71 @@  static void print_verification_stats(struct bpf_verifier_env *env)
 		env->peak_states, env->longest_mark_read_walk);
 }
 
-static int check_attach_btf_id(struct bpf_verifier_env *env)
+/*
+ * LSM hooks have a typedef associated with them. The BTF information for this
+ * type is used by the verifier to validate memory accesses made by the
+ * attached information.
+ *
+ * For example the:
+ *
+ *	int bprm_check_security(struct linux_binprm *brpm)
+ *
+ * has the following typedef:
+ *
+ *	typedef int (*lsm_btf_bprm_check_security)(struct linux_binprm *bprm);
+ */
+#define BTF_LSM_PREFIX "lsm_btf_"
+
+static inline int check_attach_btf_id_lsm(struct bpf_verifier_env *env)
+{
+	struct bpf_prog *prog = env->prog;
+	u32 btf_id = prog->aux->attach_btf_id;
+	const struct btf_type *t;
+	const char *tname;
+
+	if (!btf_id) {
+		verbose(env, "LSM programs must provide btf_id\n");
+		return -EINVAL;
+	}
+
+	t = btf_type_by_id(btf_vmlinux, btf_id);
+	if (!t) {
+		verbose(env, "attach_btf_id %u is invalid\n", btf_id);
+		return -EINVAL;
+	}
+
+	tname = btf_name_by_offset(btf_vmlinux, t->name_off);
+	if (!tname) {
+		verbose(env, "attach_btf_id %u doesn't have a name\n", btf_id);
+		return -EINVAL;
+	}
+
+	if (!btf_type_is_typedef(t)) {
+		verbose(env, "attach_btf_id %u is not a typedef\n", btf_id);
+		return -EINVAL;
+	}
+	if (strncmp(BTF_LSM_PREFIX, tname, sizeof(BTF_LSM_PREFIX) - 1)) {
+		verbose(env, "attach_btf_id %u points to wrong type name %s\n",
+			btf_id, tname);
+		return -EINVAL;
+	}
+
+	t = btf_type_by_id(btf_vmlinux, t->type);
+	/* should never happen in valid vmlinux build */
+	if (!btf_type_is_ptr(t))
+		return -EINVAL;
+	t = btf_type_by_id(btf_vmlinux, t->type);
+	/* should never happen in valid vmlinux build */
+	if (!btf_type_is_func_proto(t))
+		return -EINVAL;
+
+	tname += sizeof(BTF_LSM_PREFIX) - 1;
+	prog->aux->attach_func_name = tname;
+	prog->aux->attach_func_proto = t;
+	return 0;
+}
+
+static int check_attach_btf_id_tracing(struct bpf_verifier_env *env)
 {
 	struct bpf_prog *prog = env->prog;
 	struct bpf_prog *tgt_prog = prog->aux->linked_prog;
@@ -9519,9 +9583,6 @@  static int check_attach_btf_id(struct bpf_verifier_env *env)
 	long addr;
 	u64 key;
 
-	if (prog->type != BPF_PROG_TYPE_TRACING)
-		return 0;
-
 	if (!btf_id) {
 		verbose(env, "Tracing programs must provide btf_id\n");
 		return -EINVAL;
@@ -9659,6 +9720,20 @@  static int check_attach_btf_id(struct bpf_verifier_env *env)
 	}
 }
 
+static int check_attach_btf_id(struct bpf_verifier_env *env)
+{
+	struct bpf_prog *prog = env->prog;
+
+	switch (prog->type) {
+	case BPF_PROG_TYPE_TRACING:
+		return check_attach_btf_id_tracing(env);
+	case BPF_PROG_TYPE_LSM:
+		return check_attach_btf_id_lsm(env);
+	default:
+		return 0;
+	}
+}
+
 int bpf_check(struct bpf_prog **prog, union bpf_attr *attr,
 	      union bpf_attr __user *uattr)
 {