[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <b301f6b8-afed-6d55-42d3-6587b75fadb9@fb.com>
Date: Sun, 10 Jan 2021 20:13:13 -0800
From: Yonghong Song <yhs@...com>
To: Andrii Nakryiko <andrii@...nel.org>, <bpf@...r.kernel.org>,
<netdev@...r.kernel.org>, <ast@...com>, <daniel@...earbox.net>
CC: <kernel-team@...com>, Hao Luo <haoluo@...gle.com>
Subject: Re: [PATCH v2 bpf-next 5/7] bpf: support BPF ksym variables in kernel
modules
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@...nel.org>
Ack with a minor nit below.
Acked-by: Yonghong Song <yhs@...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;
> }
>
[...]
Powered by blists - more mailing lists