bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf-next v3 00/10] Support kernel module function calls from eBPF
@ 2021-09-15  5:09 Kumar Kartikeya Dwivedi
  2021-09-15  5:09 ` [PATCH bpf-next v3 01/10] bpf: Introduce BPF support for kernel module function calls Kumar Kartikeya Dwivedi
                   ` (10 more replies)
  0 siblings, 11 replies; 20+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2021-09-15  5:09 UTC (permalink / raw)
  To: bpf
  Cc: Kumar Kartikeya Dwivedi, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
	Jesper Dangaard Brouer, Toke Høiland-Jørgensen, netdev

This set enables kernel module function calls, and also modifies verifier logic
to permit invalid kernel function calls as long as they are pruned as part of
dead code elimination. This is done to provide better runtime portability for
BPF objects, which can conditionally disable parts of code that are pruned later
by the verifier (e.g. const volatile vars, kconfig options). libbpf
modifications are made along with kernel changes to support module function
calls. The set includes gen_loader support for emitting kfunc relocations.

It also converts TCP congestion control objects to use the module kfunc support
instead of relying on IS_BUILTIN ifdef.

Changelog:
----------
v2 -> v3:
v2: https://lore.kernel.org/bpf/20210914123750.460750-1-memxor@gmail.com

 * Fix issues pointed out by Kernel Test Robot
 * Fix find_kfunc_desc to also take offset into consideration when comparing

RFC v1 -> v2
v1: https://lore.kernel.org/bpf/20210830173424.1385796-1-memxor@gmail.com

 * Address comments from Alexei
   * Reuse fd_array instead of introducing kfunc_btf_fds array
   * Take btf and module reference as needed, instead of preloading
   * Add BTF_KIND_FUNC relocation support to gen_loader infrastructure
 * Address comments from Andrii
   * Drop hashmap in libbpf for finding index of existing BTF in fd_array
   * Preserve invalid kfunc calls only when the symbol is weak
 * Adjust verifier selftests

Kumar Kartikeya Dwivedi (10):
  bpf: Introduce BPF support for kernel module function calls
  bpf: Be conservative while processing invalid kfunc calls
  bpf: btf: Introduce helpers for dynamic BTF set registration
  tools: Allow specifying base BTF file in resolve_btfids
  bpf: Enable TCP congestion control kfunc from modules
  bpf: Bump MAX_BPF_STACK size to 768 bytes
  libbpf: Support kernel module function calls
  libbpf: Resolve invalid weak kfunc calls with imm = 0, off = 0
  libbpf: Update gen_loader to emit BTF_KIND_FUNC relocations
  bpf, selftests: Add basic test for module kfunc call

 include/linux/bpf.h                           |   8 +-
 include/linux/bpf_verifier.h                  |   2 +
 include/linux/bpfptr.h                        |   1 +
 include/linux/btf.h                           |  38 +++
 include/linux/filter.h                        |   4 +-
 kernel/bpf/btf.c                              |  56 +++++
 kernel/bpf/core.c                             |   4 +
 kernel/bpf/verifier.c                         | 217 +++++++++++++++---
 kernel/trace/bpf_trace.c                      |   1 +
 net/bpf/test_run.c                            |   2 +-
 net/ipv4/bpf_tcp_ca.c                         |  36 +--
 net/ipv4/tcp_bbr.c                            |  28 ++-
 net/ipv4/tcp_cubic.c                          |  26 ++-
 net/ipv4/tcp_dctcp.c                          |  26 ++-
 scripts/Makefile.modfinal                     |   1 +
 tools/bpf/resolve_btfids/main.c               |  19 +-
 tools/lib/bpf/bpf.c                           |   1 +
 tools/lib/bpf/bpf_gen_internal.h              |  12 +-
 tools/lib/bpf/gen_loader.c                    |  93 +++++++-
 tools/lib/bpf/libbpf.c                        |  81 +++++--
 tools/lib/bpf/libbpf_internal.h               |   1 +
 tools/testing/selftests/bpf/Makefile          |   1 +
 .../selftests/bpf/bpf_testmod/bpf_testmod.c   |  23 +-
 .../selftests/bpf/prog_tests/ksyms_module.c   |  13 +-
 .../bpf/prog_tests/ksyms_module_libbpf.c      |  18 ++
 .../selftests/bpf/progs/test_ksyms_module.c   |   9 +
 .../bpf/progs/test_ksyms_module_libbpf.c      |  35 +++
 tools/testing/selftests/bpf/verifier/calls.c  |  22 +-
 .../selftests/bpf/verifier/raw_stack.c        |   4 +-
 .../selftests/bpf/verifier/stack_ptr.c        |   6 +-
 .../testing/selftests/bpf/verifier/var_off.c  |   4 +-
 31 files changed, 673 insertions(+), 119 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/ksyms_module_libbpf.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_ksyms_module_libbpf.c

-- 
2.33.0


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH bpf-next v3 01/10] bpf: Introduce BPF support for kernel module function calls
  2021-09-15  5:09 [PATCH bpf-next v3 00/10] Support kernel module function calls from eBPF Kumar Kartikeya Dwivedi
@ 2021-09-15  5:09 ` Kumar Kartikeya Dwivedi
  2021-09-15  5:09 ` [PATCH bpf-next v3 02/10] bpf: Be conservative while processing invalid kfunc calls Kumar Kartikeya Dwivedi
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2021-09-15  5:09 UTC (permalink / raw)
  To: bpf
  Cc: Kumar Kartikeya Dwivedi, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
	Jesper Dangaard Brouer, Toke Høiland-Jørgensen, netdev

This change adds support on the kernel side to allow for BPF programs to
call kernel module functions. Userspace will prepare an array of module
BTF fds that is passed in during BPF_PROG_LOAD using fd_array parameter.
In the kernel, the module BTFs are placed in the auxilliary struct for
bpf_prog, and loaded as needed.

The verifier then uses insn->off to index into the fd_array. insn->off
is used by subtracting one from it, as userspace has to set the index of
array in insn->off incremented by 1. This lets us denote vmlinux btf by
insn->off == 0, and the module kfunc using insn->off > 0.  They are
sorted based on offset in an array, and each offset corresponds to one
descriptor, with a max limit up to 256 such module BTFs.

Another change is to check_kfunc_call callback, which now include a
struct module * pointer, this is to be used in later patch such that the
kfunc_id and module pointer are matched for dynamically registered BTF
sets from loadable modules, so that same kfunc_id in two modules doesn't
lead to check_kfunc_call succeeding. For the duration of the
check_kfunc_call, the reference to struct module exists, as it returns
the pointer stored in kfunc_btf_tab.

Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 include/linux/bpf.h          |   8 +-
 include/linux/bpf_verifier.h |   2 +
 kernel/bpf/core.c            |   4 +
 kernel/bpf/verifier.c        | 199 ++++++++++++++++++++++++++++++-----
 net/bpf/test_run.c           |   2 +-
 net/ipv4/bpf_tcp_ca.c        |   2 +-
 6 files changed, 185 insertions(+), 32 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index f4c16f19f83e..148bd899411e 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -511,7 +511,7 @@ struct bpf_verifier_ops {
 				 const struct btf_type *t, int off, int size,
 				 enum bpf_access_type atype,
 				 u32 *next_btf_id);
-	bool (*check_kfunc_call)(u32 kfunc_btf_id);
+	bool (*check_kfunc_call)(u32 kfunc_btf_id, struct module *owner);
 };
 
 struct bpf_prog_offload_ops {
@@ -874,6 +874,7 @@ struct bpf_prog_aux {
 	void *jit_data; /* JIT specific data. arch dependent */
 	struct bpf_jit_poke_descriptor *poke_tab;
 	struct bpf_kfunc_desc_tab *kfunc_tab;
+	struct bpf_kfunc_btf_tab *kfunc_btf_tab;
 	u32 size_poke_tab;
 	struct bpf_ksym ksym;
 	const struct bpf_prog_ops *ops;
@@ -1635,7 +1636,7 @@ int bpf_prog_test_run_raw_tp(struct bpf_prog *prog,
 int bpf_prog_test_run_sk_lookup(struct bpf_prog *prog,
 				const union bpf_attr *kattr,
 				union bpf_attr __user *uattr);
-bool bpf_prog_test_check_kfunc_call(u32 kfunc_id);
+bool bpf_prog_test_check_kfunc_call(u32 kfunc_id, struct module *owner);
 bool btf_ctx_access(int off, int size, enum bpf_access_type type,
 		    const struct bpf_prog *prog,
 		    struct bpf_insn_access_aux *info);
@@ -1856,7 +1857,8 @@ static inline int bpf_prog_test_run_sk_lookup(struct bpf_prog *prog,
 	return -ENOTSUPP;
 }
 
-static inline bool bpf_prog_test_check_kfunc_call(u32 kfunc_id)
+static inline bool bpf_prog_test_check_kfunc_call(u32 kfunc_id,
+						  struct module *owner)
 {
 	return false;
 }
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index 5424124dbe36..c8a78e830fca 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -527,5 +527,7 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
 			    const struct bpf_prog *tgt_prog,
 			    u32 btf_id,
 			    struct bpf_attach_target_info *tgt_info);
+void bpf_free_kfunc_btf_tab(struct bpf_kfunc_btf_tab *tab);
+
 
 #endif /* _LINUX_BPF_VERIFIER_H */
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 9f4636d021b1..8fe14942e6b6 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -32,6 +32,7 @@
 #include <linux/perf_event.h>
 #include <linux/extable.h>
 #include <linux/log2.h>
+#include <linux/bpf_verifier.h>
 
 #include <asm/barrier.h>
 #include <asm/unaligned.h>
@@ -2255,6 +2256,9 @@ static void bpf_prog_free_deferred(struct work_struct *work)
 	int i;
 
 	aux = container_of(work, struct bpf_prog_aux, work);
+#ifdef CONFIG_BPF_SYSCALL
+	bpf_free_kfunc_btf_tab(aux->kfunc_btf_tab);
+#endif
 	bpf_free_used_maps(aux);
 	bpf_free_used_btfs(aux);
 	if (bpf_prog_is_dev_bound(aux))
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 047ac4b4703b..3a35af7d1180 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -1626,52 +1626,170 @@ static int add_subprog(struct bpf_verifier_env *env, int off)
 	return env->subprog_cnt - 1;
 }
 
+#define MAX_KFUNC_DESCS 256
+#define MAX_KFUNC_BTFS	256
+
 struct bpf_kfunc_desc {
 	struct btf_func_model func_model;
 	u32 func_id;
 	s32 imm;
+	u16 offset;
+};
+
+struct bpf_kfunc_btf {
+	struct btf *btf;
+	struct module *module;
+	u16 offset;
 };
 
-#define MAX_KFUNC_DESCS 256
 struct bpf_kfunc_desc_tab {
 	struct bpf_kfunc_desc descs[MAX_KFUNC_DESCS];
 	u32 nr_descs;
 };
 
-static int kfunc_desc_cmp_by_id(const void *a, const void *b)
+struct bpf_kfunc_btf_tab {
+	struct bpf_kfunc_btf descs[MAX_KFUNC_BTFS];
+	u32 nr_descs;
+};
+
+static int kfunc_desc_cmp_by_id_off(const void *a, const void *b)
 {
 	const struct bpf_kfunc_desc *d0 = a;
 	const struct bpf_kfunc_desc *d1 = b;
 
 	/* func_id is not greater than BTF_MAX_TYPE */
-	return d0->func_id - d1->func_id;
+	return d0->func_id - d1->func_id ?: d0->offset - d1->offset;
+}
+
+static int kfunc_btf_cmp_by_off(const void *a, const void *b)
+{
+	const struct bpf_kfunc_btf *d0 = a;
+	const struct bpf_kfunc_btf *d1 = b;
+
+	return d0->offset - d1->offset;
 }
 
 static const struct bpf_kfunc_desc *
-find_kfunc_desc(const struct bpf_prog *prog, u32 func_id)
+find_kfunc_desc(const struct bpf_prog *prog, u32 func_id, u16 offset)
 {
 	struct bpf_kfunc_desc desc = {
 		.func_id = func_id,
+		.offset = offset,
 	};
 	struct bpf_kfunc_desc_tab *tab;
 
 	tab = prog->aux->kfunc_tab;
 	return bsearch(&desc, tab->descs, tab->nr_descs,
-		       sizeof(tab->descs[0]), kfunc_desc_cmp_by_id);
+		       sizeof(tab->descs[0]), kfunc_desc_cmp_by_id_off);
+}
+
+/* Expects offset always greater than 0 */
+static struct btf *__find_kfunc_desc_btf(struct bpf_verifier_env *env,
+					 s16 offset, struct module **btf_modp)
+{
+	struct bpf_kfunc_btf kf_btf = { .offset = offset - 1 };
+	struct bpf_kfunc_btf_tab *tab;
+	struct module *mod = NULL;
+	struct bpf_kfunc_btf *b;
+	struct btf *btf;
+	int btf_fd;
+
+	tab = env->prog->aux->kfunc_btf_tab;
+	b = bsearch(&kf_btf, tab->descs, tab->nr_descs,
+		    sizeof(tab->descs[0]), kfunc_btf_cmp_by_off);
+	if (!b) {
+		if (tab->nr_descs == MAX_KFUNC_BTFS) {
+			verbose(env, "too many different module BTFs\n");
+			return ERR_PTR(-E2BIG);
+		}
+
+		offset -= 1;
+		if (copy_from_bpfptr_offset(&btf_fd, env->fd_array,
+					    offset * sizeof(btf_fd),
+					    sizeof(btf_fd)))
+			return ERR_PTR(-EFAULT);
+
+		btf = btf_get_by_fd(btf_fd);
+		if (IS_ERR(btf))
+			return btf;
+
+		if (!btf_is_module(btf)) {
+			verbose(env, "BTF fd for kfunc is not a module BTF\n");
+			btf_put(btf);
+			return ERR_PTR(-EINVAL);
+		}
+
+		mod = btf_try_get_module(btf);
+		if (!mod) {
+			btf_put(btf);
+			return ERR_PTR(-ENXIO);
+		}
+
+		b = &tab->descs[tab->nr_descs++];
+		b->btf = btf;
+		b->module = mod;
+		b->offset = offset;
+
+		sort(tab->descs, tab->nr_descs, sizeof(tab->descs[0]),
+		     kfunc_btf_cmp_by_off, NULL);
+	}
+	if (btf_modp)
+		*btf_modp = b->module;
+	return b->btf;
+}
+
+void bpf_free_kfunc_btf_tab(struct bpf_kfunc_btf_tab *tab)
+{
+	if (!tab)
+		return;
+
+	while (tab->nr_descs--) {
+		module_put(tab->descs[tab->nr_descs].module);
+		btf_put(tab->descs[tab->nr_descs].btf);
+	}
+	kfree(tab);
+}
+
+static struct btf *find_kfunc_desc_btf(struct bpf_verifier_env *env,
+				       u32 func_id, s16 offset,
+				       struct module **btf_modp)
+{
+	struct btf *kfunc_btf;
+
+	if (offset) {
+		if (offset < 0) {
+			/* In the future, this can be allowed to increase limit
+			 * of fd index into fd_array, interpreted as unsigned u16.
+			 */
+			verbose(env, "negative offset disallowed for kernel module function call\n");
+			return ERR_PTR(-EINVAL);
+		}
+
+		kfunc_btf = __find_kfunc_desc_btf(env, offset, btf_modp);
+		if (IS_ERR_OR_NULL(kfunc_btf)) {
+			verbose(env, "cannot find module BTF for func_id %u\n", func_id);
+			return kfunc_btf ?: ERR_PTR(-ENOENT);
+		}
+		return kfunc_btf;
+	}
+	return btf_vmlinux ?: ERR_PTR(-ENOENT);
 }
 
