BPF indirect jumps

[kernel-patches-daemon-bpf]

Pull request for series with
subject: BPF indirect jumps
version: 10
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=1018668

[kernel-patches-daemon-bpf]

Upstream branch: ab01bfa
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1018668
version: 10

[kernel-patches-review-bot]

commit b44690c09995d34f7665c7d687b359d41a6ab79f
Author: Anton Protopopov <[email protected]>

libbpf: support llvm-generated indirect jumps

This commit adds support for LLVM-generated indirect jumps in BPF v4
instruction set. It handles jump table metadata from the .jumptables
ELF section and creates BPF_MAP_TYPE_INSN_ARRAY maps for them.

> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index fbe74686c..ed14090a9 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c

[ ... ]

> @@ -6144,6 +6192,157 @@ static void poison_kfunc_call(struct bpf_program *prog, int relo_idx,
>  	insn->imm = POISON_CALL_KFUNC_BASE + ext_idx;
>  }
>
> +static int find_jt_map(struct bpf_object *obj, struct bpf_program *prog, int sym_off)
> +{

[ ... ]

> +static int create_jt_map(struct bpf_object *obj, struct bpf_program *prog, struct reloc_desc *relo)
> +{
> +	const __u32 jt_entry_size = 8;
> +	int sym_off = relo->sym_off;
> +	int jt_size = relo->sym_size;
> +	__u32 max_entries = jt_size / jt_entry_size;
> +	__u32 value_size = sizeof(struct bpf_insn_array_value);
> +	struct bpf_insn_array_value val = {};
> +	int subprog_idx;
> +	int map_fd, err;
> +	__u64 insn_off;
> +	__u64 *jt;
> +	__u32 i;
> +
> +	map_fd = find_jt_map(obj, prog, sym_off);
> +	if (map_fd >= 0)
> +		return map_fd;
> +
> +	if (sym_off % jt_entry_size) {
> +		pr_warn("map '.jumptables': jumptable start %d should be multiple of %u\n",
> +			sym_off, jt_entry_size);
> +		return -EINVAL;
> +	}
> +
> +	if (jt_size % jt_entry_size) {
> +		pr_warn("map '.jumptables': jumptable size %d should be multiple of %u\n",
> +			jt_size, jt_entry_size);
> +		return -EINVAL;
> +	}
> +
> +	map_fd = bpf_map_create(BPF_MAP_TYPE_INSN_ARRAY, ".jumptables",
> +				4, value_size, max_entries, NULL);
> +	if (map_fd < 0)
> +		return map_fd;
> +
> +	if (!obj->jumptables_data) {
> +		pr_warn("map '.jumptables': ELF file is missing jump table data\n");
> +		err = -EINVAL;
> +		goto err_close;
> +	}
> +	if (sym_off + jt_size > obj->jumptables_data_sz) {
> +		pr_warn("map '.jumptables': jumptables_data size is %zd, trying to access %d\n",
> +			obj->jumptables_data_sz, sym_off + jt_size);
> +		err = -EINVAL;
> +		goto err_close;
> +	}
> +
> +	subprog_idx = -1; /* main program */
> +	if (relo->insn_idx < 0 || relo->insn_idx >= prog->insns_cnt) {
> +		pr_warn("map '.jumptables': invalid instruction index %d\n", relo->insn_idx);
> +		err = -EINVAL;
> +		goto err_close;
> +	}
> +	if (prog->subprogs)
> +		subprog_idx = find_subprog_idx(prog, relo->insn_idx);
> +
> +	jt = (__u64 *)(obj->jumptables_data + sym_off);
> +	for (i = 0; i < max_entries; i++) {
> +		/*
> +		 * The offset should be made to be relative to the beginning of
> +		 * the main function, not the subfunction.
> +		 */
> +		insn_off = jt[i]/sizeof(struct bpf_insn);
> +		if (subprog_idx >= 0) {
> +			insn_off -= prog->subprogs[subprog_idx].sec_insn_off;
> +			insn_off += prog->subprogs[subprog_idx].sub_insn_off;
> +		} else {
> +			insn_off -= prog->sec_insn_off;
> +		}
> +
> +		/*
> +		 * LLVM-generated jump tables contain u64 records, however
> +		 * should contain values that fit in u32.
> +		 */
> +		if (insn_off > UINT32_MAX) {
> +			pr_warn("map '.jumptables': invalid jump table value 0x%llx at offset %d\n",
> +				(long long)jt[i], sym_off + i);
                                                      ^^^^^^^^^^^^

Does this error message print the correct offset? Since jt is a pointer
to __u64, the array access jt[i] is at byte offset "sym_off + i * 8",
not "sym_off + i". All the other error messages in create_jt_map report
byte offsets and sizes (sym_off, jt_size, sym_off + jt_size), so this
one should probably be "sym_off + i * jt_entry_size" for consistency.

> +			err = -EINVAL;
> +			goto err_close;
> +		}
> +
> +		val.orig_off = insn_off;
> +		err = bpf_map_update_elem(map_fd, &i, &val, 0);
> +		if (err)
> +			goto err_close;
> +	}
> +
> +	err = bpf_map_freeze(map_fd);
> +	if (err)
> +		goto err_close;
> +
> +	err = add_jt_map(obj, prog, sym_off, map_fd);
> +	if (err)
> +		goto err_close;
> +
> +	return map_fd;
> +
> +err_close:
> +	close(map_fd);
> +	return err;
> +}

[ ... ]

---

AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

In-Reply-To-Subject: libbpf: support llvm-generated indirect jumps
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/19018051915

[kernel-patches-daemon-bpf]

Forwarding comment 3478352639 via email
In-Reply-To: [email protected]
Patch: https://patchwork.kernel.org/project/netdevbpf/patch/[email protected]/

[kernel-patches-review-bot]

commit 6cb8001495faae09541d55309638b3c1920bae06
Author: Anton Protopopov <[email protected]>

bpf, x86: add support for indirect jumps

This patch adds support for indirect jumps in BPF through a new
BPF_JMP|BPF_X|BPF_JA instruction. It introduces the PTR_TO_INSN register
type for instruction pointers loaded from INSN_ARRAY maps, along with
verifier support for tracking jump tables and x86 JIT implementation.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c

[ ... ]

> @@ -21374,6 +21374,28 @@ static int bpf_adj_linfo_after_remove(struct bpf_verifier_env *env, u32 off,
>  	return 0;
>  }
>
> +/*
> + * Clean up dynamically allocated fields of aux data for instructions [start, ...]
> + */
> +static void clear_insn_aux_data(struct bpf_verifier_env *env, int start, int len)
> +{
> +	struct bpf_insn_aux_data *aux_data = env->insn_aux_data;
> +	struct bpf_insn *insns = env->prog->insnsi;
> +	int end = start + len;
> +	int i;
> +
> +	for (i = start; i < end; i++) {
> +		if (insn_is_gotox(&insns[i])) {
> +			kvfree(aux_data[i].jt);
> +			aux_data[i].jt = NULL;
> +		}
> +
> +		if (bpf_is_ldimm64(&insns[i]))
> +			i++;
> +	}
> +}
> +
>  static int verifier_remove_insns(struct bpf_verifier_env *env, u32 off, u32 cnt)
>  {
>  	struct bpf_insn_aux_data *aux_data = env->insn_aux_data;
> @@ -21393,6 +21415,8 @@ static int verifier_remove_insns(struct bpf_verifier_env *env, u32 off, u32 cnt
>
>  	adjust_insn_arrays_after_remove(env, off, cnt);
>
> +	clear_insn_aux_data(env, off, cnt);
> +
>  	memmove(aux_data + off,	aux_data + off + cnt,
>  		sizeof(*aux_data) * (orig_prog_len - off - cnt));

Can clear_insn_aux_data() correctly identify which aux_data entries need
cleanup when it's called after bpf_remove_insns()? After instructions are
removed, the insns[] array has been modified - insns[off] now contains what
was previously insns[off+cnt]. But aux_data hasn't been shifted yet, so
clear_insn_aux_data() checks the wrong instructions when deciding which
aux_data entries to free.

For example, if instructions [1,2] are removed where insn 2 is a gotox:
- bpf_remove_insns() removes insns[1] and insns[2], shifts later insns down
- insns[1] now contains what was insns[3]
- clear_insn_aux_data() checks insn_is_gotox(&insns[1]) (now insn 3!)
- It might free aux_data[1].jt if insn 3 is a gotox, even though insn 3
  isn't being removed
- It never checks the original insns[2] which was the actual gotox that
  needed cleanup

Should clear_insn_aux_data() be called before bpf_remove_insns() instead?

---

AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

In-Reply-To-Subject: bpf, x86: add support for indirect jumps
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/19018051915

[kernel-patches-daemon-bpf]

Forwarding comment 3478354329 via email
In-Reply-To: [email protected]
Patch: https://patchwork.kernel.org/project/netdevbpf/patch/[email protected]/

[kernel-patches-daemon-bpf]

Upstream branch: e6e10c5
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1018668
version: 10

[kernel-patches-daemon-bpf]

Upstream branch: 5dae745
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1018668
version: 10

[kernel-patches-daemon-bpf]

At least one diff in series https://patchwork.kernel.org/project/netdevbpf/list/?series=1018668 expired. Closing PR.