2019-12-20 15:43:19

by KP Singh

[permalink] [raw]
Subject: [PATCH bpf-next v1 10/13] bpf: lsm: Handle attachment of the same program

From: KP Singh <[email protected]>

Allow userspace to attach a newer version of a program without having
duplicates of the same program.

If BPF_F_ALLOW_OVERRIDE is passed, the attachment logic compares the
name of the new program to the names of existing attached programs. The
names are only compared till a "__" (or '\0', if there is no "__"). If
a successful match is found, the existing program is replaced with the
newer attachment.

./loader Attaches "env_dumper__v1" followed by "env_dumper__v2"
to the bprm_check_security hook..

./loader
./loader

Before:

cat /sys/kernel/security/bpf/process_execution
env_dumper__v1
env_dumper__v2

After:

cat /sys/kernel/security/bpf/process_execution
env_dumper__v2

Signed-off-by: KP Singh <[email protected]>
---
security/bpf/ops.c | 57 +++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 56 insertions(+), 1 deletion(-)

diff --git a/security/bpf/ops.c b/security/bpf/ops.c
index e9aae2ce718c..481e6ee75f27 100644
--- a/security/bpf/ops.c
+++ b/security/bpf/ops.c
@@ -64,11 +64,52 @@ static struct bpf_lsm_hook *get_hook_from_fd(int fd)
return ERR_PTR(ret);
}

+/*
+ * match_prog_name matches the name of the program till "__"
+ * or the end of the string is encountered. This allows
+ * the matched program to be replaced by a newer version.
+ *
+ * For example:
+ *
+ * env_dumper__v1 is matched with env_dumper__v2
+ *
+ */
+static bool match_prog_name(const char *a, const char *b)
+{
+ size_t m, n;
+ char *end;
+
+ end = strstr(a, "__");
+ n = end ? end - a : strlen(a);
+
+ end = strstr(b, "__");
+ m = end ? end - b : strlen(b);
+
+ if (m != n)
+ return false;
+
+ return strncmp(a, b, n) == 0;
+}
+
+static struct bpf_prog *find_attached_prog(struct bpf_prog_array *array,
+ struct bpf_prog *prog)
+{
+ struct bpf_prog_array_item *item = array->items;
+
+ for (; item->prog; item++) {
+ if (match_prog_name(item->prog->aux->name, prog->aux->name))
+ return item->prog;
+ }
+
+ return NULL;
+}
+
int bpf_lsm_attach(const union bpf_attr *attr, struct bpf_prog *prog)
{
struct bpf_prog_array *old_array;
struct bpf_prog_array *new_array;
struct bpf_lsm_hook *h;
+ struct bpf_prog *old_prog = NULL;
int ret = 0;

h = get_hook_from_fd(attr->target_fd);
@@ -78,13 +119,27 @@ int bpf_lsm_attach(const union bpf_attr *attr, struct bpf_prog *prog)
mutex_lock(&h->mutex);
old_array = rcu_dereference_protected(h->progs,
lockdep_is_held(&h->mutex));
+ /*
+ * Check if a matching program already exists and replace
+ * the existing program if BPF_F_ALLOW_OVERRIDE is specified in
+ * the attach flags.
+ */
+ if (old_array) {
+ old_prog = find_attached_prog(old_array, prog);
+ if (old_prog && !(attr->attach_flags & BPF_F_ALLOW_OVERRIDE)) {
+ ret = -EEXIST;
+ goto unlock;
+ }
+ }

- ret = bpf_prog_array_copy(old_array, NULL, prog, &new_array);
+ ret = bpf_prog_array_copy(old_array, old_prog, prog, &new_array);
if (ret < 0)
goto unlock;

rcu_assign_pointer(h->progs, new_array);
bpf_prog_array_free(old_array);
+ if (old_prog)
+ bpf_prog_put(old_prog);

unlock:
mutex_unlock(&h->mutex);
--
2.20.1


2019-12-24 06:39:42

by Andrii Nakryiko

[permalink] [raw]
Subject: Re: [PATCH bpf-next v1 10/13] bpf: lsm: Handle attachment of the same program

On Fri, Dec 20, 2019 at 7:42 AM KP Singh <[email protected]> wrote:
>
> From: KP Singh <[email protected]>
>
> Allow userspace to attach a newer version of a program without having
> duplicates of the same program.
>
> If BPF_F_ALLOW_OVERRIDE is passed, the attachment logic compares the
> name of the new program to the names of existing attached programs. The
> names are only compared till a "__" (or '\0', if there is no "__"). If
> a successful match is found, the existing program is replaced with the
> newer attachment.
>
> ./loader Attaches "env_dumper__v1" followed by "env_dumper__v2"
> to the bprm_check_security hook..
>
> ./loader
> ./loader
>
> Before:
>
> cat /sys/kernel/security/bpf/process_execution
> env_dumper__v1
> env_dumper__v2
>
> After:
>
> cat /sys/kernel/security/bpf/process_execution
> env_dumper__v2
>
> Signed-off-by: KP Singh <[email protected]>
> ---


Andrey Ignatov just posted patch set few days ago solving similar
problem for cgroup BPF programs. His approach was to actually also
specify FD of BPF program to be replaced. This seems like a more
reliable way than doing this based on name only. Please take a look at
that patch and see if same approach can work for your use case.

> security/bpf/ops.c | 57 +++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 56 insertions(+), 1 deletion(-)
>

[...]

2020-01-08 20:00:17

by James Morris

[permalink] [raw]
Subject: Re: [PATCH bpf-next v1 10/13] bpf: lsm: Handle attachment of the same program

On Fri, 20 Dec 2019, KP Singh wrote:

> From: KP Singh <[email protected]>
>
> Allow userspace to attach a newer version of a program without having
> duplicates of the same program.
>
> If BPF_F_ALLOW_OVERRIDE is passed, the attachment logic compares the
> name of the new program to the names of existing attached programs. The
> names are only compared till a "__" (or '\0', if there is no "__"). If
> a successful match is found, the existing program is replaced with the
> newer attachment.
>
> ./loader Attaches "env_dumper__v1" followed by "env_dumper__v2"
> to the bprm_check_security hook..
>
> ./loader
> ./loader
>
> Before:
>
> cat /sys/kernel/security/bpf/process_execution
> env_dumper__v1
> env_dumper__v2
>
> After:
>
> cat /sys/kernel/security/bpf/process_execution
> env_dumper__v2
>
> Signed-off-by: KP Singh <[email protected]>


Reviewed-by: James Morris <[email protected]>


--
James Morris
<[email protected]>