With bpf_get_probe_write_proto() no longer printing a message, we can
avoid it being a special case with its own permission check.

Refactor bpf_tracing_func_proto() similar to bpf_base_func_proto() to
have a section conditional on bpf_token_capable(CAP_SYS_ADMIN), where
the proto for bpf_probe_write_user() is returned. Finally, remove the
unnecessary bpf_get_probe_write_proto().

This simplifies the code, and adding additional CAP_SYS_ADMIN-only
helpers in future avoids duplicating the same CAP_SYS_ADMIN check.

Suggested-by: Andrii Nakryiko <and...@kernel.org>
Signed-off-by: Marco Elver <el...@google.com>
Acked-by: Jiri Olsa <jo...@kernel.org>
---
v4:
* Call bpf_base_func_proto() before bpf_token_capable() (no protos after
  should override bpf_base_func_proto() protos), so we can avoid
  indenting the switch-block after bpf_token_capable() (suggested by Alexei).

v3:
* Fix where bpf_base_func_proto() is called - it needs to be last,
  because we may override protos (as is e.g. done for
  BPF_FUNC_get_smp_processor_id).

v2:
* New patch.
---
 kernel/trace/bpf_trace.c | 30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 0ab56af2e298..b07d8067aa6e 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -357,14 +357,6 @@ static const struct bpf_func_proto 
bpf_probe_write_user_proto = {
        .arg3_type      = ARG_CONST_SIZE,
 };
 
-static const struct bpf_func_proto *bpf_get_probe_write_proto(void)
-{
-       if (!capable(CAP_SYS_ADMIN))
-               return NULL;
-
-       return &bpf_probe_write_user_proto;
-}
-
 #define MAX_TRACE_PRINTK_VARARGS       3
 #define BPF_TRACE_PRINTK_SIZE          1024
 
@@ -1417,6 +1409,8 @@ late_initcall(bpf_key_sig_kfuncs_init);
 static const struct bpf_func_proto *
 bpf_tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 {
+       const struct bpf_func_proto *func_proto;
+
        switch (func_id) {
        case BPF_FUNC_map_lookup_elem:
                return &bpf_map_lookup_elem_proto;
@@ -1458,9 +1452,6 @@ bpf_tracing_func_proto(enum bpf_func_id func_id, const 
struct bpf_prog *prog)
                return &bpf_perf_event_read_proto;
        case BPF_FUNC_get_prandom_u32:
                return &bpf_get_prandom_u32_proto;
-       case BPF_FUNC_probe_write_user:
-               return security_locked_down(LOCKDOWN_BPF_WRITE_USER) < 0 ?
-                      NULL : bpf_get_probe_write_proto();
        case BPF_FUNC_probe_read_user:
                return &bpf_probe_read_user_proto;
        case BPF_FUNC_probe_read_kernel:
@@ -1539,7 +1530,22 @@ bpf_tracing_func_proto(enum bpf_func_id func_id, const 
struct bpf_prog *prog)
        case BPF_FUNC_trace_vprintk:
                return bpf_get_trace_vprintk_proto();
        default:
-               return bpf_base_func_proto(func_id, prog);
+               break;
+       }
+
+       func_proto = bpf_base_func_proto(func_id, prog);
+       if (func_proto)
+               return func_proto;
+
+       if (!bpf_token_capable(prog->aux->token, CAP_SYS_ADMIN))
+               return NULL;
+
+       switch (func_id) {
+       case BPF_FUNC_probe_write_user:
+               return security_locked_down(LOCKDOWN_BPF_WRITE_USER) < 0 ?
+                      NULL : &bpf_probe_write_user_proto;
+       default:
+               return NULL;
        }
 }
 
-- 
2.47.0.338.g60cca15819-goog


Reply via email to