diff mbox series

[bpf-next,RFC,v1,2/8] bpf: Be conservative during verification for invalid kfunc calls

Message ID 20210830173424.1385796-3-memxor@gmail.com (mailing list archive)
State RFC
Delegated to: BPF
Headers show
Series Support kernel module function calls from eBPF | expand

Checks

Context Check Description
bpf/vmtest-bpf-next pending Kernel LATEST + selftests
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for bpf-next
netdev/subject_prefix success Link
netdev/cc_maintainers warning 2 maintainers not CCed: kpsingh@kernel.org john.fastabend@gmail.com
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 30 this patch: 30
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch warning WARNING: line length of 82 exceeds 80 columns WARNING: line length of 95 exceeds 80 columns
netdev/build_allmodconfig_warn success Errors and warnings before: 30 this patch: 30
netdev/header_inline success Link

Commit Message

Kumar Kartikeya Dwivedi Aug. 30, 2021, 5:34 p.m. UTC
This change modifies the BPF verifier to only return error for invalid
kfunc calls specially marked by userspace (with insn->imm == 0) after
the verifier has eliminated dead instructions. This can be handled in
the fixup stage, and skip processing during add and check stages.

If such an invalid call is dropped, the fixup stage will not encounter
insn->imm as 0, otherwise it bails out and returns an error.

This can be used by userspace to use branches to call old and new kfunc
helpers across kernel versions by setting the rodata map value before
loading the BPF program, enhancing runtime portability. The next patch
introduces libbpf support for this.

Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 kernel/bpf/verifier.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)
diff mbox series

Patch

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index de0670a8b1df..9904b9a96b04 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -1730,6 +1730,15 @@  static int add_kfunc_call(struct bpf_verifier_env *env, u32 func_id, s16 offset)
 		prog_aux->kfunc_tab = tab;
 	}
 
+	/* btf_idr allocates IDs from 1, so func_id == 0 is always invalid, but
+	 * instead of returning an error, be conservative and wait until the
+	 * code elimination pass before returning error, so that invalid calls
+	 * that get pruned out can be in BPF programs loaded from userspace.
+	 * It is also required that offset be 0.
+	 */
+	if (!func_id && !offset)
+		return 0;
+
 	desc_btf = find_kfunc_desc_btf(env, func_id, offset);
 	if (IS_ERR(desc_btf)) {
 		verbose(env, "failed to find BTF for kernel function\n");
@@ -6527,6 +6536,10 @@  static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn)
 	struct btf *desc_btf;
 	int err;
 
+	/* skip for now, but return error when we find this in fixup_kfunc_call */
+	if (!insn->imm)
+		return 0;
+
 	desc_btf = find_kfunc_desc_btf(env, insn->imm, insn->off);
 	if (IS_ERR(desc_btf))
 		return PTR_ERR(desc_btf);
@@ -12658,6 +12671,11 @@  static int fixup_kfunc_call(struct bpf_verifier_env *env,
 {
 	const struct bpf_kfunc_desc *desc;
 
+	if (!insn->imm) {
+		verbose(env, "invalid kernel function call not eliminated in verifier pass\n");
+		return -EINVAL;
+	}
+
 	/* insn->imm has the btf func_id. Replace it with
 	 * an address (relative to __bpf_base_call).
 	 */