bpf: Attachment verification for BPF_MODIFY_RETURN

- Allow BPF_MODIFY_RETURN attachment only to functions that are:

    * Whitelisted for error injection by checking
      within_error_injection_list. Similar discussions happened for the
      bpf_override_return helper.

    * security hooks, this is expected to be cleaned up with the LSM
      changes after the KRSI patches introduce the LSM_HOOK macro:

        https://lore.kernel.org/bpf/20200220175250.10795-1-kpsingh@chromium.org/

- The attachment is currently limited to functions that return an int.
  This can be extended later other types (e.g. PTR).

Signed-off-by: KP Singh <kpsingh@google.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Andrii Nakryiko <andriin@fb.com>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
Link: https://lore.kernel.org/bpf/20200304191853.1529-5-kpsingh@chromium.org

diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 30841fb8b3c0ff12ee15777dad9fcdc241fd67b4..50080add2ab9fed28e3a007dab5d87cfcb3b25ba 100644 (file)
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -3710,14 +3710,26 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
                nr_args--;
        }
 
-       if ((prog->expected_attach_type == BPF_TRACE_FEXIT ||
-            prog->expected_attach_type == BPF_MODIFY_RETURN) &&
-           arg == nr_args) {
-               if (!t)
-                       /* Default prog with 5 args. 6th arg is retval. */
-                       return true;
-               /* function return type */
-               t = btf_type_by_id(btf, t->type);
+       if (arg == nr_args) {
+               if (prog->expected_attach_type == BPF_TRACE_FEXIT) {
+                       if (!t)
+                               return true;
+                       t = btf_type_by_id(btf, t->type);
+               } else if (prog->expected_attach_type == BPF_MODIFY_RETURN) {
+                       /* For now the BPF_MODIFY_RETURN can only be attached to
+                        * functions that return an int.
+                        */
+                       if (!t)
+                               return false;
+
+                       t = btf_type_skip_modifiers(btf, t->type, NULL);
+                       if (!btf_type_is_int(t)) {
+                               bpf_log(log,
+                                       "ret type %s not allowed for fmod_ret\n",
+                                       btf_kind_str[BTF_INFO_KIND(t->info)]);
+                               return false;
+                       }
+               }
        } else if (arg >= nr_args) {
                bpf_log(log, "func '%s' doesn't have %d-th argument\n",
                        tname, arg + 1);

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 2460c8e6b5be47f8522235f768e87c331bf17310..ae32517d4ccda8eeae01fa03f504704bbd080e05 100644 (file)
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -19,6 +19,7 @@
 #include <linux/sort.h>
 #include <linux/perf_event.h>
 #include <linux/ctype.h>
+#include <linux/error-injection.h>
 
 #include "disasm.h"
 
@@ -9800,6 +9801,33 @@ static int check_struct_ops_btf_id(struct bpf_verifier_env *env)
 
        return 0;
 }
+#define SECURITY_PREFIX "security_"
+
+static int check_attach_modify_return(struct bpf_verifier_env *env)
+{
+       struct bpf_prog *prog = env->prog;
+       unsigned long addr = (unsigned long) prog->aux->trampoline->func.addr;
+
+       if (within_error_injection_list(addr))
+               return 0;
+
+       /* This is expected to be cleaned up in the future with the KRSI effort
+        * introducing the LSM_HOOK macro for cleaning up lsm_hooks.h.
+        */
+       if (!strncmp(SECURITY_PREFIX, prog->aux->attach_func_name,
+                    sizeof(SECURITY_PREFIX) - 1)) {
+
+               if (!capable(CAP_MAC_ADMIN))
+                       return -EPERM;
+
+               return 0;
+       }
+
+       verbose(env, "fmod_ret attach_btf_id %u (%s) is not modifiable\n",
+               prog->aux->attach_btf_id, prog->aux->attach_func_name);
+
+       return -EINVAL;
+}
 
 static int check_attach_btf_id(struct bpf_verifier_env *env)
 {
@@ -10000,6 +10028,9 @@ static int check_attach_btf_id(struct bpf_verifier_env *env)
                }
                tr->func.addr = (void *)addr;
                prog->aux->trampoline = tr;
+
+               if (prog->expected_attach_type == BPF_MODIFY_RETURN)
+                       ret = check_attach_modify_return(env);
 out:
                mutex_unlock(&tr->mutex);
                if (ret)