From: Jiri Olsa <jolsa@kernel.org>
To: Alexei Starovoitov <ast@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Andrii Nakryiko <andrii@kernel.org>
Cc: Hao Sun <sunhao.th@gmail.com>,
bpf@vger.kernel.org, Martin KaFai Lau <kafai@fb.com>,
Song Liu <songliubraving@fb.com>, Yonghong Song <yhs@fb.com>,
John Fastabend <john.fastabend@gmail.com>,
KP Singh <kpsingh@chromium.org>,
Stanislav Fomichev <sdf@google.com>, Hao Luo <haoluo@google.com>
Subject: [PATCH bpf-next] bpf: Restrict attachment of bpf program to some tracepoints
Date: Mon, 21 Nov 2022 22:31:23 +0100 [thread overview]
Message-ID: <20221121213123.1373229-1-jolsa@kernel.org> (raw)
We hit following issues [1] [2] when we attach bpf program that calls
bpf_trace_printk helper to the contention_begin tracepoint.
As described in [3] with multiple bpf programs that call bpf_trace_printk
helper attached to the contention_begin might result in exhaustion of
printk buffer or cause a deadlock [2].
There's also another possible deadlock when multiple bpf programs attach
to bpf_trace_printk tracepoint and call one of the printk bpf helpers.
This change denies the attachment of bpf program to contention_begin
and bpf_trace_printk tracepoints if the bpf program calls one of the
printk bpf helpers.
Adding also verifier check for tb_btf programs, so this can be cought
in program loading time with error message like:
Can't attach program with bpf_trace_printk#6 helper to contention_begin tracepoint.
[1] https://lore.kernel.org/bpf/CACkBjsakT_yWxnSWr4r-0TpPvbKm9-OBmVUhJb7hV3hY8fdCkw@mail.gmail.com/
[2] https://lore.kernel.org/bpf/CACkBjsaCsTovQHFfkqJKto6S4Z8d02ud1D7MPESrHa1cVNNTrw@mail.gmail.com/
[3] https://lore.kernel.org/bpf/Y2j6ivTwFmA0FtvY@krava/
Reported-by: Hao Sun <sunhao.th@gmail.com>
Suggested-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
include/linux/bpf.h | 1 +
include/linux/bpf_verifier.h | 2 ++
kernel/bpf/syscall.c | 3 +++
kernel/bpf/verifier.c | 46 ++++++++++++++++++++++++++++++++++++
4 files changed, 52 insertions(+)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index c9eafa67f2a2..3ccabede0f50 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1319,6 +1319,7 @@ struct bpf_prog {
enforce_expected_attach_type:1, /* Enforce expected_attach_type checking at attach time */
call_get_stack:1, /* Do we call bpf_get_stack() or bpf_get_stackid() */
call_get_func_ip:1, /* Do we call get_func_ip() */
+ call_printk:1, /* Do we call trace_printk/trace_vprintk */
tstamp_type_access:1; /* Accessed __sk_buff->tstamp_type */
enum bpf_prog_type type; /* Type of BPF program */
enum bpf_attach_type expected_attach_type; /* For some prog types */
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index 545152ac136c..7118c2fda59d 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -618,6 +618,8 @@ bool is_dynptr_type_expected(struct bpf_verifier_env *env,
struct bpf_reg_state *reg,
enum bpf_arg_type arg_type);
+int bpf_check_tp_printk_denylist(const char *name, struct bpf_prog *prog);
+
/* this lives here instead of in bpf.h because it needs to dereference tgt_prog */
static inline u64 bpf_trampoline_compute_key(const struct bpf_prog *tgt_prog,
struct btf *btf, u32 btf_id)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 35972afb6850..9a69bda7d62b 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -3329,6 +3329,9 @@ static int bpf_raw_tp_link_attach(struct bpf_prog *prog,
return -EINVAL;
}
+ if (bpf_check_tp_printk_denylist(tp_name, prog))
+ return -EACCES;
+
btp = bpf_get_raw_tracepoint(tp_name);
if (!btp)
return -ENOENT;
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index f07bec227fef..b662bc851e1c 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -7472,6 +7472,47 @@ static void update_loop_inline_state(struct bpf_verifier_env *env, u32 subprogno
state->callback_subprogno == subprogno);
}
+int bpf_check_tp_printk_denylist(const char *name, struct bpf_prog *prog)
+{
+ static const char * const denylist[] = {
+ "contention_begin",
+ "bpf_trace_printk",
+ };
+ int i;
+
+ /* Do not allow attachment to denylist[] tracepoints,
+ * if the program calls some of the printk helpers,
+ * because there's possibility of deadlock.
+ */
+ if (!prog->call_printk)
+ return 0;
+
+ for (i = 0; i < ARRAY_SIZE(denylist); i++) {
+ if (!strcmp(denylist[i], name))
+ return 1;
+ }
+ return 0;
+}
+
+static int check_tp_printk_denylist(struct bpf_verifier_env *env, int func_id)
+{
+ struct bpf_prog *prog = env->prog;
+
+ if (prog->type != BPF_PROG_TYPE_TRACING ||
+ prog->expected_attach_type != BPF_TRACE_RAW_TP)
+ return 0;
+
+ if (WARN_ON_ONCE(!prog->aux->attach_func_name))
+ return -EINVAL;
+
+ if (!bpf_check_tp_printk_denylist(prog->aux->attach_func_name, prog))
+ return 0;
+
+ verbose(env, "Can't attach program with %s#%d helper to %s tracepoint.\n",
+ func_id_name(func_id), func_id, prog->aux->attach_func_name);
+ return -EACCES;
+}
+
static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
int *insn_idx_p)
{
@@ -7675,6 +7716,11 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
err = __check_func_call(env, insn, insn_idx_p, meta.subprogno,
set_user_ringbuf_callback_state);
break;
+ case BPF_FUNC_trace_printk:
+ case BPF_FUNC_trace_vprintk:
+ env->prog->call_printk = 1;
+ err = check_tp_printk_denylist(env, func_id);
+ break;
}
if (err)
--
2.38.1
next reply other threads:[~2022-11-21 21:31 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-21 21:31 Jiri Olsa [this message]
2022-11-24 0:41 ` [PATCH bpf-next] bpf: Restrict attachment of bpf program to some tracepoints Daniel Borkmann
2022-11-24 9:42 ` Jiri Olsa
2022-11-24 17:17 ` Alexei Starovoitov
2022-11-25 9:35 ` Jiri Olsa
2022-11-30 23:29 ` Andrii Nakryiko
2022-12-03 17:58 ` Namhyung Kim
2022-12-05 12:28 ` Jiri Olsa
2022-12-06 4:00 ` Namhyung Kim
2022-12-06 8:14 ` Jiri Olsa
2022-12-06 18:20 ` Namhyung Kim
2022-12-06 20:09 ` Alexei Starovoitov
2022-12-07 2:14 ` Namhyung Kim
2022-12-07 5:23 ` Hao Sun
2022-12-07 22:58 ` Namhyung Kim
2022-12-07 8:18 ` Jiri Olsa
2022-12-07 19:08 ` Namhyung Kim
2022-12-08 6:15 ` Namhyung Kim
2022-12-08 12:04 ` Jiri Olsa
2022-12-04 21:44 ` Jiri Olsa
2022-12-07 13:39 ` Jiri Olsa
2022-12-07 19:10 ` Alexei Starovoitov
2022-12-08 2:47 ` Hao Sun
2022-12-03 17:42 ` Namhyung Kim
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20221121213123.1373229-1-jolsa@kernel.org \
--to=jolsa@kernel.org \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=haoluo@google.com \
--cc=john.fastabend@gmail.com \
--cc=kafai@fb.com \
--cc=kpsingh@chromium.org \
--cc=sdf@google.com \
--cc=songliubraving@fb.com \
--cc=sunhao.th@gmail.com \
--cc=yhs@fb.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.