-static int add_kfunc_call(struct bpf_verifier_env *env, u32 func_id)
+static int add_kfunc_call(struct bpf_verifier_env *env, u32 func_id, s16 offset)
 {
 	const struct btf_type *func, *func_proto;
+	struct bpf_kfunc_btf_tab *btf_tab;
 	struct bpf_kfunc_desc_tab *tab;
 	struct bpf_prog_aux *prog_aux;
 	struct bpf_kfunc_desc *desc;
 	const char *func_name;
+	struct btf *desc_btf;
 	unsigned long addr;
 	int err;
 
 	prog_aux = env->prog->aux;
 	tab = prog_aux->kfunc_tab;
+	btf_tab = prog_aux->kfunc_btf_tab;
 	if (!tab) {
 		if (!btf_vmlinux) {
 			verbose(env, "calling kernel function is not supported without CONFIG_DEBUG_INFO_BTF\n");
@@ -1699,7 +1817,20 @@ static int add_kfunc_call(struct bpf_verifier_env *env, u32 func_id)
 		prog_aux->kfunc_tab = tab;
 	}
 
-	if (find_kfunc_desc(env->prog, func_id))
+	if (!btf_tab && offset) {
+		btf_tab = kzalloc(sizeof(*btf_tab), GFP_KERNEL);
+		if (!btf_tab)
+			return -ENOMEM;
+		prog_aux->kfunc_btf_tab = btf_tab;
+	}
+
+	desc_btf = find_kfunc_desc_btf(env, func_id, offset, NULL);
+	if (IS_ERR(desc_btf)) {
+		verbose(env, "failed to find BTF for kernel function\n");
+		return PTR_ERR(desc_btf);
+	}
+
+	if (find_kfunc_desc(env->prog, func_id, offset))
 		return 0;
 
 	if (tab->nr_descs == MAX_KFUNC_DESCS) {
@@ -1707,20 +1838,20 @@ static int add_kfunc_call(struct bpf_verifier_env *env, u32 func_id)
 		return -E2BIG;
 	}
 
-	func = btf_type_by_id(btf_vmlinux, func_id);
+	func = btf_type_by_id(desc_btf, func_id);
 	if (!func || !btf_type_is_func(func)) {
 		verbose(env, "kernel btf_id %u is not a function\n",
 			func_id);
 		return -EINVAL;
 	}
-	func_proto = btf_type_by_id(btf_vmlinux, func->type);
+	func_proto = btf_type_by_id(desc_btf, func->type);
 	if (!func_proto || !btf_type_is_func_proto(func_proto)) {
 		verbose(env, "kernel function btf_id %u does not have a valid func_proto\n",
 			func_id);
 		return -EINVAL;
 	}
 
-	func_name = btf_name_by_offset(btf_vmlinux, func->name_off);
+	func_name = btf_name_by_offset(desc_btf, func->name_off);
 	addr = kallsyms_lookup_name(func_name);
 	if (!addr) {
 		verbose(env, "cannot find address for kernel function %s\n",
@@ -1730,13 +1861,14 @@ static int add_kfunc_call(struct bpf_verifier_env *env, u32 func_id)
 
 	desc = &tab->descs[tab->nr_descs++];
 	desc->func_id = func_id;
+	desc->offset = offset;
 	desc->imm = BPF_CAST_CALL(addr) - __bpf_call_base;
-	err = btf_distill_func_proto(&env->log, btf_vmlinux,
+	err = btf_distill_func_proto(&env->log, desc_btf,
 				     func_proto, func_name,
 				     &desc->func_model);
 	if (!err)
 		sort(tab->descs, tab->nr_descs, sizeof(tab->descs[0]),
-		     kfunc_desc_cmp_by_id, NULL);
+		     kfunc_desc_cmp_by_id_off, NULL);
 	return err;
 }
 
@@ -1815,7 +1947,7 @@ static int add_subprog_and_kfunc(struct bpf_verifier_env *env)
 		} else if (bpf_pseudo_call(insn)) {
 			ret = add_subprog(env, i + insn->imm + 1);
 		} else {
-			ret = add_kfunc_call(env, insn->imm);
+			ret = add_kfunc_call(env, insn->imm, insn->off);
 		}
 
 		if (ret < 0)
@@ -2152,12 +2284,17 @@ static int get_prev_insn_idx(struct bpf_verifier_state *st, int i,
 static const char *disasm_kfunc_name(void *data, const struct bpf_insn *insn)
 {
 	const struct btf_type *func;
+	struct btf *desc_btf;
 
 	if (insn->src_reg != BPF_PSEUDO_KFUNC_CALL)
 		return NULL;
 
-	func = btf_type_by_id(btf_vmlinux, insn->imm);
-	return btf_name_by_offset(btf_vmlinux, func->name_off);
+	desc_btf = find_kfunc_desc_btf(data, insn->imm, insn->off, NULL);
+	if (IS_ERR(desc_btf))
+		return "<error>";
+
+	func = btf_type_by_id(desc_btf, insn->imm);
+	return btf_name_by_offset(desc_btf, func->name_off);
 }
 
 /* For given verifier state backtrack_insn() is called from the last insn to
@@ -6485,23 +6622,29 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn)
 	struct bpf_reg_state *regs = cur_regs(env);
 	const char *func_name, *ptr_type_name;
 	u32 i, nargs, func_id, ptr_type_id;
+	struct module *btf_mod = NULL;
 	const struct btf_param *args;
+	struct btf *desc_btf;
 	int err;
 
+	desc_btf = find_kfunc_desc_btf(env, insn->imm, insn->off, &btf_mod);
+	if (IS_ERR(desc_btf))
+		return PTR_ERR(desc_btf);
+
 	func_id = insn->imm;
-	func = btf_type_by_id(btf_vmlinux, func_id);
-	func_name = btf_name_by_offset(btf_vmlinux, func->name_off);
-	func_proto = btf_type_by_id(btf_vmlinux, func->type);
+	func = btf_type_by_id(desc_btf, func_id);
+	func_name = btf_name_by_offset(desc_btf, func->name_off);
+	func_proto = btf_type_by_id(desc_btf, func->type);
 
 	if (!env->ops->check_kfunc_call ||
-	    !env->ops->check_kfunc_call(func_id)) {
+	    !env->ops->check_kfunc_call(func_id, btf_mod)) {
 		verbose(env, "calling kernel function %s is not allowed\n",
 			func_name);
 		return -EACCES;
 	}
 
 	/* Check the arguments */
-	err = btf_check_kfunc_arg_match(env, btf_vmlinux, func_id, regs);
+	err = btf_check_kfunc_arg_match(env, desc_btf, func_id, regs);
 	if (err)
 		return err;
 
@@ -6509,15 +6652,15 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn)
 		mark_reg_not_init(env, regs, caller_saved[i]);
 
 	/* Check return type */
-	t = btf_type_skip_modifiers(btf_vmlinux, func_proto->type, NULL);
+	t = btf_type_skip_modifiers(desc_btf, func_proto->type, NULL);
 	if (btf_type_is_scalar(t)) {
 		mark_reg_unknown(env, regs, BPF_REG_0);
 		mark_btf_func_reg_size(env, BPF_REG_0, t->size);
 	} else if (btf_type_is_ptr(t)) {
-		ptr_type = btf_type_skip_modifiers(btf_vmlinux, t->type,
+		ptr_type = btf_type_skip_modifiers(desc_btf, t->type,
 						   &ptr_type_id);
 		if (!btf_type_is_struct(ptr_type)) {
-			ptr_type_name = btf_name_by_offset(btf_vmlinux,
+			ptr_type_name = btf_name_by_offset(desc_btf,
 							   ptr_type->name_off);
 			verbose(env, "kernel function %s returns pointer type %s %s is not supported\n",
 				func_name, btf_type_str(ptr_type),
@@ -6525,7 +6668,7 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn)
 			return -EINVAL;
 		}
 		mark_reg_known_zero(env, regs, BPF_REG_0);
-		regs[BPF_REG_0].btf = btf_vmlinux;
+		regs[BPF_REG_0].btf = desc_btf;
 		regs[BPF_REG_0].type = PTR_TO_BTF_ID;
 		regs[BPF_REG_0].btf_id = ptr_type_id;
 		mark_btf_func_reg_size(env, BPF_REG_0, sizeof(void *));
@@ -6536,7 +6679,7 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn)
 	for (i = 0; i < nargs; i++) {
 		u32 regno = i + 1;
 
-		t = btf_type_skip_modifiers(btf_vmlinux, args[i].type, NULL);
+		t = btf_type_skip_modifiers(desc_btf, args[i].type, NULL);
 		if (btf_type_is_ptr(t))
 			mark_btf_func_reg_size(env, regno, sizeof(void *));
 		else
@@ -11074,7 +11217,8 @@ static int do_check(struct bpf_verifier_env *env)
 			env->jmps_processed++;
 			if (opcode == BPF_CALL) {
 				if (BPF_SRC(insn->code) != BPF_K ||
-				    insn->off != 0 ||
+				    (insn->src_reg != BPF_PSEUDO_KFUNC_CALL
+				     && insn->off != 0) ||
 				    (insn->src_reg != BPF_REG_0 &&
 				     insn->src_reg != BPF_PSEUDO_CALL &&
 				     insn->src_reg != BPF_PSEUDO_KFUNC_CALL) ||
@@ -12430,6 +12574,7 @@ static int jit_subprogs(struct bpf_verifier_env *env)
 		func[i]->aux->stack_depth = env->subprog_info[i].stack_depth;
 		func[i]->jit_requested = 1;
 		func[i]->aux->kfunc_tab = prog->aux->kfunc_tab;
+		func[i]->aux->kfunc_btf_tab = prog->aux->kfunc_btf_tab;
 		func[i]->aux->linfo = prog->aux->linfo;
 		func[i]->aux->nr_linfo = prog->aux->nr_linfo;
 		func[i]->aux->jited_linfo = prog->aux->jited_linfo;
@@ -12619,7 +12764,7 @@ static int fixup_kfunc_call(struct bpf_verifier_env *env,
 	/* insn->imm has the btf func_id. Replace it with
 	 * an address (relative to __bpf_base_call).
 	 */
-	desc = find_kfunc_desc(env->prog, insn->imm);
+	desc = find_kfunc_desc(env->prog, insn->imm, insn->off);
 	if (!desc) {
 		verbose(env, "verifier internal error: kernel function descriptor not found for func_id %u\n",
 			insn->imm);
diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index fcb2f493f710..fe5c34f414a2 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -241,7 +241,7 @@ BTF_ID(func, bpf_kfunc_call_test2)
 BTF_ID(func, bpf_kfunc_call_test3)
 BTF_SET_END(test_sk_kfunc_ids)
 
-bool bpf_prog_test_check_kfunc_call(u32 kfunc_id)
+bool bpf_prog_test_check_kfunc_call(u32 kfunc_id, struct module *owner)
 {
 	return btf_id_set_contains(&test_sk_kfunc_ids, kfunc_id);
 }
diff --git a/net/ipv4/bpf_tcp_ca.c b/net/ipv4/bpf_tcp_ca.c
index 0dcee9df1326..b3afd3361f34 100644
--- a/net/ipv4/bpf_tcp_ca.c
+++ b/net/ipv4/bpf_tcp_ca.c
@@ -255,7 +255,7 @@ BTF_ID(func, bbr_set_state)
 #endif	/* CONFIG_X86 */
 BTF_SET_END(bpf_tcp_ca_kfunc_ids)
 
-static bool bpf_tcp_ca_check_kfunc_call(u32 kfunc_btf_id)
+static bool bpf_tcp_ca_check_kfunc_call(u32 kfunc_btf_id, struct module *owner)
 {
 	return btf_id_set_contains(&bpf_tcp_ca_kfunc_ids, kfunc_btf_id);
 }
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH bpf-next v3 02/10] bpf: Be conservative while processing invalid kfunc calls
  2021-09-15  5:09 [PATCH bpf-next v3 00/10] Support kernel module function calls from eBPF Kumar Kartikeya Dwivedi
  2021-09-15  5:09 ` [PATCH bpf-next v3 01/10] bpf: Introduce BPF support for kernel module function calls Kumar Kartikeya Dwivedi
@ 2021-09-15  5:09 ` Kumar Kartikeya Dwivedi
  2021-09-15  5:09 ` [PATCH bpf-next v3 03/10] bpf: btf: Introduce helpers for dynamic BTF set registration Kumar Kartikeya Dwivedi
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2021-09-15  5:09 UTC (permalink / raw)
  To: bpf
  Cc: Kumar Kartikeya Dwivedi, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
	Jesper Dangaard Brouer, Toke Høiland-Jørgensen, netdev

This patch also modifies the BPF verifier to only return error for
invalid kfunc calls specially marked by userspace (with insn->imm == 0,
insn->off == 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 will be exposed as weak ksym support in libbpf in subsequent patch.

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

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 3a35af7d1180..f241ba78b970 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -1817,6 +1817,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 untouched (0) for such calls.
+	 */
+	if (!func_id && !offset)
+		return 0;
+
 	if (!btf_tab && offset) {
 		btf_tab = kzalloc(sizeof(*btf_tab), GFP_KERNEL);
 		if (!btf_tab)
@@ -6627,6 +6636,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, &btf_mod);
 	if (IS_ERR(desc_btf))
 		return PTR_ERR(desc_btf);
@@ -12761,6 +12774,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).
 	 */
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH bpf-next v3 03/10] bpf: btf: Introduce helpers for dynamic BTF set registration
  2021-09-15  5:09 [PATCH bpf-next v3 00/10] Support kernel module function calls from eBPF Kumar Kartikeya Dwivedi
  2021-09-15  5:09 ` [PATCH bpf-next v3 01/10] bpf: Introduce BPF support for kernel module function calls Kumar Kartikeya Dwivedi
  2021-09-15  5:09 ` [PATCH bpf-next v3 02/10] bpf: Be conservative while processing invalid kfunc calls Kumar Kartikeya Dwivedi
@ 2021-09-15  5:09 ` Kumar Kartikeya Dwivedi
  2021-09-15 16:18   ` Alexei Starovoitov
  2021-09-15  5:09 ` [PATCH bpf-next v3 04/10] tools: Allow specifying base BTF file in resolve_btfids Kumar Kartikeya Dwivedi
                   ` (7 subsequent siblings)
  10 siblings, 1 reply; 20+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2021-09-15  5:09 UTC (permalink / raw)
  To: bpf
  Cc: Kumar Kartikeya Dwivedi, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
	Jesper Dangaard Brouer, Toke Høiland-Jørgensen, netdev

This adds helpers for registering btf_id_set from modules and the
check_kfunc_call callback that can be used to look them up.

With in kernel sets, the way this is supposed to work is, in kernel
callback looks up within the in-kernel kfunc whitelist, and then defers
to the dynamic BTF set lookup if it doesn't find the BTF id. If there is
no in-kernel BTF id set, this callback can be used directly.

Also fix includes for btf.h and bpfptr.h so that they can included in
isolation. This is in preparation for their usage in tcp_bbr, tcp_cubic
and tcp_dctcp modules in the next patch.

Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 include/linux/bpfptr.h |  1 +
 include/linux/btf.h    | 32 ++++++++++++++++++++++++++
 kernel/bpf/btf.c       | 51 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 84 insertions(+)

diff --git a/include/linux/bpfptr.h b/include/linux/bpfptr.h
index 546e27fc6d46..46e1757d06a3 100644
--- a/include/linux/bpfptr.h
+++ b/include/linux/bpfptr.h
@@ -3,6 +3,7 @@
 #ifndef _LINUX_BPFPTR_H
 #define _LINUX_BPFPTR_H
 
+#include <linux/mm.h>
 #include <linux/sockptr.h>
 
 typedef sockptr_t bpfptr_t;
diff --git a/include/linux/btf.h b/include/linux/btf.h
index 214fde93214b..e29a486d09d4 100644
--- a/include/linux/btf.h
+++ b/include/linux/btf.h
@@ -5,8 +5,10 @@
 #define _LINUX_BTF_H 1
 
 #include <linux/types.h>
+#include <linux/bpfptr.h>
 #include <uapi/linux/btf.h>
 #include <uapi/linux/bpf.h>
+#include <linux/mutex.h>
 
 #define BTF_TYPE_EMIT(type) ((void)(type *)0)
 #define BTF_TYPE_EMIT_ENUM(enum_val) ((void)enum_val)
@@ -238,4 +240,34 @@ static inline const char *btf_name_by_offset(const struct btf *btf,
 }
 #endif
 
+struct kfunc_btf_id_set {
+	struct list_head list;
+	struct btf_id_set *set;
+	struct module *owner;
+};
+
+struct kfunc_btf_id_list;
+
+#ifdef CONFIG_DEBUG_INFO_BTF_MODULES
+void register_kfunc_btf_id_set(struct kfunc_btf_id_list *l,
+			       struct kfunc_btf_id_set *s);
+void unregister_kfunc_btf_id_set(struct kfunc_btf_id_list *l,
+				 struct kfunc_btf_id_set *s);
+#else
+static inline void register_kfunc_btf_id_set(struct kfunc_btf_id_list *l,
+					     struct kfunc_btf_id_set *s)
+{
+}
+static inline void unregister_kfunc_btf_id_set(struct kfunc_btf_id_list *l,
+					       struct kfunc_btf_id_set *s)
+{
+}
+#endif
+
+#define DECLARE_CHECK_KFUNC_CALLBACK(type)                                     \
+	bool __bpf_check_##type##_kfunc_call(u32 kfunc_id, struct module *owner)
+#define DEFINE_KFUNC_BTF_ID_SET(set, name)                                     \
+	struct kfunc_btf_id_set name = { LIST_HEAD_INIT(name.list), (set),     \
+					 THIS_MODULE }
+
 #endif
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index c3d605b22473..d17f45b163f5 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -6343,3 +6343,54 @@ const struct bpf_func_proto bpf_btf_find_by_name_kind_proto = {
 };
 
 BTF_ID_LIST_GLOBAL_SINGLE(btf_task_struct_ids, struct, task_struct)
+
+struct kfunc_btf_id_list {
+	struct list_head list;
+	struct mutex mutex;
+};
+
+#ifdef CONFIG_DEBUG_INFO_BTF_MODULES
+
+void register_kfunc_btf_id_set(struct kfunc_btf_id_list *l,
+			       struct kfunc_btf_id_set *s)
+{
+	mutex_lock(&l->mutex);
+	list_add(&s->list, &l->list);
+	mutex_unlock(&l->mutex);
+}
+EXPORT_SYMBOL_GPL(register_kfunc_btf_id_set);
+
+void unregister_kfunc_btf_id_set(struct kfunc_btf_id_list *l,
+				 struct kfunc_btf_id_set *s)
+{
+	mutex_lock(&l->mutex);
+	list_del_init(&s->list);
+	mutex_unlock(&l->mutex);
+}
+EXPORT_SYMBOL_GPL(unregister_kfunc_btf_id_set);
+
+#endif
+
+#define DEFINE_KFUNC_BTF_ID_LIST(name)                                         \
+	struct kfunc_btf_id_list name = { LIST_HEAD_INIT(name.list),           \
+					  __MUTEX_INITIALIZER(name.mutex) }; \
+	EXPORT_SYMBOL_GPL(name)
+
+#define DEFINE_CHECK_KFUNC_CALLBACK(type, list_name)                           \
+	bool __bpf_check_##type##_kfunc_call(u32 kfunc_id,                     \
+					     struct module *owner)             \
+	{                                                                      \
+		struct kfunc_btf_id_set *s;                                    \
+		if (!owner || !IS_ENABLED(CONFIG_DEBUG_INFO_BTF_MODULES))      \
+			return false;                                          \
+		mutex_lock(&list_name.mutex);                                  \
+		list_for_each_entry(s, &list_name.list, list) {                \
+			if (s->owner == owner &&                               \
+			    btf_id_set_contains(s->set, kfunc_id)) {           \
+				mutex_unlock(&list_name.mutex);                \
+				return true;                                   \
+			}                                                      \
+		}                                                              \
+		mutex_unlock(&list_name.mutex);                                \
+		return false;                                                  \
+	}
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH bpf-next v3 04/10] tools: Allow specifying base BTF file in resolve_btfids
  2021-09-15  5:09 [PATCH bpf-next v3 00/10] Support kernel module function calls from eBPF Kumar Kartikeya Dwivedi
                   ` (2 preceding siblings ...)
  2021-09-15  5:09 ` [PATCH bpf-next v3 03/10] bpf: btf: Introduce helpers for dynamic BTF set registration Kumar Kartikeya Dwivedi
@ 2021-09-15  5:09 ` Kumar Kartikeya Dwivedi
  2021-09-15  5:09 ` [PATCH bpf-next v3 05/10] bpf: Enable TCP congestion control kfunc from modules Kumar Kartikeya Dwivedi
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2021-09-15  5:09 UTC (permalink / raw)
  To: bpf
  Cc: Kumar Kartikeya Dwivedi, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
	Jesper Dangaard Brouer, Toke Høiland-Jørgensen, netdev

This commits allows specifying the base BTF for resolving btf id
lists/sets during link time in the resolve_btfids tool. The base BTF is
set to NULL if no path is passed. This allows resolving BTF ids for
module kernel objects.

Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 tools/bpf/resolve_btfids/main.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/tools/bpf/resolve_btfids/main.c b/tools/bpf/resolve_btfids/main.c
index de6365b53c9c..206e1120082f 100644
--- a/tools/bpf/resolve_btfids/main.c
+++ b/tools/bpf/resolve_btfids/main.c
@@ -89,6 +89,7 @@ struct btf_id {
 struct object {
 	const char *path;
 	const char *btf;
+	const char *base_btf_path;
 
 	struct {
 		int		 fd;
@@ -477,16 +478,27 @@ static int symbols_resolve(struct object *obj)
 	int nr_structs  = obj->nr_structs;
 	int nr_unions   = obj->nr_unions;
 	int nr_funcs    = obj->nr_funcs;
+	struct btf *base_btf = NULL;
 	int err, type_id;
 	struct btf *btf;
 	__u32 nr_types;
 
-	btf = btf__parse(obj->btf ?: obj->path, NULL);
+	if (obj->base_btf_path) {
+		base_btf = btf__parse(obj->base_btf_path, NULL);
+		err = libbpf_get_error(base_btf);
+		if (err) {
+			pr_err("FAILED: load base BTF from %s: %s\n",
+			       obj->base_btf_path, strerror(-err));
+			return -1;
+		}
+	}
+
+	btf = btf__parse_split(obj->btf ?: obj->path, base_btf);
 	err = libbpf_get_error(btf);
 	if (err) {
 		pr_err("FAILED: load BTF from %s: %s\n",
 			obj->btf ?: obj->path, strerror(-err));
-		return -1;
+		goto out;
 	}
 
 	err = -1;
@@ -545,6 +557,7 @@ static int symbols_resolve(struct object *obj)
 
 	err = 0;
 out:
+	btf__free(base_btf);
 	btf__free(btf);
 	return err;
 }
@@ -697,6 +710,8 @@ int main(int argc, const char **argv)
 			   "BTF data"),
 		OPT_BOOLEAN(0, "no-fail", &no_fail,
 			   "do not fail if " BTF_IDS_SECTION " section is not found"),
+		OPT_STRING('s', "base-btf", &obj.base_btf_path, "file",
+			   "path of file providing base BTF data"),
 		OPT_END()
 	};
 	int err = -1;
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH bpf-next v3 05/10] bpf: Enable TCP congestion control kfunc from modules
  2021-09-15  5:09 [PATCH bpf-next v3 00/10] Support kernel module function calls from eBPF Kumar Kartikeya Dwivedi
                   ` (3 preceding siblings ...)
  2021-09-15  5:09 ` [PATCH bpf-next v3 04/10] tools: Allow specifying base BTF file in resolve_btfids Kumar Kartikeya Dwivedi
@ 2021-09-15  5:09 ` Kumar Kartikeya Dwivedi
  2021-09-15  5:09 ` [PATCH bpf-next v3 06/10] bpf: Bump MAX_BPF_STACK size to 768 bytes Kumar Kartikeya Dwivedi
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2021-09-15  5:09 UTC (permalink / raw)
  To: bpf
  Cc: Kumar Kartikeya Dwivedi, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
	Jesper Dangaard Brouer, Toke Høiland-Jørgensen, netdev

This commit moves BTF ID lookup into the newly added registration
helper, in a way that the bbr, cubic, and dctcp implementation set up
their sets in the bpf_tcp_ca kfunc_btf_set list, while the ones not
dependent on modules are looked up from the wrapper function.

This lifts the restriction for them to be compiled as built in objects,
and can be loaded as modules if required. Also modify Makefile.modfinal
to resolve_btfids in TCP congestion control modules if the config option
is set, using the base BTF support added in the previous commit.

See following commits for background on use of:

 CONFIG_X86 ifdef:
 569c484f9995 (bpf: Limit static tcp-cc functions in the .BTF_ids list to x86)

 CONFIG_DYNAMIC_FTRACE ifdef:
 7aae231ac93b (bpf: tcp: Limit calling some tcp cc functions to CONFIG_DYNAMIC_FTRACE)

[ resolve_btfids uses --no-fail because some crypto kernel modules
  under arch/x86/crypto generated from ASM do not have the .BTF sections ]

Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 include/linux/btf.h       |  4 ++++
 kernel/bpf/btf.c          |  3 +++
 net/ipv4/bpf_tcp_ca.c     | 34 +++-------------------------------
 net/ipv4/tcp_bbr.c        | 28 +++++++++++++++++++++++++++-
 net/ipv4/tcp_cubic.c      | 26 +++++++++++++++++++++++++-
 net/ipv4/tcp_dctcp.c      | 26 +++++++++++++++++++++++++-
 scripts/Makefile.modfinal |  1 +
 7 files changed, 88 insertions(+), 34 deletions(-)

diff --git a/include/linux/btf.h b/include/linux/btf.h
index e29a486d09d4..c7b6382123e1 100644
--- a/include/linux/btf.h
+++ b/include/linux/btf.h
@@ -270,4 +270,8 @@ static inline void unregister_kfunc_btf_id_set(struct kfunc_btf_id_list *l,
 	struct kfunc_btf_id_set name = { LIST_HEAD_INIT(name.list), (set),     \
 					 THIS_MODULE }
 
+extern struct kfunc_btf_id_list bpf_tcp_ca_kfunc_list;
+
+DECLARE_CHECK_KFUNC_CALLBACK(bpf_tcp_ca);
+
 #endif
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index d17f45b163f5..671b4f713a51 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -6394,3 +6394,6 @@ EXPORT_SYMBOL_GPL(unregister_kfunc_btf_id_set);
 		mutex_unlock(&list_name.mutex);                                \
 		return false;                                                  \
 	}
+
+DEFINE_KFUNC_BTF_ID_LIST(bpf_tcp_ca_kfunc_list);
+DEFINE_CHECK_KFUNC_CALLBACK(bpf_tcp_ca, bpf_tcp_ca_kfunc_list);
diff --git a/net/ipv4/bpf_tcp_ca.c b/net/ipv4/bpf_tcp_ca.c
index b3afd3361f34..c9f1d2dcf67b 100644
--- a/net/ipv4/bpf_tcp_ca.c
+++ b/net/ipv4/bpf_tcp_ca.c
@@ -223,41 +223,13 @@ BTF_ID(func, tcp_reno_cong_avoid)
 BTF_ID(func, tcp_reno_undo_cwnd)
 BTF_ID(func, tcp_slow_start)
 BTF_ID(func, tcp_cong_avoid_ai)
-#ifdef CONFIG_X86
-#ifdef CONFIG_DYNAMIC_FTRACE
-#if IS_BUILTIN(CONFIG_TCP_CONG_CUBIC)
-BTF_ID(func, cubictcp_init)
-BTF_ID(func, cubictcp_recalc_ssthresh)
-BTF_ID(func, cubictcp_cong_avoid)
-BTF_ID(func, cubictcp_state)
-BTF_ID(func, cubictcp_cwnd_event)
-BTF_ID(func, cubictcp_acked)
-#endif
-#if IS_BUILTIN(CONFIG_TCP_CONG_DCTCP)
-BTF_ID(func, dctcp_init)
-BTF_ID(func, dctcp_update_alpha)
-BTF_ID(func, dctcp_cwnd_event)
-BTF_ID(func, dctcp_ssthresh)
-BTF_ID(func, dctcp_cwnd_undo)
-BTF_ID(func, dctcp_state)
-#endif
-#if IS_BUILTIN(CONFIG_TCP_CONG_BBR)
-BTF_ID(func, bbr_init)
-BTF_ID(func, bbr_main)
-BTF_ID(func, bbr_sndbuf_expand)
-BTF_ID(func, bbr_undo_cwnd)
-BTF_ID(func, bbr_cwnd_event)
-BTF_ID(func, bbr_ssthresh)
-BTF_ID(func, bbr_min_tso_segs)
-BTF_ID(func, bbr_set_state)
-#endif
-#endif  /* CONFIG_DYNAMIC_FTRACE */
-#endif	/* CONFIG_X86 */
 BTF_SET_END(bpf_tcp_ca_kfunc_ids)
 
 static bool bpf_tcp_ca_check_kfunc_call(u32 kfunc_btf_id, struct module *owner)
 {
-	return btf_id_set_contains(&bpf_tcp_ca_kfunc_ids, kfunc_btf_id);
+	if (btf_id_set_contains(&bpf_tcp_ca_kfunc_ids, kfunc_btf_id))
+		return true;
+	return __bpf_check_bpf_tcp_ca_kfunc_call(kfunc_btf_id, owner);
 }
 
 static const struct bpf_verifier_ops bpf_tcp_ca_verifier_ops = {
diff --git a/net/ipv4/tcp_bbr.c b/net/ipv4/tcp_bbr.c
index 6274462b86b4..ec5550089b4d 100644
--- a/net/ipv4/tcp_bbr.c
+++ b/net/ipv4/tcp_bbr.c
@@ -56,6 +56,8 @@
  * otherwise TCP stack falls back to an internal pacing using one high
  * resolution timer per TCP socket and may use more resources.
  */
+#include <linux/btf.h>
+#include <linux/btf_ids.h>
 #include <linux/module.h>
 #include <net/tcp.h>
 #include <linux/inet_diag.h>
@@ -1152,14 +1154,38 @@ static struct tcp_congestion_ops tcp_bbr_cong_ops __read_mostly = {
 	.set_state	= bbr_set_state,
 };
 
+BTF_SET_START(tcp_bbr_kfunc_ids)
+#ifdef CONFIG_X86
+#ifdef CONFIG_DYNAMIC_FTRACE
+BTF_ID(func, bbr_init)
+BTF_ID(func, bbr_main)
+BTF_ID(func, bbr_sndbuf_expand)
+BTF_ID(func, bbr_undo_cwnd)
+BTF_ID(func, bbr_cwnd_event)
+BTF_ID(func, bbr_ssthresh)
+BTF_ID(func, bbr_min_tso_segs)
+BTF_ID(func, bbr_set_state)
+#endif
+#endif
+BTF_SET_END(tcp_bbr_kfunc_ids)
+
+static DEFINE_KFUNC_BTF_ID_SET(&tcp_bbr_kfunc_ids, tcp_bbr_kfunc_btf_set);
+
 static int __init bbr_register(void)
 {
+	int ret;
+
 	BUILD_BUG_ON(sizeof(struct bbr) > ICSK_CA_PRIV_SIZE);
-	return tcp_register_congestion_control(&tcp_bbr_cong_ops);
+	ret = tcp_register_congestion_control(&tcp_bbr_cong_ops);
+	if (ret)
+		return ret;
+	register_kfunc_btf_id_set(&bpf_tcp_ca_kfunc_list, &tcp_bbr_kfunc_btf_set);
+	return 0;
 }
 
 static void __exit bbr_unregister(void)
 {
+	unregister_kfunc_btf_id_set(&bpf_tcp_ca_kfunc_list, &tcp_bbr_kfunc_btf_set);
 	tcp_unregister_congestion_control(&tcp_bbr_cong_ops);
 }
 
diff --git a/net/ipv4/tcp_cubic.c b/net/ipv4/tcp_cubic.c
index 4a30deaa9a37..5e9d9c51164c 100644
--- a/net/ipv4/tcp_cubic.c
+++ b/net/ipv4/tcp_cubic.c
@@ -25,6 +25,8 @@
  */
 
 #include <linux/mm.h>
+#include <linux/btf.h>
+#include <linux/btf_ids.h>
 #include <linux/module.h>
 #include <linux/math64.h>
 #include <net/tcp.h>
@@ -482,8 +484,25 @@ static struct tcp_congestion_ops cubictcp __read_mostly = {
 	.name		= "cubic",
 };
 
+BTF_SET_START(tcp_cubic_kfunc_ids)
+#ifdef CONFIG_X86
+#ifdef CONFIG_DYNAMIC_FTRACE
+BTF_ID(func, cubictcp_init)
+BTF_ID(func, cubictcp_recalc_ssthresh)
+BTF_ID(func, cubictcp_cong_avoid)
+BTF_ID(func, cubictcp_state)
+BTF_ID(func, cubictcp_cwnd_event)
+BTF_ID(func, cubictcp_acked)
+#endif
+#endif
+BTF_SET_END(tcp_cubic_kfunc_ids)
+
+static DEFINE_KFUNC_BTF_ID_SET(&tcp_cubic_kfunc_ids, tcp_cubic_kfunc_btf_set);
+
 static int __init cubictcp_register(void)
 {
+	int ret;
+
 	BUILD_BUG_ON(sizeof(struct bictcp) > ICSK_CA_PRIV_SIZE);
 
 	/* Precompute a bunch of the scaling factors that are used per-packet
@@ -514,11 +533,16 @@ static int __init cubictcp_register(void)
 	/* divide by bic_scale and by constant Srtt (100ms) */
 	do_div(cube_factor, bic_scale * 10);
 
-	return tcp_register_congestion_control(&cubictcp);
+	ret = tcp_register_congestion_control(&cubictcp);
+	if (ret)
+		return ret;
+	register_kfunc_btf_id_set(&bpf_tcp_ca_kfunc_list, &tcp_cubic_kfunc_btf_set);
+	return 0;
 }
 
 static void __exit cubictcp_unregister(void)
 {
+	unregister_kfunc_btf_id_set(&bpf_tcp_ca_kfunc_list, &tcp_cubic_kfunc_btf_set);
 	tcp_unregister_congestion_control(&cubictcp);
 }
 
diff --git a/net/ipv4/tcp_dctcp.c b/net/ipv4/tcp_dctcp.c
index 79f705450c16..0d7ab3cc7b61 100644
--- a/net/ipv4/tcp_dctcp.c
+++ b/net/ipv4/tcp_dctcp.c
@@ -36,6 +36,8 @@
  *	Glenn Judd <glenn.judd@morganstanley.com>
  */
 
+#include <linux/btf.h>
+#include <linux/btf_ids.h>
 #include <linux/module.h>
 #include <linux/mm.h>
 #include <net/tcp.h>
@@ -236,14 +238,36 @@ static struct tcp_congestion_ops dctcp_reno __read_mostly = {
 	.name		= "dctcp-reno",
 };
 
+BTF_SET_START(tcp_dctcp_kfunc_ids)
+#ifdef CONFIG_X86
+#ifdef CONFIG_DYNAMIC_FTRACE
+BTF_ID(func, dctcp_init)
+BTF_ID(func, dctcp_update_alpha)
+BTF_ID(func, dctcp_cwnd_event)
+BTF_ID(func, dctcp_ssthresh)
+BTF_ID(func, dctcp_cwnd_undo)
+BTF_ID(func, dctcp_state)
+#endif
+#endif
+BTF_SET_END(tcp_dctcp_kfunc_ids)
+
+static DEFINE_KFUNC_BTF_ID_SET(&tcp_dctcp_kfunc_ids, tcp_dctcp_kfunc_btf_set);
+
 static int __init dctcp_register(void)
 {
+	int ret;
+
 	BUILD_BUG_ON(sizeof(struct dctcp) > ICSK_CA_PRIV_SIZE);
-	return tcp_register_congestion_control(&dctcp);
+	ret = tcp_register_congestion_control(&dctcp);
+	if (ret)
+		return ret;
+	register_kfunc_btf_id_set(&bpf_tcp_ca_kfunc_list, &tcp_dctcp_kfunc_btf_set);
+	return 0;
 }
 
 static void __exit dctcp_unregister(void)
 {
+	unregister_kfunc_btf_id_set(&bpf_tcp_ca_kfunc_list, &tcp_dctcp_kfunc_btf_set);
 	tcp_unregister_congestion_control(&dctcp);
 }
 
diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal
index ff805777431c..b4f83533eda6 100644
--- a/scripts/Makefile.modfinal
+++ b/scripts/Makefile.modfinal
@@ -41,6 +41,7 @@ quiet_cmd_btf_ko = BTF [M] $@
       cmd_btf_ko = 							\
 	if [ -f vmlinux ]; then						\
 		LLVM_OBJCOPY="$(OBJCOPY)" $(PAHOLE) -J --btf_base vmlinux $@; \
+		$(RESOLVE_BTFIDS) --no-fail -s vmlinux $@; 		\
 	else								\
 		printf "Skipping BTF generation for %s due to unavailability of vmlinux\n" $@ 1>&2; \
 	fi;
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH bpf-next v3 06/10] bpf: Bump MAX_BPF_STACK size to 768 bytes
  2021-09-15  5:09 [PATCH bpf-next v3 00/10] Support kernel module function calls from eBPF Kumar Kartikeya Dwivedi
                   ` (4 preceding siblings ...)
  2021-09-15  5:09 ` [PATCH bpf-next v3 05/10] bpf: Enable TCP congestion control kfunc from modules Kumar Kartikeya Dwivedi
@ 2021-09-15  5:09 ` Kumar Kartikeya Dwivedi
  2021-09-15 16:33   ` Alexei Starovoitov
  2021-09-15  5:09 ` [PATCH bpf-next v3 07/10] libbpf: Support kernel module function calls Kumar Kartikeya Dwivedi
                   ` (4 subsequent siblings)
  10 siblings, 1 reply; 20+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2021-09-15  5:09 UTC (permalink / raw)
  To: bpf
  Cc: Kumar Kartikeya Dwivedi, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
	Jesper Dangaard Brouer, Toke Høiland-Jørgensen, netdev

Increase the maximum stack size accessible to BPF program to 768 bytes.
This is done so that gen_loader can use 94 additional fds for kfunc BTFs
that it passes in to fd_array from the remaining space available for the
loader_stack struct to expand.

Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 include/linux/filter.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/filter.h b/include/linux/filter.h
index 4a93c12543ee..b214189ece62 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -82,8 +82,8 @@ struct ctl_table_header;
  */
 #define BPF_SYM_ELF_TYPE	't'
 
-/* BPF program can access up to 512 bytes of stack space. */
-#define MAX_BPF_STACK	512
+/* BPF program can access up to 768 bytes of stack space. */
+#define MAX_BPF_STACK	768
 
 /* Helper macros for filter block array initializers. */
 
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH bpf-next v3 07/10] libbpf: Support kernel module function calls
  2021-09-15  5:09 [PATCH bpf-next v3 00/10] Support kernel module function calls from eBPF Kumar Kartikeya Dwivedi
                   ` (5 preceding siblings ...)
  2021-09-15  5:09 ` [PATCH bpf-next v3 06/10] bpf: Bump MAX_BPF_STACK size to 768 bytes Kumar Kartikeya Dwivedi
@ 2021-09-15  5:09 ` Kumar Kartikeya Dwivedi
  2021-09-15  5:09 ` [PATCH bpf-next v3 08/10] libbpf: Resolve invalid weak kfunc calls with imm = 0, off = 0 Kumar Kartikeya Dwivedi
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2021-09-15  5:09 UTC (permalink / raw)
  To: bpf
  Cc: Kumar Kartikeya Dwivedi, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
	Jesper Dangaard Brouer, Toke Høiland-Jørgensen, netdev

This patch adds libbpf support for kernel module function call support.
The fd_array parameter is used during BPF program load is used to pass
module BTFs referenced by the program. insn->off is set to index into
this array + 1, because insn->off as 0 is reserved for btf_vmlinux. The
kernel subtracts 1 from insn->off when indexing into env->fd_array.

We try to use existing insn->off for a module, since the kernel limits
the maximum distinct module BTFs for kfuncs to 256, and also because
index must never exceed the maximum allowed value that can fit in
insn->off (INT16_MAX). In the future, if kernel interprets signed offset
as unsigned for kfunc calls, this limit can be increased to UINT16_MAX.

Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 tools/lib/bpf/bpf.c             |  1 +
 tools/lib/bpf/libbpf.c          | 56 +++++++++++++++++++++++++++++++--
 tools/lib/bpf/libbpf_internal.h |  1 +
 3 files changed, 55 insertions(+), 3 deletions(-)

diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
index 2401fad090c5..7d1741ceaa32 100644
--- a/tools/lib/bpf/bpf.c
+++ b/tools/lib/bpf/bpf.c
@@ -264,6 +264,7 @@ int libbpf__bpf_prog_load(const struct bpf_prog_load_params *load_attr)
 	attr.line_info_rec_size = load_attr->line_info_rec_size;
 	attr.line_info_cnt = load_attr->line_info_cnt;
 	attr.line_info = ptr_to_u64(load_attr->line_info);
+	attr.fd_array = ptr_to_u64(load_attr->fd_array);
 
 	if (load_attr->name)
 		memcpy(attr.prog_name, load_attr->name,
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 62a43c408d73..accf2586fa76 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -420,6 +420,12 @@ struct extern_desc {
 
 			/* local btf_id of the ksym extern's type. */
 			__u32 type_id;
+			/* offset to be patched in for insn->off,
+			 * this is 0 for btf_vmlinux, and index + 1
+			 * for module BTF, where index is BTF index in
+			 * obj->fd_array
+			 */
+			__s16 offset;
 		} ksym;
 	};
 };
@@ -516,6 +522,10 @@ struct bpf_object {
 	void *priv;
 	bpf_object_clear_priv_t clear_priv;
 
+	int *fd_array;
+	size_t fd_cap_cnt;
+	int nr_fds;
+
 	char path[];
 };
 #define obj_elf_valid(o)	((o)->efile.elf)
@@ -5357,6 +5367,7 @@ bpf_object__relocate_data(struct bpf_object *obj, struct bpf_program *prog)
 			ext = &obj->externs[relo->sym_off];
 			insn[0].src_reg = BPF_PSEUDO_KFUNC_CALL;
 			insn[0].imm = ext->ksym.kernel_btf_id;
+			insn[0].off = ext->ksym.offset;
 			break;
 		case RELO_SUBPROG_ADDR:
 			if (insn[0].src_reg != BPF_PSEUDO_FUNC) {
@@ -6151,6 +6162,7 @@ load_program(struct bpf_program *prog, struct bpf_insn *insns, int insns_cnt,
 	}
 	load_attr.log_level = prog->log_level;
 	load_attr.prog_flags = prog->prog_flags;
+	load_attr.fd_array = prog->obj->fd_array;
 
 	if (prog->obj->gen_loader) {
 		bpf_gen__prog_load(prog->obj->gen_loader, &load_attr,
@@ -6760,9 +6772,44 @@ static int bpf_object__resolve_ksym_func_btf_id(struct bpf_object *obj,
 	}
 
 	if (kern_btf != obj->btf_vmlinux) {
-		pr_warn("extern (func ksym) '%s': function in kernel module is not supported\n",
-			ext->name);
-		return -ENOTSUP;
+		int index = -1;
+
+		if (!obj->fd_array) {
+			obj->fd_array = calloc(8, sizeof(*obj->fd_array));
+			if (!obj->fd_array)
+				return -ENOMEM;
+			obj->fd_cap_cnt = 8;
+		}
+
+		for (int i = 0; i < obj->nr_fds; i++) {
+			if (obj->fd_array[i] == kern_btf_fd) {
+				index = i;
+				break;
+			}
+		}
+
+		if (index == -1) {
+			if (obj->nr_fds == obj->fd_cap_cnt) {
+				ret = libbpf_ensure_mem((void **)&obj->fd_array,
+							&obj->fd_cap_cnt, sizeof(int),
+							obj->fd_cap_cnt + 1);
+				if (ret)
+					return ret;
+			}
+
+			index = obj->nr_fds;
+			obj->fd_array[obj->nr_fds++] = kern_btf_fd;
+		}
+
+		if (index >= INT16_MAX) {
+			/* insn->off is s16 */
+			pr_warn("extern (func ksym) '%s': module btf fd index too big\n",
+				ext->name);
+			return -E2BIG;
+		}
+		ext->ksym.offset = index + 1;
+	} else {
+		ext->ksym.offset = 0;
 	}
 
 	kern_func = btf__type_by_id(kern_btf, kfunc_id);
@@ -6938,6 +6985,9 @@ int bpf_object__load_xattr(struct bpf_object_load_attr *attr)
 			err = bpf_gen__finish(obj->gen_loader);
 	}
 
+	/* clean up fd_array */
+	zfree(&obj->fd_array);
+
 	/* clean up module BTFs */
 	for (i = 0; i < obj->btf_module_cnt; i++) {
 		close(obj->btf_modules[i].fd);
diff --git a/tools/lib/bpf/libbpf_internal.h b/tools/lib/bpf/libbpf_internal.h
index ceb0c98979bc..44b8f381b035 100644
--- a/tools/lib/bpf/libbpf_internal.h
+++ b/tools/lib/bpf/libbpf_internal.h
@@ -291,6 +291,7 @@ struct bpf_prog_load_params {
 	__u32 log_level;
 	char *log_buf;
 	size_t log_buf_sz;
+	int *fd_array;
 };
 
 int libbpf__bpf_prog_load(const struct bpf_prog_load_params *load_attr);
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH bpf-next v3 08/10] libbpf: Resolve invalid weak kfunc calls with imm = 0, off = 0
  2021-09-15  5:09 [PATCH bpf-next v3 00/10] Support kernel module function calls from eBPF Kumar Kartikeya Dwivedi
                   ` (6 preceding siblings ...)
  2021-09-15  5:09 ` [PATCH bpf-next v3 07/10] libbpf: Support kernel module function calls Kumar Kartikeya Dwivedi
@ 2021-09-15  5:09 ` Kumar Kartikeya Dwivedi
  2021-09-15  5:09 ` [PATCH bpf-next v3 09/10] libbpf: Update gen_loader to emit BTF_KIND_FUNC relocations Kumar Kartikeya Dwivedi
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 20+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2021-09-15  5:09 UTC (permalink / raw)
  To: bpf
  Cc: Kumar Kartikeya Dwivedi, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
	Jesper Dangaard Brouer, Toke Høiland-Jørgensen, netdev

Preserve these calls as it allows verifier to succeed in loading the
program if they are determined to be unreachable after dead code
elimination during program load. If not, the verifier will fail at
runtime. This is done for ext->is_weak symbols similar to the case for
variable ksyms.

Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 tools/lib/bpf/libbpf.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index accf2586fa76..50a7c995979a 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -3413,11 +3413,6 @@ static int bpf_object__collect_externs(struct bpf_object *obj)
 				return -ENOTSUP;
 			}
 		} else if (strcmp(sec_name, KSYMS_SEC) == 0) {
-			if (btf_is_func(t) && ext->is_weak) {
-				pr_warn("extern weak function %s is unsupported\n",
-					ext->name);
-				return -ENOTSUP;
-			}
 			ksym_sec = sec;
 			ext->type = EXT_KSYM;
 			skip_mods_and_typedefs(obj->btf, t->type,
@@ -5366,8 +5361,12 @@ bpf_object__relocate_data(struct bpf_object *obj, struct bpf_program *prog)
 		case RELO_EXTERN_FUNC:
 			ext = &obj->externs[relo->sym_off];
 			insn[0].src_reg = BPF_PSEUDO_KFUNC_CALL;
-			insn[0].imm = ext->ksym.kernel_btf_id;
-			insn[0].off = ext->ksym.offset;
+			if (ext->is_set) {
+				insn[0].imm = ext->ksym.kernel_btf_id;
+				insn[0].off = ext->ksym.offset;
+			} else { /* unresolved weak kfunc */
+				insn[0].imm = insn[0].off = 0;
+			}
 			break;
 		case RELO_SUBPROG_ADDR:
 			if (insn[0].src_reg != BPF_PSEUDO_FUNC) {
@@ -6765,8 +6764,10 @@ static int bpf_object__resolve_ksym_func_btf_id(struct bpf_object *obj,
 
 	kfunc_id = find_ksym_btf_id(obj, ext->name, BTF_KIND_FUNC,
 				    &kern_btf, &kern_btf_fd);
-	if (kfunc_id < 0) {
-		pr_warn("extern (func ksym) '%s': not found in kernel BTF\n",
+	if (kfunc_id == -ESRCH && ext->is_weak) {
+		return 0;
+	} else if (kfunc_id < 0) {
+		pr_warn("extern (func ksym) '%s': not found in kernel or module BTFs\n",
 			ext->name);
 		return kfunc_id;
 	}
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH bpf-next v3 09/10] libbpf: Update gen_loader to emit BTF_KIND_FUNC relocations
  2021-09-15  5:09 [PATCH bpf-next v3 00/10] Support kernel module function calls from eBPF Kumar Kartikeya Dwivedi
                   ` (7 preceding siblings ...)
  2021-09-15  5:09 ` [PATCH bpf-next v3 08/10] libbpf: Resolve invalid weak kfunc calls with imm = 0, off = 0 Kumar Kartikeya Dwivedi
@ 2021-09-15  5:09 ` Kumar Kartikeya Dwivedi
  2021-09-15  5:09 ` [PATCH bpf-next v3 10/10] bpf, selftests: Add basic test for module kfunc call Kumar Kartikeya Dwivedi
  2021-09-15 16:04 ` [PATCH bpf-next v3 00/10] Support kernel module function calls from eBPF Andrii Nakryiko
  10 siblings, 0 replies; 20+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2021-09-15  5:09 UTC (permalink / raw)
  To: bpf
  Cc: Kumar Kartikeya Dwivedi, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
	Jesper Dangaard Brouer, Toke Høiland-Jørgensen, netdev

This change updates the BPF syscall loader to relocate BTF_KIND_FUNC
relocations, with support for weak kfunc relocations.

One of the disadvantages of gen_loader is that due to stack size
limitation, BTF fd array size is clamped to a smaller limit than what
the kernel allows. Also, finding an existing BTF fd's slot is not
trivial, because that would require to open all module BTFs and match on
the open fds (like we do for libbpf), so we do the next best thing:
deduplicate slots for the same symbol.

Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 tools/lib/bpf/bpf_gen_internal.h | 12 ++++-
 tools/lib/bpf/gen_loader.c       | 93 ++++++++++++++++++++++++++++----
 tools/lib/bpf/libbpf.c           |  8 +--
 3 files changed, 99 insertions(+), 14 deletions(-)

diff --git a/tools/lib/bpf/bpf_gen_internal.h b/tools/lib/bpf/bpf_gen_internal.h
index 615400391e57..4826adf18d7b 100644
--- a/tools/lib/bpf/bpf_gen_internal.h
+++ b/tools/lib/bpf/bpf_gen_internal.h
@@ -7,8 +7,15 @@ struct ksym_relo_desc {
 	const char *name;
 	int kind;
 	int insn_idx;
+	bool is_weak;
 };
 
+struct kfunc_desc {
+	const char *name;
+	int index;
+};
+
+#define MAX_KFUNC_DESCS 94
 struct bpf_gen {
 	struct gen_loader_opts *opts;
 	void *data_start;
@@ -24,6 +31,8 @@ struct bpf_gen {
 	int relo_cnt;
 	char attach_target[128];
 	int attach_kind;
+	struct kfunc_desc kfunc_descs[MAX_KFUNC_DESCS];
+	__u32 nr_kfuncs;
 };
 
 void bpf_gen__init(struct bpf_gen *gen, int log_level);
@@ -36,6 +45,7 @@ void bpf_gen__prog_load(struct bpf_gen *gen, struct bpf_prog_load_params *load_a
 void bpf_gen__map_update_elem(struct bpf_gen *gen, int map_idx, void *value, __u32 value_size);
 void bpf_gen__map_freeze(struct bpf_gen *gen, int map_idx);
 void bpf_gen__record_attach_target(struct bpf_gen *gen, const char *name, enum bpf_attach_type type);
-void bpf_gen__record_extern(struct bpf_gen *gen, const char *name, int kind, int insn_idx);
+void bpf_gen__record_extern(struct bpf_gen *gen, const char *name, bool is_weak, int kind,
+			    int insn_idx);
 
 #endif
diff --git a/tools/lib/bpf/gen_loader.c b/tools/lib/bpf/gen_loader.c
index 8df718a6b142..5e8c15e36c46 100644
--- a/tools/lib/bpf/gen_loader.c
+++ b/tools/lib/bpf/gen_loader.c
@@ -5,6 +5,7 @@
 #include <string.h>
 #include <errno.h>
 #include <linux/filter.h>
+#include <linux/kernel.h>
 #include "btf.h"
 #include "bpf.h"
 #include "libbpf.h"
@@ -13,6 +14,7 @@
 #include "bpf_gen_internal.h"
 #include "skel_internal.h"
 
+/* MAX_BPF_STACK is 768 bytes, so (64 + 32 + 94 (MAX_KFUNC_DESCS) + 2) * 4 */
 #define MAX_USED_MAPS 64
 #define MAX_USED_PROGS 32
 
@@ -31,6 +33,8 @@ struct loader_stack {
 	__u32 btf_fd;
 	__u32 map_fd[MAX_USED_MAPS];
 	__u32 prog_fd[MAX_USED_PROGS];
+	/* Update insn->off store when reordering kfunc_btf_fd */
+	__u32 kfunc_btf_fd[MAX_KFUNC_DESCS];
 	__u32 inner_map_fd;
 };
 
@@ -506,8 +510,8 @@ static void emit_find_attach_target(struct bpf_gen *gen)
 	 */
 }
 
-void bpf_gen__record_extern(struct bpf_gen *gen, const char *name, int kind,
-			    int insn_idx)
+void bpf_gen__record_extern(struct bpf_gen *gen, const char *name, bool is_weak,
+			    int kind, int insn_idx)
 {
 	struct ksym_relo_desc *relo;
 
@@ -519,14 +523,39 @@ void bpf_gen__record_extern(struct bpf_gen *gen, const char *name, int kind,
 	gen->relos = relo;
 	relo += gen->relo_cnt;
 	relo->name = name;
+	relo->is_weak = is_weak;
 	relo->kind = kind;
 	relo->insn_idx = insn_idx;
 	gen->relo_cnt++;
 }
 
+static struct kfunc_desc *find_kfunc_desc(struct bpf_gen *gen, const char *name)
+{
+	/* Try to reuse BTF fd index for repeating symbol */
+	for (int i = 0; i < gen->nr_kfuncs; i++) {
+		if (!strcmp(gen->kfunc_descs[i].name, name))
+			return &gen->kfunc_descs[i];
+	}
+	return NULL;
+}
+
+static struct kfunc_desc *add_kfunc_desc(struct bpf_gen *gen, const char *name)
+{
+	struct kfunc_desc *kdesc;
+
+	if (gen->nr_kfuncs == ARRAY_SIZE(gen->kfunc_descs))
+		return NULL;
+	kdesc = &gen->kfunc_descs[gen->nr_kfuncs];
+	kdesc->name = name;
+	kdesc->index = gen->nr_kfuncs++;
+	return kdesc;
+}
+
 static void emit_relo(struct bpf_gen *gen, struct ksym_relo_desc *relo, int insns)
 {
 	int name, insn, len = strlen(relo->name) + 1;
+	int off = MAX_USED_MAPS + MAX_USED_PROGS;
+	struct kfunc_desc *kdesc;
 
 	pr_debug("gen: emit_relo: %s at %d\n", relo->name, relo->insn_idx);
 	name = add_data(gen, relo->name, len);
@@ -539,18 +568,64 @@ static void emit_relo(struct bpf_gen *gen, struct ksym_relo_desc *relo, int insn
 	emit(gen, BPF_EMIT_CALL(BPF_FUNC_btf_find_by_name_kind));
 	emit(gen, BPF_MOV64_REG(BPF_REG_7, BPF_REG_0));
 	debug_ret(gen, "find_by_name_kind(%s,%d)", relo->name, relo->kind);
-	emit_check_err(gen);
+	/* if not weak kfunc, emit err check */
+	if (relo->kind != BTF_KIND_FUNC || !relo->is_weak)
+		emit_check_err(gen);
+	insn = insns + sizeof(struct bpf_insn) * relo->insn_idx;
+	emit2(gen, BPF_LD_IMM64_RAW_FULL(BPF_REG_0, BPF_PSEUDO_MAP_IDX_VALUE, 0, 0, 0, insn));
+	/* set a default value */
+	emit(gen, BPF_ST_MEM(BPF_W, BPF_REG_0, offsetof(struct bpf_insn, imm), 0));
+	/* skip success case store if ret < 0 */
+	emit(gen, BPF_JMP_IMM(BPF_JSLT, BPF_REG_7, 0, 1));
 	/* store btf_id into insn[insn_idx].imm */
-	insn = insns + sizeof(struct bpf_insn) * relo->insn_idx +
-		offsetof(struct bpf_insn, imm);
-	emit2(gen, BPF_LD_IMM64_RAW_FULL(BPF_REG_0, BPF_PSEUDO_MAP_IDX_VALUE,
-					 0, 0, 0, insn));
-	emit(gen, BPF_STX_MEM(BPF_W, BPF_REG_0, BPF_REG_7, 0));
+	emit(gen, BPF_STX_MEM(BPF_W, BPF_REG_0, BPF_REG_7, offsetof(struct bpf_insn, imm)));
 	if (relo->kind == BTF_KIND_VAR) {
 		/* store btf_obj_fd into insn[insn_idx + 1].imm */
 		emit(gen, BPF_ALU64_IMM(BPF_RSH, BPF_REG_7, 32));
 		emit(gen, BPF_STX_MEM(BPF_W, BPF_REG_0, BPF_REG_7,
-				      sizeof(struct bpf_insn)));
+				      sizeof(struct bpf_insn) + offsetof(struct bpf_insn, imm)));
+	} else if (relo->kind == BTF_KIND_FUNC) {
+		kdesc = find_kfunc_desc(gen, relo->name);
+		if (!kdesc)
+			kdesc = add_kfunc_desc(gen, relo->name);
+		if (kdesc) {
+			/* store btf_obj_fd in index in kfunc_btf_fd array
+			 * but skip storing fd if ret < 0
+			 */
+			emit(gen, BPF_ST_MEM(BPF_W, BPF_REG_10,
+			     stack_off(kfunc_btf_fd[kdesc->index]), 0));
+			emit(gen, BPF_MOV64_REG(BPF_REG_8, BPF_REG_7));
+			emit(gen, BPF_JMP_IMM(BPF_JSLT, BPF_REG_7, 0, 4));
+			emit(gen, BPF_ALU64_IMM(BPF_RSH, BPF_REG_7, 32));
+			/* if vmlinux BTF, skip store */
+			emit(gen, BPF_JMP_IMM(BPF_JEQ, BPF_REG_7, 0, 1));
+			emit(gen, BPF_STX_MEM(BPF_W, BPF_REG_10, BPF_REG_7,
+			     stack_off(kfunc_btf_fd[kdesc->index])));
+			emit(gen, BPF_MOV64_REG(BPF_REG_7, BPF_REG_8));
+			/* remember BTF obj fd */
+			emit(gen, BPF_ALU64_IMM(BPF_RSH, BPF_REG_8, 32));
+		} else {
+			pr_warn("Out of BTF fd slots (total: %u), skipping for %s\n",
+				gen->nr_kfuncs, relo->name);
+			emit(gen, BPF_MOV64_REG(BPF_REG_1, BPF_REG_7));
+			emit(gen, BPF_ALU64_IMM(BPF_RSH, BPF_REG_1, 32));
+			__emit_sys_close(gen);
+		}
+		/* store index + 1 into insn[insn_idx].off */
+		off = kdesc ? off + kdesc->index + 1 : 0;
+		off = off > INT16_MAX ? 0 : off;
+		/* set a default value */
+		emit(gen, BPF_ST_MEM(BPF_H, BPF_REG_0, offsetof(struct bpf_insn, off), 0));
+		/* skip success case store if ret < 0 */
+		emit(gen, BPF_JMP_IMM(BPF_JSLT, BPF_REG_7, 0, 2));
+		/* skip if vmlinux BTF */
+		emit(gen, BPF_JMP_IMM(BPF_JEQ, BPF_REG_8, 0, 1));
+		/* store offset */
+		emit(gen, BPF_ST_MEM(BPF_H, BPF_REG_0, offsetof(struct bpf_insn, off), off));
+		/* log relocation */
+		emit(gen, BPF_LDX_MEM(BPF_W, BPF_REG_7, BPF_REG_0, offsetof(struct bpf_insn, imm)));
+		emit(gen, BPF_LDX_MEM(BPF_H, BPF_REG_8, BPF_REG_0, offsetof(struct bpf_insn, off)));
+		debug_regs(gen, BPF_REG_7, BPF_REG_8, "sym (%s): imm: %%d, off: %%d", relo->name);
 	}
 }
 
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 50a7c995979a..3ac26dcb60b0 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -6264,12 +6264,12 @@ static int bpf_program__record_externs(struct bpf_program *prog)
 					ext->name);
 				return -ENOTSUP;
 			}
-			bpf_gen__record_extern(obj->gen_loader, ext->name, BTF_KIND_VAR,
-					       relo->insn_idx);
+			bpf_gen__record_extern(obj->gen_loader, ext->name, ext->is_weak,
+					       BTF_KIND_VAR, relo->insn_idx);
 			break;
 		case RELO_EXTERN_FUNC:
-			bpf_gen__record_extern(obj->gen_loader, ext->name, BTF_KIND_FUNC,
-					       relo->insn_idx);
+			bpf_gen__record_extern(obj->gen_loader, ext->name, ext->is_weak,
+					       BTF_KIND_FUNC, relo->insn_idx);
 			break;
 		default:
 			continue;
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH bpf-next v3 10/10] bpf, selftests: Add basic test for module kfunc call
  2021-09-15  5:09 [PATCH bpf-next v3 00/10] Support kernel module function calls from eBPF Kumar Kartikeya Dwivedi
                   ` (8 preceding siblings ...)
  2021-09-15  5:09 ` [PATCH bpf-next v3 09/10] libbpf: Update gen_loader to emit BTF_KIND_FUNC relocations Kumar Kartikeya Dwivedi
@ 2021-09-15  5:09 ` Kumar Kartikeya Dwivedi
  2021-09-15 16:04 ` [PATCH bpf-next v3 00/10] Support kernel module function calls from eBPF Andrii Nakryiko
  10 siblings, 0 replies; 20+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2021-09-15  5:09 UTC (permalink / raw)
  To: bpf
  Cc: Kumar Kartikeya Dwivedi, Alexei Starovoitov, Daniel Borkmann,
	Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
	Jesper Dangaard Brouer, Toke Høiland-Jørgensen, netdev

This also tests support for invalid kfunc calls we added in prior
changes, such that verifier handles invalid call as long as it is
removed by code elimination pass (before fixup_kfunc_call). A separate
test for libbpf is added, which tests failure in loading.

Also adjust verifier selftests which assume 512 byte stack to now assume
768 byte stack.

Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 include/linux/btf.h                           |  2 ++
 kernel/bpf/btf.c                              |  2 ++
 kernel/trace/bpf_trace.c                      |  1 +
 tools/testing/selftests/bpf/Makefile          |  1 +
 .../selftests/bpf/bpf_testmod/bpf_testmod.c   | 23 +++++++++++-
 .../selftests/bpf/prog_tests/ksyms_module.c   | 13 ++++---
 .../bpf/prog_tests/ksyms_module_libbpf.c      | 18 ++++++++++
 .../selftests/bpf/progs/test_ksyms_module.c   |  9 +++++
 .../bpf/progs/test_ksyms_module_libbpf.c      | 35 +++++++++++++++++++
 tools/testing/selftests/bpf/verifier/calls.c  | 22 ++++++------
 .../selftests/bpf/verifier/raw_stack.c        |  4 +--
 .../selftests/bpf/verifier/stack_ptr.c        |  6 ++--
 .../testing/selftests/bpf/verifier/var_off.c  |  4 +--
 13 files changed, 116 insertions(+), 24 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/ksyms_module_libbpf.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_ksyms_module_libbpf.c

diff --git a/include/linux/btf.h b/include/linux/btf.h
index c7b6382123e1..585c66aa0529 100644
--- a/include/linux/btf.h
+++ b/include/linux/btf.h
@@ -271,7 +271,9 @@ static inline void unregister_kfunc_btf_id_set(struct kfunc_btf_id_list *l,
 					 THIS_MODULE }
 
 extern struct kfunc_btf_id_list bpf_tcp_ca_kfunc_list;
+extern struct kfunc_btf_id_list raw_tp_kfunc_list;
 
 DECLARE_CHECK_KFUNC_CALLBACK(bpf_tcp_ca);
+DECLARE_CHECK_KFUNC_CALLBACK(raw_tp);
 
 #endif
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 671b4f713a51..c6632894ed05 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -6397,3 +6397,5 @@ EXPORT_SYMBOL_GPL(unregister_kfunc_btf_id_set);
 
 DEFINE_KFUNC_BTF_ID_LIST(bpf_tcp_ca_kfunc_list);
 DEFINE_CHECK_KFUNC_CALLBACK(bpf_tcp_ca, bpf_tcp_ca_kfunc_list);
+DEFINE_KFUNC_BTF_ID_LIST(raw_tp_kfunc_list);
+DEFINE_CHECK_KFUNC_CALLBACK(raw_tp, raw_tp_kfunc_list);
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 067e88c3d2ee..54cba3391f35 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -1629,6 +1629,7 @@ int __weak bpf_prog_test_run_tracing(struct bpf_prog *prog,
 const struct bpf_verifier_ops raw_tracepoint_verifier_ops = {
 	.get_func_proto  = raw_tp_prog_func_proto,
 	.is_valid_access = raw_tp_prog_is_valid_access,
+	.check_kfunc_call = __bpf_check_raw_tp_kfunc_call,
 };
 
 const struct bpf_prog_ops raw_tracepoint_prog_ops = {
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 1a4d30ff3275..064eef69e96a 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -174,6 +174,7 @@ $(OUTPUT)/bpf_testmod.ko: $(VMLINUX_BTF) $(wildcard bpf_testmod/Makefile bpf_tes
 	$(Q)$(RM) bpf_testmod/bpf_testmod.ko # force re-compilation
 	$(Q)$(MAKE) $(submake_extras) -C bpf_testmod
 	$(Q)cp bpf_testmod/bpf_testmod.ko $@
+	$(Q)$(RESOLVE_BTFIDS) -s ../../../../vmlinux bpf_testmod.ko
 
 $(OUTPUT)/test_stub.o: test_stub.c $(BPFOBJ)
 	$(call msg,CC,,$@)
diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
index 50fc5561110a..5b365a7b3f93 100644
--- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
+++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
@@ -1,6 +1,8 @@
 // SPDX-License-Identifier: GPL-2.0
 /* Copyright (c) 2020 Facebook */
 #include <linux/error-injection.h>
+#include <linux/btf.h>
+#include <linux/btf_ids.h>
 #include <linux/init.h>
 #include <linux/module.h>
 #include <linux/percpu-defs.h>
@@ -13,6 +15,12 @@
 
 DEFINE_PER_CPU(int, bpf_testmod_ksym_percpu) = 123;
 
+noinline void
+bpf_testmod_test_mod_kfunc(int i)
+{
+	pr_info("mod kfunc i=%d\n", i);
+}
+
 noinline int bpf_testmod_loop_test(int n)
 {
 	int i, sum = 0;
@@ -71,13 +79,26 @@ static struct bin_attribute bin_attr_bpf_testmod_file __ro_after_init = {
 	.write = bpf_testmod_test_write,
 };
 
+BTF_SET_START(bpf_testmod_kfunc_ids)
+BTF_ID(func, bpf_testmod_test_mod_kfunc)
+BTF_SET_END(bpf_testmod_kfunc_ids)
+
+static DEFINE_KFUNC_BTF_ID_SET(&bpf_testmod_kfunc_ids, bpf_testmod_kfunc_btf_set);
+
 static int bpf_testmod_init(void)
 {
-	return sysfs_create_bin_file(kernel_kobj, &bin_attr_bpf_testmod_file);
+	int ret;
+
+	ret = sysfs_create_bin_file(kernel_kobj, &bin_attr_bpf_testmod_file);
+	if (ret)
+		return ret;
+	register_kfunc_btf_id_set(&raw_tp_kfunc_list, &bpf_testmod_kfunc_btf_set);
+	return 0;
 }
 
 static void bpf_testmod_exit(void)
 {
+	unregister_kfunc_btf_id_set(&raw_tp_kfunc_list, &bpf_testmod_kfunc_btf_set);
 	return sysfs_remove_bin_file(kernel_kobj, &bin_attr_bpf_testmod_file);
 }
 
diff --git a/tools/testing/selftests/bpf/prog_tests/ksyms_module.c b/tools/testing/selftests/bpf/prog_tests/ksyms_module.c
index 2cd5cded543f..7643141ec67b 100644
--- a/tools/testing/selftests/bpf/prog_tests/ksyms_module.c
+++ b/tools/testing/selftests/bpf/prog_tests/ksyms_module.c
@@ -6,19 +6,22 @@
 #include <bpf/btf.h>
 #include "test_ksyms_module.lskel.h"
 
-static int duration;
-
 void test_ksyms_module(void)
 {
-	struct test_ksyms_module* skel;
+	struct test_ksyms_module *skel;
 	int err;
 
+	if (!env.has_testmod) {
+		test__skip();
+		return;
+	}
+
 	skel = test_ksyms_module__open_and_load();
-	if (CHECK(!skel, "skel_open", "failed to open skeleton\n"))
+	if (!ASSERT_OK_PTR(skel, "test_ksyms_module__open_and_load"))
 		return;
 
 	err = test_ksyms_module__attach(skel);
-	if (CHECK(err, "skel_attach", "skeleton attach failed: %d\n", err))
+	if (!ASSERT_OK(err, "test_ksyms_module__attach"))
 		goto cleanup;
 
 	usleep(1);
diff --git a/tools/testing/selftests/bpf/prog_tests/ksyms_module_libbpf.c b/tools/testing/selftests/bpf/prog_tests/ksyms_module_libbpf.c
new file mode 100644
index 000000000000..61fa2a0e156e
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/ksyms_module_libbpf.c
@@ -0,0 +1,18 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <test_progs.h>
+#include "test_ksyms_module_libbpf.skel.h"
+
+void test_ksyms_module_libbpf(void)
+{
+	struct test_ksyms_module_libbpf *skel;
+
+	if (!env.has_testmod) {
+		test__skip();
+		return;
+	}
+
+	skel = test_ksyms_module_libbpf__open_and_load();
+	if (!ASSERT_EQ(skel, NULL, "test_ksyms_module__open_and_load"))
+		test_ksyms_module_libbpf__destroy(skel);
+}
diff --git a/tools/testing/selftests/bpf/progs/test_ksyms_module.c b/tools/testing/selftests/bpf/progs/test_ksyms_module.c
index d6a0b3086b90..d3fff47791fc 100644
--- a/tools/testing/selftests/bpf/progs/test_ksyms_module.c
+++ b/tools/testing/selftests/bpf/progs/test_ksyms_module.c
@@ -6,8 +6,11 @@
 #include <bpf/bpf_helpers.h>
 
 extern const int bpf_testmod_ksym_percpu __ksym;
+extern void bpf_testmod_test_mod_kfunc(int i) __ksym;
+extern void bpf_testmod_invalid_mod_kfunc(void) __ksym __weak;
 
 int out_mod_ksym_global = 0;
+const volatile int x = 0;
 bool triggered = false;
 
 SEC("raw_tp/sys_enter")
@@ -16,6 +19,12 @@ int handler(const void *ctx)
 	int *val;
 	__u32 cpu;
 
+	/* This should be preserved by clang, but DCE'd by verifier, and still
+	 * allow loading the raw_tp prog
+	 */
+	if (x)
+		bpf_testmod_invalid_mod_kfunc();
+	bpf_testmod_test_mod_kfunc(42);
 	val = (int *)bpf_this_cpu_ptr(&bpf_testmod_ksym_percpu);
 	out_mod_ksym_global = *val;
 	triggered = true;
diff --git a/tools/testing/selftests/bpf/progs/test_ksyms_module_libbpf.c b/tools/testing/selftests/bpf/progs/test_ksyms_module_libbpf.c
new file mode 100644
index 000000000000..52162858d25d
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_ksyms_module_libbpf.c
@@ -0,0 +1,35 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include "vmlinux.h"
+
+#include <bpf/bpf_helpers.h>
+
+extern void bpf_testmod_test_mod_kfunc(int i) __ksym;
+extern void bpf_testmod_invalid_mod_kfunc(void) __ksym __weak;
+
+const volatile int x = 0;
+
+SEC("raw_tp/sys_enter")
+int handler_pass(const void *ctx)
+{
+	/* This should be preserved by clang, but DCE'd by verifier, and still
+	 * allow loading the raw_tp prog
+	 */
+	if (x)
+		bpf_testmod_invalid_mod_kfunc();
+	bpf_testmod_test_mod_kfunc(42);
+	return 0;
+}
+
+SEC("raw_tp/sys_enter")
+int handler_fail(const void *ctx)
+{
+	/* This call should be preserved by clang, but fail verification.
+	 */
+	if (!x)
+		bpf_testmod_invalid_mod_kfunc();
+	bpf_testmod_test_mod_kfunc(42);
+	return 0;
+}
+
+char LICENSE[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/verifier/calls.c b/tools/testing/selftests/bpf/verifier/calls.c
index 336a749673d1..03467053996c 100644
--- a/tools/testing/selftests/bpf/verifier/calls.c
+++ b/tools/testing/selftests/bpf/verifier/calls.c
@@ -750,12 +750,12 @@
 	"calls: stack overflow using two frames (pre-call access)",
 	.insns = {
 	/* prog 1 */
-	BPF_ST_MEM(BPF_B, BPF_REG_10, -300, 0),
+	BPF_ST_MEM(BPF_B, BPF_REG_10, -400, 0),
 	BPF_RAW_INSN(BPF_JMP|BPF_CALL, 0, 1, 0, 1),
 	BPF_EXIT_INSN(),
 
 	/* prog 2 */
-	BPF_ST_MEM(BPF_B, BPF_REG_10, -300, 0),
+	BPF_ST_MEM(BPF_B, BPF_REG_10, -400, 0),
 	BPF_MOV64_IMM(BPF_REG_0, 0),
 	BPF_EXIT_INSN(),
 	},
@@ -768,11 +768,11 @@
 	.insns = {
 	/* prog 1 */
 	BPF_RAW_INSN(BPF_JMP|BPF_CALL, 0, 1, 0, 2),
-	BPF_ST_MEM(BPF_B, BPF_REG_10, -300, 0),
+	BPF_ST_MEM(BPF_B, BPF_REG_10, -400, 0),
 	BPF_EXIT_INSN(),
 
 	/* prog 2 */
-	BPF_ST_MEM(BPF_B, BPF_REG_10, -300, 0),
+	BPF_ST_MEM(BPF_B, BPF_REG_10, -400, 0),
 	BPF_MOV64_IMM(BPF_REG_0, 0),
 	BPF_EXIT_INSN(),
 	},
@@ -846,12 +846,12 @@
 	/* B */
 	BPF_JMP_IMM(BPF_JGT, BPF_REG_1, 2, 1),
 	BPF_RAW_INSN(BPF_JMP|BPF_CALL, 0, 1, 0, -6), /* call A */
-	BPF_ST_MEM(BPF_B, BPF_REG_10, -256, 0),
+	BPF_ST_MEM(BPF_B, BPF_REG_10, -512, 0),
 	BPF_EXIT_INSN(),
 	},
 	.prog_type = BPF_PROG_TYPE_XDP,
-	/* stack_main=64, stack_A=224, stack_B=256
-	 * and max(main+A, main+A+B) > 512
+	/* stack_main=64, stack_A=224, stack_B=512
+	 * and max(main+A, main+A+B) > 768
 	 */
 	.errstr = "combined stack",
 	.result = REJECT,
@@ -865,14 +865,14 @@
 	 * }
 	 * void func1(int alloc_or_recurse) {
 	 *   if (alloc_or_recurse) {
-	 *     frame_pointer[-300] = 1;
+	 *     frame_pointer[-400] = 1;
 	 *   } else {
 	 *     func2(alloc_or_recurse);
 	 *   }
 	 * }
 	 * void func2(int alloc_or_recurse) {
 	 *   if (alloc_or_recurse) {
-	 *     frame_pointer[-300] = 1;
+	 *     frame_pointer[-400] = 1;
 	 *   }
 	 * }
 	 */
@@ -888,13 +888,13 @@
 	BPF_EXIT_INSN(),
 	/* A */
 	BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, 0, 2),
-	BPF_ST_MEM(BPF_B, BPF_REG_10, -300, 0),
+	BPF_ST_MEM(BPF_B, BPF_REG_10, -400, 0),
 	BPF_EXIT_INSN(),
 	BPF_RAW_INSN(BPF_JMP|BPF_CALL, 0, 1, 0, 1), /* call B */
 	BPF_EXIT_INSN(),
 	/* B */
 	BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, 0, 1),
-	BPF_ST_MEM(BPF_B, BPF_REG_10, -300, 0),
+	BPF_ST_MEM(BPF_B, BPF_REG_10, -400, 0),
 	BPF_EXIT_INSN(),
 	},
 	.prog_type = BPF_PROG_TYPE_XDP,
diff --git a/tools/testing/selftests/bpf/verifier/raw_stack.c b/tools/testing/selftests/bpf/verifier/raw_stack.c
index cc8e8c3cdc03..238dedb3aa47 100644
--- a/tools/testing/selftests/bpf/verifier/raw_stack.c
+++ b/tools/testing/selftests/bpf/verifier/raw_stack.c
@@ -197,7 +197,7 @@
 	.insns = {
 	BPF_MOV64_IMM(BPF_REG_2, 4),
 	BPF_ALU64_REG(BPF_MOV, BPF_REG_6, BPF_REG_10),
-	BPF_ALU64_IMM(BPF_ADD, BPF_REG_6, -513),
+	BPF_ALU64_IMM(BPF_ADD, BPF_REG_6, -769),
 	BPF_MOV64_REG(BPF_REG_3, BPF_REG_6),
 	BPF_MOV64_IMM(BPF_REG_4, 8),
 	BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0, BPF_FUNC_skb_load_bytes),
@@ -205,7 +205,7 @@
 	BPF_EXIT_INSN(),
 	},
 	.result = REJECT,
-	.errstr = "invalid indirect access to stack R3 off=-513 size=8",
+	.errstr = "invalid indirect access to stack R3 off=-769 size=8",
 	.prog_type = BPF_PROG_TYPE_SCHED_CLS,
 },
 {
diff --git a/tools/testing/selftests/bpf/verifier/stack_ptr.c b/tools/testing/selftests/bpf/verifier/stack_ptr.c
index 8ab94d65f3d5..566d79299ccd 100644
--- a/tools/testing/selftests/bpf/verifier/stack_ptr.c
+++ b/tools/testing/selftests/bpf/verifier/stack_ptr.c
@@ -165,7 +165,7 @@
 	"PTR_TO_STACK check low 2",
 	.insns = {
 	BPF_MOV64_REG(BPF_REG_1, BPF_REG_10),
-	BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, -513),
+	BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, -769),
 	BPF_ST_MEM(BPF_B, BPF_REG_1, 1, 42),
 	BPF_LDX_MEM(BPF_B, BPF_REG_0, BPF_REG_1, 1),
 	BPF_EXIT_INSN(),
@@ -179,13 +179,13 @@
 	"PTR_TO_STACK check low 3",
 	.insns = {
 	BPF_MOV64_REG(BPF_REG_1, BPF_REG_10),
-	BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, -513),
+	BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, -769),
 	BPF_ST_MEM(BPF_B, BPF_REG_1, 0, 42),
 	BPF_LDX_MEM(BPF_B, BPF_REG_0, BPF_REG_1, 0),
 	BPF_EXIT_INSN(),
 	},
 	.errstr_unpriv = "R1 stack pointer arithmetic goes out of range",
-	.errstr = "invalid write to stack R1 off=-513 size=1",
+	.errstr = "invalid write to stack R1 off=-769 size=1",
 	.result = REJECT,
 },
 {
diff --git a/tools/testing/selftests/bpf/verifier/var_off.c b/tools/testing/selftests/bpf/verifier/var_off.c
index eab1f7f56e2f..407bdee522a6 100644
--- a/tools/testing/selftests/bpf/verifier/var_off.c
+++ b/tools/testing/selftests/bpf/verifier/var_off.c
@@ -196,8 +196,8 @@
 	BPF_LDX_MEM(BPF_W, BPF_REG_2, BPF_REG_1, 0),
 	/* Make it small and 4-byte aligned */
 	BPF_ALU64_IMM(BPF_AND, BPF_REG_2, 4),
-	BPF_ALU64_IMM(BPF_SUB, BPF_REG_2, 516),
-	/* add it to fp.  We now have either fp-516 or fp-512, but
+	BPF_ALU64_IMM(BPF_SUB, BPF_REG_2, 772),
+	/* add it to fp.  We now have either fp-772 or fp-768, but
 	 * we don't know which
 	 */
 	BPF_ALU64_REG(BPF_ADD, BPF_REG_2, BPF_REG_10),
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH bpf-next v3 00/10] Support kernel module function calls from eBPF
  2021-09-15  5:09 [PATCH bpf-next v3 00/10] Support kernel module function calls from eBPF Kumar Kartikeya Dwivedi
                   ` (9 preceding siblings ...)
  2021-09-15  5:09 ` [PATCH bpf-next v3 10/10] bpf, selftests: Add basic test for module kfunc call Kumar Kartikeya Dwivedi
@ 2021-09-15 16:04 ` Andrii Nakryiko
  2021-09-15 18:03   ` Kumar Kartikeya Dwivedi
  10 siblings, 1 reply; 20+ messages in thread
From: Andrii Nakryiko @ 2021-09-15 16:04 UTC (permalink / raw)
  To: Kumar Kartikeya Dwivedi
  Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song,
	Jesper Dangaard Brouer, Toke Høiland-Jørgensen,
	Networking

On Tue, Sep 14, 2021 at 10:09 PM Kumar Kartikeya Dwivedi
<memxor@gmail.com> wrote:
>
> This set enables kernel module function calls, and also modifies verifier logic
> to permit invalid kernel function calls as long as they are pruned as part of
> dead code elimination. This is done to provide better runtime portability for
> BPF objects, which can conditionally disable parts of code that are pruned later
> by the verifier (e.g. const volatile vars, kconfig options). libbpf
> modifications are made along with kernel changes to support module function
> calls. The set includes gen_loader support for emitting kfunc relocations.
>
> It also converts TCP congestion control objects to use the module kfunc support
> instead of relying on IS_BUILTIN ifdef.
>
> Changelog:
> ----------
> v2 -> v3:
> v2: https://lore.kernel.org/bpf/20210914123750.460750-1-memxor@gmail.com
>
>  * Fix issues pointed out by Kernel Test Robot
>  * Fix find_kfunc_desc to also take offset into consideration when comparing

See [0]:


  [  444.075332] mod kfunc i=42
  [  444.075383] mod kfunc i=42
  [  444.075522] mod kfunc i=42
  [  444.075578] mod kfunc i=42
  [  444.075631] mod kfunc i=42
  [  444.075683] mod kfunc i=42
  [  444.075735] mod kfunc i=42
  [  444.0
This step has been truncated due to its large size. Download the full
logs from the
menu once the workflow run has completed.

  [0] https://github.com/kernel-patches/bpf/runs/3606513281?check_suite_focus=true

>
> RFC v1 -> v2
> v1: https://lore.kernel.org/bpf/20210830173424.1385796-1-memxor@gmail.com
>
>  * Address comments from Alexei
>    * Reuse fd_array instead of introducing kfunc_btf_fds array
>    * Take btf and module reference as needed, instead of preloading
>    * Add BTF_KIND_FUNC relocation support to gen_loader infrastructure
>  * Address comments from Andrii
>    * Drop hashmap in libbpf for finding index of existing BTF in fd_array
>    * Preserve invalid kfunc calls only when the symbol is weak
>  * Adjust verifier selftests
>
> Kumar Kartikeya Dwivedi (10):
>   bpf: Introduce BPF support for kernel module function calls
>   bpf: Be conservative while processing invalid kfunc calls
>   bpf: btf: Introduce helpers for dynamic BTF set registration
>   tools: Allow specifying base BTF file in resolve_btfids
>   bpf: Enable TCP congestion control kfunc from modules
>   bpf: Bump MAX_BPF_STACK size to 768 bytes
>   libbpf: Support kernel module function calls
>   libbpf: Resolve invalid weak kfunc calls with imm = 0, off = 0
>   libbpf: Update gen_loader to emit BTF_KIND_FUNC relocations
>   bpf, selftests: Add basic test for module kfunc call
>
>  include/linux/bpf.h                           |   8 +-
>  include/linux/bpf_verifier.h                  |   2 +
>  include/linux/bpfptr.h                        |   1 +
>  include/linux/btf.h                           |  38 +++
>  include/linux/filter.h                        |   4 +-
>  kernel/bpf/btf.c                              |  56 +++++
>  kernel/bpf/core.c                             |   4 +
>  kernel/bpf/verifier.c                         | 217 +++++++++++++++---
>  kernel/trace/bpf_trace.c                      |   1 +
>  net/bpf/test_run.c                            |   2 +-
>  net/ipv4/bpf_tcp_ca.c                         |  36 +--
>  net/ipv4/tcp_bbr.c                            |  28 ++-
>  net/ipv4/tcp_cubic.c                          |  26 ++-
>  net/ipv4/tcp_dctcp.c                          |  26 ++-
>  scripts/Makefile.modfinal                     |   1 +
>  tools/bpf/resolve_btfids/main.c               |  19 +-
>  tools/lib/bpf/bpf.c                           |   1 +
>  tools/lib/bpf/bpf_gen_internal.h              |  12 +-
>  tools/lib/bpf/gen_loader.c                    |  93 +++++++-
>  tools/lib/bpf/libbpf.c                        |  81 +++++--
>  tools/lib/bpf/libbpf_internal.h               |   1 +
>  tools/testing/selftests/bpf/Makefile          |   1 +
>  .../selftests/bpf/bpf_testmod/bpf_testmod.c   |  23 +-
>  .../selftests/bpf/prog_tests/ksyms_module.c   |  13 +-
>  .../bpf/prog_tests/ksyms_module_libbpf.c      |  18 ++
>  .../selftests/bpf/progs/test_ksyms_module.c   |   9 +
>  .../bpf/progs/test_ksyms_module_libbpf.c      |  35 +++
>  tools/testing/selftests/bpf/verifier/calls.c  |  22 +-
>  .../selftests/bpf/verifier/raw_stack.c        |   4 +-
>  .../selftests/bpf/verifier/stack_ptr.c        |   6 +-
>  .../testing/selftests/bpf/verifier/var_off.c  |   4 +-
>  31 files changed, 673 insertions(+), 119 deletions(-)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/ksyms_module_libbpf.c
>  create mode 100644 tools/testing/selftests/bpf/progs/test_ksyms_module_libbpf.c
>
> --
> 2.33.0
>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH bpf-next v3 03/10] bpf: btf: Introduce helpers for dynamic BTF set registration
  2021-09-15  5:09 ` [PATCH bpf-next v3 03/10] bpf: btf: Introduce helpers for dynamic BTF set registration Kumar Kartikeya Dwivedi
@ 2021-09-15 16:18   ` Alexei Starovoitov
  2021-09-15 18:06     ` Kumar Kartikeya Dwivedi
  0 siblings, 1 reply; 20+ messages in thread
From: Alexei Starovoitov @ 2021-09-15 16:18 UTC (permalink / raw)
  To: Kumar Kartikeya Dwivedi
  Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song,
	Jesper Dangaard Brouer, Toke Høiland-Jørgensen, netdev

On Wed, Sep 15, 2021 at 10:39:36AM +0530, Kumar Kartikeya Dwivedi wrote:
> This adds helpers for registering btf_id_set from modules and the
> check_kfunc_call callback that can be used to look them up.
> 
> With in kernel sets, the way this is supposed to work is, in kernel
> callback looks up within the in-kernel kfunc whitelist, and then defers
> to the dynamic BTF set lookup if it doesn't find the BTF id. If there is
> no in-kernel BTF id set, this callback can be used directly.
> 
> Also fix includes for btf.h and bpfptr.h so that they can included in
> isolation. This is in preparation for their usage in tcp_bbr, tcp_cubic
> and tcp_dctcp modules in the next patch.
> 
> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> ---
>  include/linux/bpfptr.h |  1 +
>  include/linux/btf.h    | 32 ++++++++++++++++++++++++++
>  kernel/bpf/btf.c       | 51 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 84 insertions(+)
> 
> diff --git a/include/linux/bpfptr.h b/include/linux/bpfptr.h
> index 546e27fc6d46..46e1757d06a3 100644
> --- a/include/linux/bpfptr.h
> +++ b/include/linux/bpfptr.h
> @@ -3,6 +3,7 @@
>  #ifndef _LINUX_BPFPTR_H
>  #define _LINUX_BPFPTR_H
>  
> +#include <linux/mm.h>

Could you explain what this is for?

>  #include <linux/sockptr.h>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH bpf-next v3 06/10] bpf: Bump MAX_BPF_STACK size to 768 bytes
  2021-09-15  5:09 ` [PATCH bpf-next v3 06/10] bpf: Bump MAX_BPF_STACK size to 768 bytes Kumar Kartikeya Dwivedi
@ 2021-09-15 16:33   ` Alexei Starovoitov
  2021-09-15 17:57     ` Kumar Kartikeya Dwivedi
  0 siblings, 1 reply; 20+ messages in thread
From: Alexei Starovoitov @ 2021-09-15 16:33 UTC (permalink / raw)
  To: Kumar Kartikeya Dwivedi
  Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song,
	Jesper Dangaard Brouer, Toke Høiland-Jørgensen, netdev

On Wed, Sep 15, 2021 at 10:39:39AM +0530, Kumar Kartikeya Dwivedi wrote:
> Increase the maximum stack size accessible to BPF program to 768 bytes.
> This is done so that gen_loader can use 94 additional fds for kfunc BTFs
> that it passes in to fd_array from the remaining space available for the
> loader_stack struct to expand.
> 
> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> ---
>  include/linux/filter.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/filter.h b/include/linux/filter.h
> index 4a93c12543ee..b214189ece62 100644
> --- a/include/linux/filter.h
> +++ b/include/linux/filter.h
> @@ -82,8 +82,8 @@ struct ctl_table_header;
>   */
>  #define BPF_SYM_ELF_TYPE	't'
>  
> -/* BPF program can access up to 512 bytes of stack space. */
> -#define MAX_BPF_STACK	512
> +/* BPF program can access up to 768 bytes of stack space. */
> +#define MAX_BPF_STACK	768

Yikes.
I guess you meant as RFC, right? You didn't really propose
to increase prog stack size just for that, right?

In the later patch:
+/* MAX_BPF_STACK is 768 bytes, so (64 + 32 + 94 (MAX_KFUNC_DESCS) + 2) * 4 */
 #define MAX_USED_MAPS 64
 #define MAX_USED_PROGS 32

@@ -31,6 +33,8 @@ struct loader_stack {
        __u32 btf_fd;
        __u32 map_fd[MAX_USED_MAPS];
        __u32 prog_fd[MAX_USED_PROGS];
+       /* Update insn->off store when reordering kfunc_btf_fd */
+       __u32 kfunc_btf_fd[MAX_KFUNC_DESCS];
        __u32 inner_map_fd;
};

There are few other ways to do that.
For example:
A: rename map_fd[] into fds[] and store both map and btf FDs in there.
B: move map and btf FDs into data instead of stack.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH bpf-next v3 06/10] bpf: Bump MAX_BPF_STACK size to 768 bytes
  2021-09-15 16:33   ` Alexei Starovoitov
@ 2021-09-15 17:57     ` Kumar Kartikeya Dwivedi
  2021-09-16  2:56       ` Alexei Starovoitov
  0 siblings, 1 reply; 20+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2021-09-15 17:57 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song,
	Jesper Dangaard Brouer, Toke Høiland-Jørgensen, netdev

On Wed, Sep 15, 2021 at 10:03:53PM IST, Alexei Starovoitov wrote:
> On Wed, Sep 15, 2021 at 10:39:39AM +0530, Kumar Kartikeya Dwivedi wrote:
> > Increase the maximum stack size accessible to BPF program to 768 bytes.
> > This is done so that gen_loader can use 94 additional fds for kfunc BTFs
> > that it passes in to fd_array from the remaining space available for the
> > loader_stack struct to expand.
> >
> > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> > ---
> >  include/linux/filter.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/linux/filter.h b/include/linux/filter.h
> > index 4a93c12543ee..b214189ece62 100644
> > --- a/include/linux/filter.h
> > +++ b/include/linux/filter.h
> > @@ -82,8 +82,8 @@ struct ctl_table_header;
> >   */
> >  #define BPF_SYM_ELF_TYPE	't'
> >
> > -/* BPF program can access up to 512 bytes of stack space. */
> > -#define MAX_BPF_STACK	512
> > +/* BPF program can access up to 768 bytes of stack space. */
> > +#define MAX_BPF_STACK	768
>
> Yikes.
> I guess you meant as RFC, right? You didn't really propose
> to increase prog stack size just for that, right?
>

Yes, and right, it's ugly :/.

> In the later patch:
> +/* MAX_BPF_STACK is 768 bytes, so (64 + 32 + 94 (MAX_KFUNC_DESCS) + 2) * 4 */
>  #define MAX_USED_MAPS 64
>  #define MAX_USED_PROGS 32
>
> @@ -31,6 +33,8 @@ struct loader_stack {
>         __u32 btf_fd;
>         __u32 map_fd[MAX_USED_MAPS];
>         __u32 prog_fd[MAX_USED_PROGS];
> +       /* Update insn->off store when reordering kfunc_btf_fd */
> +       __u32 kfunc_btf_fd[MAX_KFUNC_DESCS];
>         __u32 inner_map_fd;
> };
>
> There are few other ways to do that.
> For example:
> A: rename map_fd[] into fds[] and store both map and btf FDs in there.
> B: move map and btf FDs into data instead of stack.

Both are great suggestions, I thought about A but not B, but it will be better
(even though it requires more changes, we can do full 256 BTF fds using B).
Thanks!

--
Kartikeya

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH bpf-next v3 00/10] Support kernel module function calls from eBPF
  2021-09-15 16:04 ` [PATCH bpf-next v3 00/10] Support kernel module function calls from eBPF Andrii Nakryiko
@ 2021-09-15 18:03   ` Kumar Kartikeya Dwivedi
  2021-09-15 18:05     ` Andrii Nakryiko
  0 siblings, 1 reply; 20+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2021-09-15 18:03 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song,
	Jesper Dangaard Brouer, Toke Høiland-Jørgensen,
	Networking

On Wed, Sep 15, 2021 at 09:34:21PM IST, Andrii Nakryiko wrote:
> On Tue, Sep 14, 2021 at 10:09 PM Kumar Kartikeya Dwivedi
> <memxor@gmail.com> wrote:
> >
> > This set enables kernel module function calls, and also modifies verifier logic
> > to permit invalid kernel function calls as long as they are pruned as part of
> > dead code elimination. This is done to provide better runtime portability for
> > BPF objects, which can conditionally disable parts of code that are pruned later
> > by the verifier (e.g. const volatile vars, kconfig options). libbpf
> > modifications are made along with kernel changes to support module function
> > calls. The set includes gen_loader support for emitting kfunc relocations.
> >
> > It also converts TCP congestion control objects to use the module kfunc support
> > instead of relying on IS_BUILTIN ifdef.
> >
> > Changelog:
> > ----------
> > v2 -> v3:
> > v2: https://lore.kernel.org/bpf/20210914123750.460750-1-memxor@gmail.com
> >
> >  * Fix issues pointed out by Kernel Test Robot
> >  * Fix find_kfunc_desc to also take offset into consideration when comparing
>
> See [0]:
>
>
>   [  444.075332] mod kfunc i=42
>   [  444.075383] mod kfunc i=42
>   [  444.075522] mod kfunc i=42
>   [  444.075578] mod kfunc i=42
>   [  444.075631] mod kfunc i=42
>   [  444.075683] mod kfunc i=42
>   [  444.075735] mod kfunc i=42
>   [  444.0
> This step has been truncated due to its large size. Download the full
> logs from the
> menu once the workflow run has completed.
>
>   [0] https://github.com/kernel-patches/bpf/runs/3606513281?check_suite_focus=true
>

I'll change it to not use printk in the next spin (maybe just set a variable).
This probably blew up because of parallel test_progs stuff, since it would be
called for each sys_enter as long the raw_tp prog is attached.

--
Kartikeya

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH bpf-next v3 00/10] Support kernel module function calls from eBPF
  2021-09-15 18:03   ` Kumar Kartikeya Dwivedi
@ 2021-09-15 18:05     ` Andrii Nakryiko
  0 siblings, 0 replies; 20+ messages in thread
From: Andrii Nakryiko @ 2021-09-15 18:05 UTC (permalink / raw)
  To: Kumar Kartikeya Dwivedi
  Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song,
	Jesper Dangaard Brouer, Toke Høiland-Jørgensen,
	Networking

On Wed, Sep 15, 2021 at 11:03 AM Kumar Kartikeya Dwivedi
<memxor@gmail.com> wrote:
>
> On Wed, Sep 15, 2021 at 09:34:21PM IST, Andrii Nakryiko wrote:
> > On Tue, Sep 14, 2021 at 10:09 PM Kumar Kartikeya Dwivedi
> > <memxor@gmail.com> wrote:
> > >
> > > This set enables kernel module function calls, and also modifies verifier logic
> > > to permit invalid kernel function calls as long as they are pruned as part of
> > > dead code elimination. This is done to provide better runtime portability for
> > > BPF objects, which can conditionally disable parts of code that are pruned later
> > > by the verifier (e.g. const volatile vars, kconfig options). libbpf
> > > modifications are made along with kernel changes to support module function
> > > calls. The set includes gen_loader support for emitting kfunc relocations.
> > >
> > > It also converts TCP congestion control objects to use the module kfunc support
> > > instead of relying on IS_BUILTIN ifdef.
> > >
> > > Changelog:
> > > ----------
> > > v2 -> v3:
> > > v2: https://lore.kernel.org/bpf/20210914123750.460750-1-memxor@gmail.com
> > >
> > >  * Fix issues pointed out by Kernel Test Robot
> > >  * Fix find_kfunc_desc to also take offset into consideration when comparing
> >
> > See [0]:
> >
> >
> >   [  444.075332] mod kfunc i=42
> >   [  444.075383] mod kfunc i=42
> >   [  444.075522] mod kfunc i=42
> >   [  444.075578] mod kfunc i=42
> >   [  444.075631] mod kfunc i=42
> >   [  444.075683] mod kfunc i=42
> >   [  444.075735] mod kfunc i=42
> >   [  444.0
> > This step has been truncated due to its large size. Download the full
> > logs from the
> > menu once the workflow run has completed.
> >
> >   [0] https://github.com/kernel-patches/bpf/runs/3606513281?check_suite_focus=true
> >
>
> I'll change it to not use printk in the next spin (maybe just set a variable).
> This probably blew up because of parallel test_progs stuff, since it would be
> called for each sys_enter as long the raw_tp prog is attached.

We should always be filtering by pid for cases where we use raw_tp/tp
on sys_enter*.

>
> --
> Kartikeya

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH bpf-next v3 03/10] bpf: btf: Introduce helpers for dynamic BTF set registration
  2021-09-15 16:18   ` Alexei Starovoitov
@ 2021-09-15 18:06     ` Kumar Kartikeya Dwivedi
  2021-09-16  3:04       ` Alexei Starovoitov
  0 siblings, 1 reply; 20+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2021-09-15 18:06 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song,
	Jesper Dangaard Brouer, Toke Høiland-Jørgensen, netdev

On Wed, Sep 15, 2021 at 09:48:35PM IST, Alexei Starovoitov wrote:
> On Wed, Sep 15, 2021 at 10:39:36AM +0530, Kumar Kartikeya Dwivedi wrote:
> > This adds helpers for registering btf_id_set from modules and the
> > check_kfunc_call callback that can be used to look them up.
> >
> > With in kernel sets, the way this is supposed to work is, in kernel
> > callback looks up within the in-kernel kfunc whitelist, and then defers
> > to the dynamic BTF set lookup if it doesn't find the BTF id. If there is
> > no in-kernel BTF id set, this callback can be used directly.
> >
> > Also fix includes for btf.h and bpfptr.h so that they can included in
> > isolation. This is in preparation for their usage in tcp_bbr, tcp_cubic
> > and tcp_dctcp modules in the next patch.
> >
> > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> > ---
> >  include/linux/bpfptr.h |  1 +
> >  include/linux/btf.h    | 32 ++++++++++++++++++++++++++
> >  kernel/bpf/btf.c       | 51 ++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 84 insertions(+)
> >
> > diff --git a/include/linux/bpfptr.h b/include/linux/bpfptr.h
> > index 546e27fc6d46..46e1757d06a3 100644
> > --- a/include/linux/bpfptr.h
> > +++ b/include/linux/bpfptr.h
> > @@ -3,6 +3,7 @@
> >  #ifndef _LINUX_BPFPTR_H
> >  #define _LINUX_BPFPTR_H
> >
> > +#include <linux/mm.h>
>
> Could you explain what this is for?
>

When e.g. tcp_bbr.c includes btf.h and btf_ids.h without this, it leads to this
error.

                 from net/ipv4/tcp_bbr.c:59:
./include/linux/bpfptr.h: In function ‘kvmemdup_bpfptr’:
./include/linux/bpfptr.h:67:19: error: implicit declaration of function ‘kvmalloc’;
 did you mean ‘kmalloc’? [-Werror=implicit-function-declaration]
   67 |         void *p = kvmalloc(len, GFP_USER | __GFP_NOWARN);
      |                   ^~~~~~~~
      |                   kmalloc
./include/linux/bpfptr.h:67:19: warning: initialization of ‘void *’ from ‘int’
	makes pointer from integer without a cast [-Wint-conversion]
./include/linux/bpfptr.h:72:17: error: implicit declaration of function ‘kvfree’;
	did you mean ‘kfree’? [-Werror=implicit-function-declaration]
   72 |                 kvfree(p);
      |                 ^~~~~~
      |                 kfree

> >  #include <linux/sockptr.h>

--
Kartikeya

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH bpf-next v3 06/10] bpf: Bump MAX_BPF_STACK size to 768 bytes
  2021-09-15 17:57     ` Kumar Kartikeya Dwivedi
@ 2021-09-16  2:56       ` Alexei Starovoitov
  0 siblings, 0 replies; 20+ messages in thread
From: Alexei Starovoitov @ 2021-09-16  2:56 UTC (permalink / raw)
  To: Kumar Kartikeya Dwivedi
  Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song,
	Jesper Dangaard Brouer, Toke Høiland-Jørgensen,
	Network Development

On Wed, Sep 15, 2021 at 10:57 AM Kumar Kartikeya Dwivedi
<memxor@gmail.com> wrote:
>
> On Wed, Sep 15, 2021 at 10:03:53PM IST, Alexei Starovoitov wrote:
> > On Wed, Sep 15, 2021 at 10:39:39AM +0530, Kumar Kartikeya Dwivedi wrote:
> > > Increase the maximum stack size accessible to BPF program to 768 bytes.
> > > This is done so that gen_loader can use 94 additional fds for kfunc BTFs
> > > that it passes in to fd_array from the remaining space available for the
> > > loader_stack struct to expand.
> > >
> > > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> > > ---
> > >  include/linux/filter.h | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/include/linux/filter.h b/include/linux/filter.h
> > > index 4a93c12543ee..b214189ece62 100644
> > > --- a/include/linux/filter.h
> > > +++ b/include/linux/filter.h
> > > @@ -82,8 +82,8 @@ struct ctl_table_header;
> > >   */
> > >  #define BPF_SYM_ELF_TYPE   't'
> > >
> > > -/* BPF program can access up to 512 bytes of stack space. */
> > > -#define MAX_BPF_STACK      512
> > > +/* BPF program can access up to 768 bytes of stack space. */
> > > +#define MAX_BPF_STACK      768
> >
> > Yikes.
> > I guess you meant as RFC, right? You didn't really propose
> > to increase prog stack size just for that, right?
> >
>
> Yes, and right, it's ugly :/.
>
> > In the later patch:
> > +/* MAX_BPF_STACK is 768 bytes, so (64 + 32 + 94 (MAX_KFUNC_DESCS) + 2) * 4 */
> >  #define MAX_USED_MAPS 64
> >  #define MAX_USED_PROGS 32
> >
> > @@ -31,6 +33,8 @@ struct loader_stack {
> >         __u32 btf_fd;
> >         __u32 map_fd[MAX_USED_MAPS];
> >         __u32 prog_fd[MAX_USED_PROGS];
> > +       /* Update insn->off store when reordering kfunc_btf_fd */
> > +       __u32 kfunc_btf_fd[MAX_KFUNC_DESCS];
> >         __u32 inner_map_fd;
> > };
> >
> > There are few other ways to do that.
> > For example:
> > A: rename map_fd[] into fds[] and store both map and btf FDs in there.
> > B: move map and btf FDs into data instead of stack.
>
> Both are great suggestions, I thought about A but not B, but it will be better
> (even though it requires more changes, we can do full 256 BTF fds using B).
> Thanks!

btw fd_array doesn't have to have valid FDs in all slots
when passed into the prog_load command.
If bpf prog doesn't index into them they can have garbage.
Just mentioning in case that simplifies the implementation.
I suspect packing btf_fds right after map_fds with no gaps will not be
hard to do, but in case it's somehow too painful there could be a gap.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH bpf-next v3 03/10] bpf: btf: Introduce helpers for dynamic BTF set registration
  2021-09-15 18:06     ` Kumar Kartikeya Dwivedi
@ 2021-09-16  3:04       ` Alexei Starovoitov
  0 siblings, 0 replies; 20+ messages in thread
From: Alexei Starovoitov @ 2021-09-16  3:04 UTC (permalink / raw)
  To: Kumar Kartikeya Dwivedi
  Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, Yonghong Song,
	Jesper Dangaard Brouer, Toke Høiland-Jørgensen,
	Network Development

On Wed, Sep 15, 2021 at 11:06 AM Kumar Kartikeya Dwivedi
<memxor@gmail.com> wrote:
>
> On Wed, Sep 15, 2021 at 09:48:35PM IST, Alexei Starovoitov wrote:
> > On Wed, Sep 15, 2021 at 10:39:36AM +0530, Kumar Kartikeya Dwivedi wrote:
> > > This adds helpers for registering btf_id_set from modules and the
> > > check_kfunc_call callback that can be used to look them up.
> > >
> > > With in kernel sets, the way this is supposed to work is, in kernel
> > > callback looks up within the in-kernel kfunc whitelist, and then defers
> > > to the dynamic BTF set lookup if it doesn't find the BTF id. If there is
> > > no in-kernel BTF id set, this callback can be used directly.
> > >
> > > Also fix includes for btf.h and bpfptr.h so that they can included in
> > > isolation. This is in preparation for their usage in tcp_bbr, tcp_cubic
> > > and tcp_dctcp modules in the next patch.
> > >
> > > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> > > ---
> > >  include/linux/bpfptr.h |  1 +
> > >  include/linux/btf.h    | 32 ++++++++++++++++++++++++++
> > >  kernel/bpf/btf.c       | 51 ++++++++++++++++++++++++++++++++++++++++++
> > >  3 files changed, 84 insertions(+)
> > >
> > > diff --git a/include/linux/bpfptr.h b/include/linux/bpfptr.h
> > > index 546e27fc6d46..46e1757d06a3 100644
> > > --- a/include/linux/bpfptr.h
> > > +++ b/include/linux/bpfptr.h
> > > @@ -3,6 +3,7 @@
> > >  #ifndef _LINUX_BPFPTR_H
> > >  #define _LINUX_BPFPTR_H
> > >
> > > +#include <linux/mm.h>
> >
> > Could you explain what this is for?
> >
>
> When e.g. tcp_bbr.c includes btf.h and btf_ids.h without this, it leads to this
> error.
>
>                  from net/ipv4/tcp_bbr.c:59:
> ./include/linux/bpfptr.h: In function ‘kvmemdup_bpfptr’:
> ./include/linux/bpfptr.h:67:19: error: implicit declaration of function ‘kvmalloc’;
>  did you mean ‘kmalloc’? [-Werror=implicit-function-declaration]
>    67 |         void *p = kvmalloc(len, GFP_USER | __GFP_NOWARN);
>       |                   ^~~~~~~~
>       |                   kmalloc
> ./include/linux/bpfptr.h:67:19: warning: initialization of ‘void *’ from ‘int’
>         makes pointer from integer without a cast [-Wint-conversion]
> ./include/linux/bpfptr.h:72:17: error: implicit declaration of function ‘kvfree’;
>         did you mean ‘kfree’? [-Werror=implicit-function-declaration]
>    72 |                 kvfree(p);
>       |                 ^~~~~~
>       |                 kfree

Interesting.
It's because of kvmalloc in kvmemdup_bpfptr.
Which is used in ___bpf_copy_key.
Which is used in map_update_elem.
And afair all maps enforce key_size < KMALLOC_MAX_SIZE.
Not sure why kvmalloc was there.
If it was kmalloc instead then
#include <linux/slab.h>
in linux/sockptr.h that is included by linux/bpfptr.h
would have been enough.
A food for thought.

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2021-09-16  3:04 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-15  5:09 [PATCH bpf-next v3 00/10] Support kernel module function calls from eBPF Kumar Kartikeya Dwivedi
2021-09-15  5:09 ` [PATCH bpf-next v3 01/10] bpf: Introduce BPF support for kernel module function calls Kumar Kartikeya Dwivedi
2021-09-15  5:09 ` [PATCH bpf-next v3 02/10] bpf: Be conservative while processing invalid kfunc calls Kumar Kartikeya Dwivedi
2021-09-15  5:09 ` [PATCH bpf-next v3 03/10] bpf: btf: Introduce helpers for dynamic BTF set registration Kumar Kartikeya Dwivedi
2021-09-15 16:18   ` Alexei Starovoitov
2021-09-15 18:06     ` Kumar Kartikeya Dwivedi
2021-09-16  3:04       ` Alexei Starovoitov
2021-09-15  5:09 ` [PATCH bpf-next v3 04/10] tools: Allow specifying base BTF file in resolve_btfids Kumar Kartikeya Dwivedi
2021-09-15  5:09 ` [PATCH bpf-next v3 05/10] bpf: Enable TCP congestion control kfunc from modules Kumar Kartikeya Dwivedi
2021-09-15  5:09 ` [PATCH bpf-next v3 06/10] bpf: Bump MAX_BPF_STACK size to 768 bytes Kumar Kartikeya Dwivedi
2021-09-15 16:33   ` Alexei Starovoitov
2021-09-15 17:57     ` Kumar Kartikeya Dwivedi
2021-09-16  2:56       ` Alexei Starovoitov
2021-09-15  5:09 ` [PATCH bpf-next v3 07/10] libbpf: Support kernel module function calls Kumar Kartikeya Dwivedi
2021-09-15  5:09 ` [PATCH bpf-next v3 08/10] libbpf: Resolve invalid weak kfunc calls with imm = 0, off = 0 Kumar Kartikeya Dwivedi
2021-09-15  5:09 ` [PATCH bpf-next v3 09/10] libbpf: Update gen_loader to emit BTF_KIND_FUNC relocations Kumar Kartikeya Dwivedi
2021-09-15  5:09 ` [PATCH bpf-next v3 10/10] bpf, selftests: Add basic test for module kfunc call Kumar Kartikeya Dwivedi
2021-09-15 16:04 ` [PATCH bpf-next v3 00/10] Support kernel module function calls from eBPF Andrii Nakryiko
2021-09-15 18:03   ` Kumar Kartikeya Dwivedi
2021-09-15 18:05     ` Andrii Nakryiko

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).