From: "Toke Høiland-Jørgensen" <toke@redhat.com>
To: Alexei Starovoitov <ast@kernel.org>
Cc: Daniel Borkmann <daniel@iogearbox.net>,
Martin KaFai Lau <kafai@fb.com>, Song Liu <songliubraving@fb.com>,
Yonghong Song <yhs@fb.com>, Andrii Nakryiko <andriin@fb.com>,
John Fastabend <john.fastabend@gmail.com>,
KP Singh <kpsingh@chromium.org>,
netdev@vger.kernel.org, bpf@vger.kernel.org
Subject: [PATCH bpf-next v2 2/6] bpf: verifier: refactor check_attach_btf_id()
Date: Wed, 15 Jul 2020 15:09:01 +0200 [thread overview]
Message-ID: <159481854144.454654.2971891166197969789.stgit@toke.dk> (raw)
In-Reply-To: <159481853923.454654.12184603524310603480.stgit@toke.dk>
From: Toke Høiland-Jørgensen <toke@redhat.com>
The check_attach_btf_id() function really does three things:
1. It performs a bunch of checks on the program to ensure that the
attachment is valid.
2. It stores a bunch of state about the attachment being requested in
the verifier environment and struct bpf_prog objects.
3. It allocates a trampoline for the attachment.
This patch splits out (1.) and (3.) into separate functions in preparation
for reusing them when the actual attachment is happening (in the
raw_tracepoint_open syscall operation), which will allow tracing programs
to have multiple (compatible) attachments.
No functional change is intended with this patch.
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
include/linux/bpf.h | 9 ++
include/linux/bpf_verifier.h | 9 ++
kernel/bpf/trampoline.c | 22 ++++++
kernel/bpf/verifier.c | 167 ++++++++++++++++++++++++------------------
4 files changed, 134 insertions(+), 73 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 154aab467728..ca3a2a1812c2 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -570,6 +570,9 @@ static __always_inline unsigned int bpf_dispatcher_nop_func(
struct bpf_trampoline *bpf_trampoline_lookup(u64 key);
int bpf_trampoline_link_prog(struct bpf_prog *prog);
int bpf_trampoline_unlink_prog(struct bpf_prog *prog);
+int bpf_trampoline_get(u64 key, void *addr,
+ struct btf_func_model *fmodel,
+ struct bpf_trampoline **trampoline);
void bpf_trampoline_put(struct bpf_trampoline *tr);
#define BPF_DISPATCHER_INIT(_name) { \
.mutex = __MUTEX_INITIALIZER(_name.mutex), \
@@ -626,6 +629,12 @@ static inline int bpf_trampoline_unlink_prog(struct bpf_prog *prog)
{
return -ENOTSUPP;
}
+static inline int bpf_trampoline_get(u64 key, void *addr,
+ struct btf_func_model *fmodel,
+ struct bpf_trampoline **trampoline)
+{
+ return -EOPNOTSUPP;
+}
static inline void bpf_trampoline_put(struct bpf_trampoline *tr) {}
#define DEFINE_BPF_DISPATCHER(name)
#define DECLARE_BPF_DISPATCHER(name)
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index f1ee7468ec28..2934255f936a 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -446,4 +446,13 @@ bpf_prog_offload_remove_insns(struct bpf_verifier_env *env, u32 off, u32 cnt);
int check_ctx_reg(struct bpf_verifier_env *env,
const struct bpf_reg_state *reg, int regno);
+int bpf_check_attach_target(struct bpf_verifier_log *log,
+ const struct bpf_prog *prog,
+ const struct bpf_prog *tgt_prog,
+ u32 btf_id,
+ struct btf_func_model *fmodel,
+ long *tgt_addr,
+ const char **tgt_name,
+ const struct btf_type **tgt_type);
+
#endif /* _LINUX_BPF_VERIFIER_H */
diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
index 9be85aa4ec5f..fadfa330f728 100644
--- a/kernel/bpf/trampoline.c
+++ b/kernel/bpf/trampoline.c
@@ -331,6 +331,28 @@ int bpf_trampoline_unlink_prog(struct bpf_prog *prog)
return err;
}
+int bpf_trampoline_get(u64 key, void *addr,
+ struct btf_func_model *fmodel,
+ struct bpf_trampoline **trampoline)
+{
+ struct bpf_trampoline *tr;
+
+ tr = bpf_trampoline_lookup(key);
+ if (!tr)
+ return -ENOMEM;
+
+ mutex_lock(&tr->mutex);
+ if (tr->func.addr)
+ goto out;
+
+ memcpy(&tr->func.model, fmodel, sizeof(*fmodel));
+ tr->func.addr = addr;
+out:
+ mutex_unlock(&tr->mutex);
+ *trampoline = tr;
+ return 0;
+}
+
void bpf_trampoline_put(struct bpf_trampoline *tr)
{
if (!tr)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 6f35b4924b4c..a1ab7298f53b 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -10731,31 +10731,23 @@ static int check_attach_modify_return(struct bpf_prog *prog, unsigned long addr)
return -EINVAL;
}
-static int check_attach_btf_id(struct bpf_verifier_env *env)
+int bpf_check_attach_target(struct bpf_verifier_log *log,
+ const struct bpf_prog *prog,
+ const struct bpf_prog *tgt_prog,
+ u32 btf_id,
+ struct btf_func_model *fmodel,
+ long *tgt_addr,
+ const char **tgt_name,
+ const struct btf_type **tgt_type)
{
- struct bpf_prog *prog = env->prog;
bool prog_extension = prog->type == BPF_PROG_TYPE_EXT;
- struct bpf_prog *tgt_prog = prog->aux->linked_prog;
- struct bpf_verifier_log *log = &env->log;
- u32 btf_id = prog->aux->attach_btf_id;
const char prefix[] = "btf_trace_";
- struct btf_func_model fmodel;
int ret = 0, subprog = -1, i;
- struct bpf_trampoline *tr;
const struct btf_type *t;
bool conservative = true;
const char *tname;
struct btf *btf;
- long addr;
- u64 key;
-
- if (prog->type == BPF_PROG_TYPE_STRUCT_OPS)
- return check_struct_ops_btf_id(env);
-
- if (prog->type != BPF_PROG_TYPE_TRACING &&
- prog->type != BPF_PROG_TYPE_LSM &&
- !prog_extension)
- return 0;
+ long addr = 0;
if (!btf_id) {
bpf_log(log, "Tracing programs must provide btf_id\n");
@@ -10801,8 +10793,6 @@ static int check_attach_btf_id(struct bpf_verifier_env *env)
"Extension programs should be JITed\n");
return -EINVAL;
}
- env->ops = bpf_verifier_ops[tgt_prog->type];
- prog->expected_attach_type = tgt_prog->expected_attach_type;
}
if (!tgt_prog->jited) {
bpf_log(log, "Can attach to only JITed progs\n");
@@ -10838,13 +10828,11 @@ static int check_attach_btf_id(struct bpf_verifier_env *env)
bpf_log(log, "Cannot extend fentry/fexit\n");
return -EINVAL;
}
- key = ((u64)aux->id) << 32 | btf_id;
} else {
if (prog_extension) {
bpf_log(log, "Cannot replace kernel functions\n");
return -EINVAL;
}
- key = btf_id;
}
switch (prog->expected_attach_type) {
@@ -10874,13 +10862,7 @@ static int check_attach_btf_id(struct bpf_verifier_env *env)
/* should never happen in valid vmlinux build */
return -EINVAL;
- /* remember two read only pointers that are valid for
- * the life time of the kernel
- */
- prog->aux->attach_func_name = tname;
- prog->aux->attach_func_proto = t;
- prog->aux->attach_btf_trace = true;
- return 0;
+ break;
case BPF_TRACE_ITER:
if (!btf_type_is_func(t)) {
bpf_log(log, "attach_btf_id %u is not a function\n",
@@ -10890,12 +10872,10 @@ static int check_attach_btf_id(struct bpf_verifier_env *env)
t = btf_type_by_id(btf, t->type);
if (!btf_type_is_func_proto(t))
return -EINVAL;
- prog->aux->attach_func_name = tname;
- prog->aux->attach_func_proto = t;
- if (!bpf_iter_prog_supported(prog))
- return -EINVAL;
- ret = btf_distill_func_proto(log, btf, t, tname, &fmodel);
- return ret;
+ ret = btf_distill_func_proto(log, btf, t, tname, fmodel);
+ if (ret)
+ return ret;
+ break;
default:
if (!prog_extension)
return -EINVAL;
@@ -10904,13 +10884,6 @@ static int check_attach_btf_id(struct bpf_verifier_env *env)
case BPF_LSM_MAC:
case BPF_TRACE_FENTRY:
case BPF_TRACE_FEXIT:
- prog->aux->attach_func_name = tname;
- if (prog->type == BPF_PROG_TYPE_LSM) {
- ret = bpf_lsm_verify_prog(log, prog);
- if (ret < 0)
- return ret;
- }
-
if (!btf_type_is_func(t)) {
bpf_log(log, "attach_btf_id %u is not a function\n",
btf_id);
@@ -10922,24 +10895,14 @@ static int check_attach_btf_id(struct bpf_verifier_env *env)
t = btf_type_by_id(btf, t->type);
if (!btf_type_is_func_proto(t))
return -EINVAL;
- tr = bpf_trampoline_lookup(key);
- if (!tr)
- return -ENOMEM;
- /* t is either vmlinux type or another program's type */
- prog->aux->attach_func_proto = t;
- mutex_lock(&tr->mutex);
- if (tr->func.addr) {
- prog->aux->trampoline = tr;
- goto out;
- }
- if (tgt_prog && conservative) {
- prog->aux->attach_func_proto = NULL;
+
+ if (tgt_prog && conservative)
t = NULL;
- }
- ret = btf_distill_func_proto(log, btf, t,
- tname, &tr->func.model);
+
+ ret = btf_distill_func_proto(log, btf, t, tname, fmodel);
if (ret < 0)
- goto out;
+ return ret;
+
if (tgt_prog) {
if (subprog == 0)
addr = (long) tgt_prog->bpf_func;
@@ -10951,27 +10914,85 @@ static int check_attach_btf_id(struct bpf_verifier_env *env)
bpf_log(log,
"The address of function %s cannot be found\n",
tname);
- ret = -ENOENT;
- goto out;
+ return -ENOENT;
}
}
+ break;
+ }
- if (prog->expected_attach_type == BPF_MODIFY_RETURN) {
- ret = check_attach_modify_return(prog, addr);
- if (ret)
- bpf_log(log, "%s() is not modifiable\n",
- prog->aux->attach_func_name);
- }
+ *tgt_addr = addr;
+ if (tgt_name)
+ *tgt_name = tname;
+ if (tgt_type)
+ *tgt_type = t;
+ return 0;
+}
- if (ret)
- goto out;
- tr->func.addr = (void *)addr;
- prog->aux->trampoline = tr;
-out:
- mutex_unlock(&tr->mutex);
- if (ret)
- bpf_trampoline_put(tr);
+static int check_attach_btf_id(struct bpf_verifier_env *env)
+{
+ struct bpf_prog *prog = env->prog;
+ struct bpf_prog *tgt_prog = prog->aux->linked_prog;
+ u32 btf_id = prog->aux->attach_btf_id;
+ struct btf_func_model fmodel;
+ const struct btf_type *t;
+ const char *tname;
+ long addr;
+ int ret;
+ u64 key;
+
+ if (prog->type == BPF_PROG_TYPE_STRUCT_OPS)
+ return check_struct_ops_btf_id(env);
+
+ if (prog->type != BPF_PROG_TYPE_TRACING &&
+ prog->type != BPF_PROG_TYPE_LSM &&
+ prog->type != BPF_PROG_TYPE_EXT)
+ return 0;
+
+ ret = bpf_check_attach_target(&env->log, prog, tgt_prog, btf_id,
+ &fmodel, &addr, &tname, &t);
+ if (ret)
return ret;
+
+ if (tgt_prog) {
+ if (prog->type == BPF_PROG_TYPE_EXT) {
+ env->ops = bpf_verifier_ops[tgt_prog->type];
+ prog->expected_attach_type = tgt_prog->expected_attach_type;
+ }
+ key = ((u64)tgt_prog->aux->id) << 32 | btf_id;
+ } else {
+ key = btf_id;
+ }
+
+ prog->aux->attach_func_proto = t;
+ prog->aux->attach_func_name = tname;
+
+ switch (prog->expected_attach_type) {
+ case BPF_TRACE_RAW_TP:
+ /* remember two read only pointers that are valid for
+ * the life time of the kernel
+ */
+ prog->aux->attach_btf_trace = true;
+ return 0;
+ case BPF_TRACE_ITER:
+ if (!bpf_iter_prog_supported(prog))
+ return -EINVAL;
+ return 0;
+ case BPF_MODIFY_RETURN:
+ ret = check_attach_modify_return(prog, addr);
+ if (ret) {
+ verbose(env, "%s() is not modifiable\n",
+ prog->aux->attach_func_name);
+ return ret;
+ }
+ fallthrough;
+ default:
+ if (prog->type == BPF_PROG_TYPE_LSM) {
+ ret = bpf_lsm_verify_prog(&env->log, prog);
+ if (ret < 0)
+ return ret;
+ }
+ return bpf_trampoline_get(key, (void *)addr, &fmodel,
+ &prog->aux->trampoline);
}
}
next prev parent reply other threads:[~2020-07-15 13:09 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-15 13:08 [PATCH bpf-next v2 0/6] bpf: Support multi-attach for freplace programs Toke Høiland-Jørgensen
2020-07-15 13:09 ` [PATCH bpf-next v2 1/6] bpf: change logging calls from verbose() to bpf_log() and use log pointer Toke Høiland-Jørgensen
2020-07-16 10:10 ` Dan Carpenter
2020-07-15 13:09 ` Toke Høiland-Jørgensen [this message]
2020-07-15 13:09 ` [PATCH bpf-next v2 3/6] bpf: support attaching freplace programs to multiple attach points Toke Høiland-Jørgensen
2020-07-15 20:44 ` Alexei Starovoitov
2020-07-16 10:50 ` Toke Høiland-Jørgensen
2020-07-17 2:05 ` Alexei Starovoitov
2020-07-17 10:52 ` Toke Høiland-Jørgensen
2020-07-20 22:57 ` Alexei Starovoitov
2020-07-20 5:02 ` Andrii Nakryiko
2020-07-20 23:34 ` Alexei Starovoitov
2020-07-21 3:48 ` Andrii Nakryiko
2020-07-22 0:29 ` Alexei Starovoitov
2020-07-22 6:02 ` Andrii Nakryiko
2020-07-23 0:32 ` Alexei Starovoitov
2020-07-23 0:56 ` Andrii Nakryiko
2020-07-16 10:14 ` Dan Carpenter
2020-07-15 13:09 ` [PATCH bpf-next v2 4/6] tools: add new members to bpf_attr.raw_tracepoint in bpf.h Toke Høiland-Jørgensen
2020-07-15 13:09 ` [PATCH bpf-next v2 5/6] libbpf: add support for supplying target to bpf_raw_tracepoint_open() Toke Høiland-Jørgensen
2020-07-15 13:09 ` [PATCH bpf-next v2 6/6] selftests: add test for multiple attachments of freplace program Toke Høiland-Jørgensen
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=159481854144.454654.2971891166197969789.stgit@toke.dk \
--to=toke@redhat.com \
--cc=andriin@fb.com \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=john.fastabend@gmail.com \
--cc=kafai@fb.com \
--cc=kpsingh@chromium.org \
--cc=netdev@vger.kernel.org \
--cc=songliubraving@fb.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).