From: Yonghong Song <yhs@fb.com>
To: Andrii Nakryiko <andrii@kernel.org>, <bpf@vger.kernel.org>,
<netdev@vger.kernel.org>, <ast@fb.com>, <daniel@iogearbox.net>
Cc: <kernel-team@fb.com>, Hao Luo <haoluo@google.com>
Subject: Re: [PATCH v2 bpf-next 5/7] bpf: support BPF ksym variables in kernel modules
Date: Sun, 10 Jan 2021 20:13:13 -0800 [thread overview]
Message-ID: <b301f6b8-afed-6d55-42d3-6587b75fadb9@fb.com> (raw)
In-Reply-To: <20210108220930.482456-6-andrii@kernel.org>
On 1/8/21 2:09 PM, Andrii Nakryiko wrote:
> Add support for directly accessing kernel module variables from BPF programs
> using special ldimm64 instructions. This functionality builds upon vmlinux
> ksym support, but extends ldimm64 with src_reg=BPF_PSEUDO_BTF_ID to allow
> specifying kernel module BTF's FD in insn[1].imm field.
>
> During BPF program load time, verifier will resolve FD to BTF object and will
> take reference on BTF object itself and, for module BTFs, corresponding module
> as well, to make sure it won't be unloaded from under running BPF program. The
> mechanism used is similar to how bpf_prog keeps track of used bpf_maps.
>
> One interesting change is also in how per-CPU variable is determined. The
> logic is to find .data..percpu data section in provided BTF, but both vmlinux
> and module each have their own .data..percpu entries in BTF. So for module's
> case, the search for DATASEC record needs to look at only module's added BTF
> types. This is implemented with custom search function.
>
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Ack with a minor nit below.
Acked-by: Yonghong Song <yhs@fb.com>
> ---
> include/linux/bpf.h | 10 +++
> include/linux/bpf_verifier.h | 3 +
> include/linux/btf.h | 3 +
> kernel/bpf/btf.c | 31 +++++++-
> kernel/bpf/core.c | 23 ++++++
> kernel/bpf/verifier.c | 149 ++++++++++++++++++++++++++++-------
> 6 files changed, 189 insertions(+), 30 deletions(-)
>
[...]
> /* replace pseudo btf_id with kernel symbol address */
> static int check_pseudo_btf_id(struct bpf_verifier_env *env,
> struct bpf_insn *insn,
> @@ -9710,48 +9735,57 @@ static int check_pseudo_btf_id(struct bpf_verifier_env *env,
> {
> const struct btf_var_secinfo *vsi;
> const struct btf_type *datasec;
> + struct btf_mod_pair *btf_mod;
> const struct btf_type *t;
> const char *sym_name;
> bool percpu = false;
> u32 type, id = insn->imm;
> + struct btf *btf;
> s32 datasec_id;
> u64 addr;
> - int i;
> + int i, btf_fd, err;
>
> - if (!btf_vmlinux) {
> - verbose(env, "kernel is missing BTF, make sure CONFIG_DEBUG_INFO_BTF=y is specified in Kconfig.\n");
> - return -EINVAL;
> - }
> -
> - if (insn[1].imm != 0) {
> - verbose(env, "reserved field (insn[1].imm) is used in pseudo_btf_id ldimm64 insn.\n");
> - return -EINVAL;
> + btf_fd = insn[1].imm;
> + if (btf_fd) {
> + btf = btf_get_by_fd(btf_fd);
> + if (IS_ERR(btf)) {
> + verbose(env, "invalid module BTF object FD specified.\n");
> + return -EINVAL;
> + }
> + } else {
> + if (!btf_vmlinux) {
> + verbose(env, "kernel is missing BTF, make sure CONFIG_DEBUG_INFO_BTF=y is specified in Kconfig.\n");
> + return -EINVAL;
> + }
> + btf = btf_vmlinux;
> + btf_get(btf);
> }
>
> - t = btf_type_by_id(btf_vmlinux, id);
> + t = btf_type_by_id(btf, id);
> if (!t) {
> verbose(env, "ldimm64 insn specifies invalid btf_id %d.\n", id);
> - return -ENOENT;
> + err = -ENOENT;
> + goto err_put;
> }
>
> if (!btf_type_is_var(t)) {
> - verbose(env, "pseudo btf_id %d in ldimm64 isn't KIND_VAR.\n",
> - id);
> - return -EINVAL;
> + verbose(env, "pseudo btf_id %d in ldimm64 isn't KIND_VAR.\n", id);
> + err = -EINVAL;
> + goto err_put;
> }
>
> - sym_name = btf_name_by_offset(btf_vmlinux, t->name_off);
> + sym_name = btf_name_by_offset(btf, t->name_off);
> addr = kallsyms_lookup_name(sym_name);
> if (!addr) {
> verbose(env, "ldimm64 failed to find the address for kernel symbol '%s'.\n",
> sym_name);
> - return -ENOENT;
> + err = -ENOENT;
> + goto err_put;
> }
>
> - datasec_id = btf_find_by_name_kind(btf_vmlinux, ".data..percpu",
> - BTF_KIND_DATASEC);
> + datasec_id = find_btf_percpu_datasec(btf);
> if (datasec_id > 0) {
> - datasec = btf_type_by_id(btf_vmlinux, datasec_id);
> + datasec = btf_type_by_id(btf, datasec_id);
> for_each_vsi(i, datasec, vsi) {
> if (vsi->type == id) {
> percpu = true;
> @@ -9764,10 +9798,10 @@ static int check_pseudo_btf_id(struct bpf_verifier_env *env,
> insn[1].imm = addr >> 32;
>
> type = t->type;
> - t = btf_type_skip_modifiers(btf_vmlinux, type, NULL);
> + t = btf_type_skip_modifiers(btf, type, NULL);
> if (percpu) {
> aux->btf_var.reg_type = PTR_TO_PERCPU_BTF_ID;
> - aux->btf_var.btf = btf_vmlinux;
> + aux->btf_var.btf = btf;
> aux->btf_var.btf_id = type;
> } else if (!btf_type_is_struct(t)) {
> const struct btf_type *ret;
> @@ -9775,21 +9809,54 @@ static int check_pseudo_btf_id(struct bpf_verifier_env *env,
> u32 tsize;
>
> /* resolve the type size of ksym. */
> - ret = btf_resolve_size(btf_vmlinux, t, &tsize);
> + ret = btf_resolve_size(btf, t, &tsize);
> if (IS_ERR(ret)) {
> - tname = btf_name_by_offset(btf_vmlinux, t->name_off);
> + tname = btf_name_by_offset(btf, t->name_off);
> verbose(env, "ldimm64 unable to resolve the size of type '%s': %ld\n",
> tname, PTR_ERR(ret));
> - return -EINVAL;
> + err = -EINVAL;
> + goto err_put;
> }
> aux->btf_var.reg_type = PTR_TO_MEM;
> aux->btf_var.mem_size = tsize;
> } else {
> aux->btf_var.reg_type = PTR_TO_BTF_ID;
> - aux->btf_var.btf = btf_vmlinux;
> + aux->btf_var.btf = btf;
> aux->btf_var.btf_id = type;
> }
> +
> + /* check whether we recorded this BTF (and maybe module) already */
> + for (i = 0; i < env->used_btf_cnt; i++) {
> + if (env->used_btfs[i].btf == btf) {
> + btf_put(btf);
> + return 0;
An alternative way is to change the above code as
err = 0;
goto err_put;
> + }
> + }
> +
> + if (env->used_btf_cnt >= MAX_USED_BTFS) {
> + err = -E2BIG;
> + goto err_put;
> + }
> +
> + btf_mod = &env->used_btfs[env->used_btf_cnt];
> + btf_mod->btf = btf;
> + btf_mod->module = NULL;
> +
> + /* if we reference variables from kernel module, bump its refcount */
> + if (btf_is_module(btf)) {
> + btf_mod->module = btf_try_get_module(btf);
> + if (!btf_mod->module) {
> + err = -ENXIO;
> + goto err_put;
> + }
> + }
> +
> + env->used_btf_cnt++;
> +
> return 0;
> +err_put:
> + btf_put(btf);
> + return err;
> }
>
[...]
next prev parent reply other threads:[~2021-01-11 4:14 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-08 22:09 [PATCH v2 bpf-next 0/7] Support kernel module ksym variables Andrii Nakryiko
2021-01-08 22:09 ` [PATCH v2 bpf-next 1/7] bpf: add bpf_patch_call_args prototype to include/linux/bpf.h Andrii Nakryiko
2021-01-11 4:02 ` Yonghong Song
2021-01-08 22:09 ` [PATCH v2 bpf-next 2/7] bpf: avoid warning when re-casting __bpf_call_base into __bpf_call_base_args Andrii Nakryiko
2021-01-11 4:03 ` Yonghong Song
2021-01-08 22:09 ` [PATCH v2 bpf-next 3/7] bpf: declare __bpf_free_used_maps() unconditionally Andrii Nakryiko
2021-01-11 4:03 ` Yonghong Song
2021-01-08 22:09 ` [PATCH v2 bpf-next 4/7] selftests/bpf: sync RCU before unloading bpf_testmod Andrii Nakryiko
2021-01-11 4:05 ` Yonghong Song
2021-01-11 18:59 ` Hao Luo
2021-01-08 22:09 ` [PATCH v2 bpf-next 5/7] bpf: support BPF ksym variables in kernel modules Andrii Nakryiko
2021-01-11 4:13 ` Yonghong Song [this message]
2021-01-11 21:29 ` Andrii Nakryiko
2021-01-12 1:25 ` Yonghong Song
2021-01-11 18:59 ` Hao Luo
2021-01-11 21:31 ` Andrii Nakryiko
2021-01-08 22:09 ` [PATCH v2 bpf-next 6/7] libbpf: support kernel module ksym externs Andrii Nakryiko
2021-01-11 4:15 ` Yonghong Song
2021-01-11 21:37 ` Andrii Nakryiko
2021-01-12 1:34 ` Yonghong Song
2021-01-12 6:45 ` Andrii Nakryiko
2021-01-11 19:00 ` Hao Luo
2021-01-11 21:39 ` Andrii Nakryiko
2021-01-08 22:09 ` [PATCH v2 bpf-next 7/7] selftests/bpf: test " Andrii Nakryiko
2021-01-11 4:18 ` Yonghong Song
2021-01-11 21:40 ` Andrii Nakryiko
2021-01-11 19:00 ` Hao Luo
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=b301f6b8-afed-6d55-42d3-6587b75fadb9@fb.com \
--to=yhs@fb.com \
--cc=andrii@kernel.org \
--cc=ast@fb.com \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=haoluo@google.com \
--cc=kernel-team@fb.com \
--cc=netdev@vger.kernel.org \
/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.