bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 bpf-next 0/4] bpf: Add detection of kfuncs.
@ 2023-03-17 20:19 Alexei Starovoitov
  2023-03-17 20:19 ` [PATCH v2 bpf-next 1/4] bpf: Allow ld_imm64 instruction to point to kfunc Alexei Starovoitov
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: Alexei Starovoitov @ 2023-03-17 20:19 UTC (permalink / raw)
  To: davem
  Cc: daniel, andrii, martin.lau, void, davemarchevsky, tj, memxor,
	netdev, bpf, kernel-team

From: Alexei Starovoitov <ast@kernel.org>

Allow BPF programs detect at load time whether particular kfunc exists.

Patch 1: Allow ld_imm64 to point to kfunc in the kernel.
Patch 2: Fix relocation of kfunc in ld_imm64 insn when kfunc is in kernel module.
Patch 3: Introduce bpf_ksym_exists() macro.
Patch 4: selftest.

NOTE: detection of kfuncs from light skeleton is not supported yet.

Alexei Starovoitov (4):
  bpf: Allow ld_imm64 instruction to point to kfunc.
  libbpf: Fix relocation of kfunc ksym in ld_imm64 insn.
  libbpf: Introduce bpf_ksym_exists() macro.
  selftests/bpf: Add test for bpf_ksym_exists().

 kernel/bpf/verifier.c                         | 17 ++++++++++------
 tools/lib/bpf/bpf_helpers.h                   |  3 +++
 tools/lib/bpf/libbpf.c                        |  6 ++++++
 .../selftests/bpf/progs/task_kfunc_success.c  | 20 ++++++++++++++++++-
 4 files changed, 39 insertions(+), 7 deletions(-)

-- 
2.34.1


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

* [PATCH v2 bpf-next 1/4] bpf: Allow ld_imm64 instruction to point to kfunc.
  2023-03-17 20:19 [PATCH v2 bpf-next 0/4] bpf: Add detection of kfuncs Alexei Starovoitov
@ 2023-03-17 20:19 ` Alexei Starovoitov
  2023-03-17 20:19 ` [PATCH v2 bpf-next 2/4] libbpf: Fix relocation of kfunc ksym in ld_imm64 insn Alexei Starovoitov
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Alexei Starovoitov @ 2023-03-17 20:19 UTC (permalink / raw)
  To: davem
  Cc: daniel, andrii, martin.lau, void, davemarchevsky, tj, memxor,
	netdev, bpf, kernel-team

From: Alexei Starovoitov <ast@kernel.org>

Allow ld_imm64 insn with BPF_PSEUDO_BTF_ID to hold the address of kfunc. The
ld_imm64 pointing to a valid kfunc will be seen as non-null PTR_TO_MEM by
is_branch_taken() logic of the verifier, while libbpf will resolve address to
unknown kfunc as ld_imm64 reg, 0 which will also be recognized by
is_branch_taken() and the verifier will proceed dead code elimination. BPF
programs can use this logic to detect at load time whether kfunc is present in
the kernel with bpf_ksym_exists() macro that is introduced in the next patches.

Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Reviewed-by: Martin KaFai Lau <martin.lau@kernel.org>
Acked-by: John Fastabend <john.fastabend@gmail.com>
Reviewed-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
 kernel/bpf/verifier.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index d62b7127ff2a..1bc8a6d6fdd2 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -15952,8 +15952,8 @@ static int check_pseudo_btf_id(struct bpf_verifier_env *env,
 		goto err_put;
 	}
 
-	if (!btf_type_is_var(t)) {
-		verbose(env, "pseudo btf_id %d in ldimm64 isn't KIND_VAR.\n", id);
+	if (!btf_type_is_var(t) && !btf_type_is_func(t)) {
+		verbose(env, "pseudo btf_id %d in ldimm64 isn't KIND_VAR or KIND_FUNC\n", id);
 		err = -EINVAL;
 		goto err_put;
 	}
@@ -15966,6 +15966,14 @@ static int check_pseudo_btf_id(struct bpf_verifier_env *env,
 		err = -ENOENT;
 		goto err_put;
 	}
+	insn[0].imm = (u32)addr;
+	insn[1].imm = addr >> 32;
+
+	if (btf_type_is_func(t)) {
+		aux->btf_var.reg_type = PTR_TO_MEM | MEM_RDONLY;
+		aux->btf_var.mem_size = 0;
+		goto check_btf;
+	}
 
 	datasec_id = find_btf_percpu_datasec(btf);
 	if (datasec_id > 0) {
@@ -15978,9 +15986,6 @@ static int check_pseudo_btf_id(struct bpf_verifier_env *env,
 		}
 	}
 
-	insn[0].imm = (u32)addr;
-	insn[1].imm = addr >> 32;
-
 	type = t->type;
 	t = btf_type_skip_modifiers(btf, type, NULL);
 	if (percpu) {
@@ -16008,7 +16013,7 @@ static int check_pseudo_btf_id(struct bpf_verifier_env *env,
 		aux->btf_var.btf = btf;
 		aux->btf_var.btf_id = type;
 	}
-
+check_btf:
 	/* check whether we recorded this BTF (and maybe module) already */
 	for (i = 0; i < env->used_btf_cnt; i++) {
 		if (env->used_btfs[i].btf == btf) {
-- 
2.34.1


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

* [PATCH v2 bpf-next 2/4] libbpf: Fix relocation of kfunc ksym in ld_imm64 insn.
  2023-03-17 20:19 [PATCH v2 bpf-next 0/4] bpf: Add detection of kfuncs Alexei Starovoitov
  2023-03-17 20:19 ` [PATCH v2 bpf-next 1/4] bpf: Allow ld_imm64 instruction to point to kfunc Alexei Starovoitov
@ 2023-03-17 20:19 ` Alexei Starovoitov
  2023-03-17 22:49   ` Andrii Nakryiko
  2023-03-17 20:19 ` [PATCH v2 bpf-next 3/4] libbpf: Introduce bpf_ksym_exists() macro Alexei Starovoitov
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Alexei Starovoitov @ 2023-03-17 20:19 UTC (permalink / raw)
  To: davem
  Cc: daniel, andrii, martin.lau, void, davemarchevsky, tj, memxor,
	netdev, bpf, kernel-team

From: Alexei Starovoitov <ast@kernel.org>

void *p = kfunc; -> generates ld_imm64 insn.
kfunc() -> generates bpf_call insn.

libbpf patches bpf_call insn correctly while only btf_id part of ld_imm64 is
set in the former case. Which means that pointers to kfuncs in modules are not
patched correctly and the verifier rejects load of such programs due to btf_id
being out of range. Fix libbpf to patch ld_imm64 for kfunc.

Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 tools/lib/bpf/libbpf.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index a557718401e4..4c34fbd7b5be 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -7533,6 +7533,12 @@ static int bpf_object__resolve_ksym_func_btf_id(struct bpf_object *obj,
 	ext->is_set = true;
 	ext->ksym.kernel_btf_id = kfunc_id;
 	ext->ksym.btf_fd_idx = mod_btf ? mod_btf->fd_array_idx : 0;
+	/* Also set kernel_btf_obj_fd to make sure that bpf_object__relocate_data()
+	 * populates FD into ld_imm64 insn when it's used to point to kfunc.
+	 * {kernel_btf_id, btf_fd_idx} -> fixup bpf_call.
+	 * {kernel_btf_id, kernel_btf_obj_fd} -> fixup ld_imm64.
+	 */
+	ext->ksym.kernel_btf_obj_fd = mod_btf ? mod_btf->fd : 0;
 	pr_debug("extern (func ksym) '%s': resolved to kernel [%d]\n",
 		 ext->name, kfunc_id);
 
-- 
2.34.1


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

* [PATCH v2 bpf-next 3/4] libbpf: Introduce bpf_ksym_exists() macro.
  2023-03-17 20:19 [PATCH v2 bpf-next 0/4] bpf: Add detection of kfuncs Alexei Starovoitov
  2023-03-17 20:19 ` [PATCH v2 bpf-next 1/4] bpf: Allow ld_imm64 instruction to point to kfunc Alexei Starovoitov
  2023-03-17 20:19 ` [PATCH v2 bpf-next 2/4] libbpf: Fix relocation of kfunc ksym in ld_imm64 insn Alexei Starovoitov
@ 2023-03-17 20:19 ` Alexei Starovoitov
  2023-03-17 22:53   ` Andrii Nakryiko
  2023-03-17 20:19 ` [PATCH v2 bpf-next 4/4] selftests/bpf: Add test for bpf_ksym_exists() Alexei Starovoitov
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Alexei Starovoitov @ 2023-03-17 20:19 UTC (permalink / raw)
  To: davem
  Cc: daniel, andrii, martin.lau, void, davemarchevsky, tj, memxor,
	netdev, bpf, kernel-team

From: Alexei Starovoitov <ast@kernel.org>

Introduce bpf_ksym_exists() macro that can be used by BPF programs
to detect at load time whether particular ksym (either variable or kfunc)
is present in the kernel.

Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 tools/lib/bpf/bpf_helpers.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h
index 7d12d3e620cc..b49823117dae 100644
--- a/tools/lib/bpf/bpf_helpers.h
+++ b/tools/lib/bpf/bpf_helpers.h
@@ -177,6 +177,9 @@ enum libbpf_tristate {
 #define __kptr_untrusted __attribute__((btf_type_tag("kptr_untrusted")))
 #define __kptr __attribute__((btf_type_tag("kptr")))
 
+#define bpf_ksym_exists(sym) \
+	({ _Static_assert(!__builtin_constant_p(!!sym), #sym " should be marked as __weak"); !!sym; })
+
 #ifndef ___bpf_concat
 #define ___bpf_concat(a, b) a ## b
 #endif
-- 
2.34.1


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

* [PATCH v2 bpf-next 4/4] selftests/bpf: Add test for bpf_ksym_exists().
  2023-03-17 20:19 [PATCH v2 bpf-next 0/4] bpf: Add detection of kfuncs Alexei Starovoitov
                   ` (2 preceding siblings ...)
  2023-03-17 20:19 ` [PATCH v2 bpf-next 3/4] libbpf: Introduce bpf_ksym_exists() macro Alexei Starovoitov
@ 2023-03-17 20:19 ` Alexei Starovoitov
  2023-03-17 23:00 ` [PATCH v2 bpf-next 0/4] bpf: Add detection of kfuncs patchwork-bot+netdevbpf
  2023-07-25 20:44 ` Matt Bobrowski
  5 siblings, 0 replies; 11+ messages in thread
From: Alexei Starovoitov @ 2023-03-17 20:19 UTC (permalink / raw)
  To: davem
  Cc: daniel, andrii, martin.lau, void, davemarchevsky, tj, memxor,
	netdev, bpf, kernel-team

From: Alexei Starovoitov <ast@kernel.org>

Add load and run time test for bpf_ksym_exists() and check that the verifier
performs dead code elimination for non-existing kfunc.

Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Reviewed-by: Martin KaFai Lau <martin.lau@kernel.org>
Acked-by: John Fastabend <john.fastabend@gmail.com>
Reviewed-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
 .../selftests/bpf/progs/task_kfunc_success.c  | 20 ++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/progs/task_kfunc_success.c b/tools/testing/selftests/bpf/progs/task_kfunc_success.c
index 4f61596b0242..cfa7f12b84e8 100644
--- a/tools/testing/selftests/bpf/progs/task_kfunc_success.c
+++ b/tools/testing/selftests/bpf/progs/task_kfunc_success.c
@@ -17,6 +17,10 @@ int err, pid;
  *         TP_PROTO(struct task_struct *p, u64 clone_flags)
  */
 
+struct task_struct *bpf_task_acquire(struct task_struct *p) __ksym __weak;
+void invalid_kfunc(void) __ksym __weak;
+void bpf_testmod_test_mod_kfunc(int i) __ksym __weak;
+
 static bool is_test_kfunc_task(void)
 {
 	int cur_pid = bpf_get_current_pid_tgid() >> 32;
@@ -26,7 +30,21 @@ static bool is_test_kfunc_task(void)
 
 static int test_acquire_release(struct task_struct *task)
 {
-	struct task_struct *acquired;
+	struct task_struct *acquired = NULL;
+
+	if (!bpf_ksym_exists(bpf_task_acquire)) {
+		err = 3;
+		return 0;
+	}
+	if (!bpf_ksym_exists(bpf_testmod_test_mod_kfunc)) {
+		err = 4;
+		return 0;
+	}
+	if (bpf_ksym_exists(invalid_kfunc)) {
+		/* the verifier's dead code elimination should remove this */
+		err = 5;
+		asm volatile ("goto -1"); /* for (;;); */
+	}
 
 	acquired = bpf_task_acquire(task);
 	bpf_task_release(acquired);
-- 
2.34.1


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

* Re: [PATCH v2 bpf-next 2/4] libbpf: Fix relocation of kfunc ksym in ld_imm64 insn.
  2023-03-17 20:19 ` [PATCH v2 bpf-next 2/4] libbpf: Fix relocation of kfunc ksym in ld_imm64 insn Alexei Starovoitov
@ 2023-03-17 22:49   ` Andrii Nakryiko
  0 siblings, 0 replies; 11+ messages in thread
From: Andrii Nakryiko @ 2023-03-17 22:49 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: davem, daniel, andrii, martin.lau, void, davemarchevsky, tj,
	memxor, netdev, bpf, kernel-team

On Fri, Mar 17, 2023 at 1:19 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> From: Alexei Starovoitov <ast@kernel.org>
>
> void *p = kfunc; -> generates ld_imm64 insn.
> kfunc() -> generates bpf_call insn.
>
> libbpf patches bpf_call insn correctly while only btf_id part of ld_imm64 is
> set in the former case. Which means that pointers to kfuncs in modules are not
> patched correctly and the verifier rejects load of such programs due to btf_id
> being out of range. Fix libbpf to patch ld_imm64 for kfunc.
>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> ---
>  tools/lib/bpf/libbpf.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index a557718401e4..4c34fbd7b5be 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -7533,6 +7533,12 @@ static int bpf_object__resolve_ksym_func_btf_id(struct bpf_object *obj,
>         ext->is_set = true;
>         ext->ksym.kernel_btf_id = kfunc_id;
>         ext->ksym.btf_fd_idx = mod_btf ? mod_btf->fd_array_idx : 0;
> +       /* Also set kernel_btf_obj_fd to make sure that bpf_object__relocate_data()
> +        * populates FD into ld_imm64 insn when it's used to point to kfunc.
> +        * {kernel_btf_id, btf_fd_idx} -> fixup bpf_call.
> +        * {kernel_btf_id, kernel_btf_obj_fd} -> fixup ld_imm64.
> +        */
> +       ext->ksym.kernel_btf_obj_fd = mod_btf ? mod_btf->fd : 0;
>         pr_debug("extern (func ksym) '%s': resolved to kernel [%d]\n",
>                  ext->name, kfunc_id);

we should report module name here as well, I'll send a patch for both
func and var ksyms

>
> --
> 2.34.1
>

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

* Re: [PATCH v2 bpf-next 3/4] libbpf: Introduce bpf_ksym_exists() macro.
  2023-03-17 20:19 ` [PATCH v2 bpf-next 3/4] libbpf: Introduce bpf_ksym_exists() macro Alexei Starovoitov
@ 2023-03-17 22:53   ` Andrii Nakryiko
  0 siblings, 0 replies; 11+ messages in thread
From: Andrii Nakryiko @ 2023-03-17 22:53 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: davem, daniel, andrii, martin.lau, void, davemarchevsky, tj,
	memxor, netdev, bpf, kernel-team

On Fri, Mar 17, 2023 at 1:19 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> From: Alexei Starovoitov <ast@kernel.org>
>
> Introduce bpf_ksym_exists() macro that can be used by BPF programs
> to detect at load time whether particular ksym (either variable or kfunc)
> is present in the kernel.
>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> ---
>  tools/lib/bpf/bpf_helpers.h | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h
> index 7d12d3e620cc..b49823117dae 100644
> --- a/tools/lib/bpf/bpf_helpers.h
> +++ b/tools/lib/bpf/bpf_helpers.h
> @@ -177,6 +177,9 @@ enum libbpf_tristate {
>  #define __kptr_untrusted __attribute__((btf_type_tag("kptr_untrusted")))
>  #define __kptr __attribute__((btf_type_tag("kptr")))
>
> +#define bpf_ksym_exists(sym) \
> +       ({ _Static_assert(!__builtin_constant_p(!!sym), #sym " should be marked as __weak"); !!sym; })
> +

I reformatted this to fit under 100 characters.

-#define bpf_ksym_exists(sym) \
-       ({ _Static_assert(!__builtin_constant_p(!!sym), #sym " should
be marked as __weak"); !!sym; })
+#define bpf_ksym_exists(sym) ({
                                 \
+       _Static_assert(!__builtin_constant_p(!!sym), #sym " should be
marked as __weak");       \
+       !!sym;
                         \
+})


Other than that, it looks great! Applied to bpf-next, thanks.



>  #ifndef ___bpf_concat
>  #define ___bpf_concat(a, b) a ## b
>  #endif
> --
> 2.34.1
>

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

* Re: [PATCH v2 bpf-next 0/4] bpf: Add detection of kfuncs.
  2023-03-17 20:19 [PATCH v2 bpf-next 0/4] bpf: Add detection of kfuncs Alexei Starovoitov
                   ` (3 preceding siblings ...)
  2023-03-17 20:19 ` [PATCH v2 bpf-next 4/4] selftests/bpf: Add test for bpf_ksym_exists() Alexei Starovoitov
@ 2023-03-17 23:00 ` patchwork-bot+netdevbpf
  2023-07-25 20:44 ` Matt Bobrowski
  5 siblings, 0 replies; 11+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-03-17 23:00 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: davem, daniel, andrii, martin.lau, void, davemarchevsky, tj,
	memxor, netdev, bpf, kernel-team

Hello:

This series was applied to bpf/bpf-next.git (master)
by Andrii Nakryiko <andrii@kernel.org>:

On Fri, 17 Mar 2023 13:19:16 -0700 you wrote:
> From: Alexei Starovoitov <ast@kernel.org>
> 
> Allow BPF programs detect at load time whether particular kfunc exists.
> 
> Patch 1: Allow ld_imm64 to point to kfunc in the kernel.
> Patch 2: Fix relocation of kfunc in ld_imm64 insn when kfunc is in kernel module.
> Patch 3: Introduce bpf_ksym_exists() macro.
> Patch 4: selftest.
> 
> [...]

Here is the summary with links:
  - [v2,bpf-next,1/4] bpf: Allow ld_imm64 instruction to point to kfunc.
    https://git.kernel.org/bpf/bpf-next/c/58aa2afbb1e6
  - [v2,bpf-next,2/4] libbpf: Fix relocation of kfunc ksym in ld_imm64 insn.
    https://git.kernel.org/bpf/bpf-next/c/5fc13ad59b60
  - [v2,bpf-next,3/4] libbpf: Introduce bpf_ksym_exists() macro.
    https://git.kernel.org/bpf/bpf-next/c/5cbd3fe3a91d
  - [v2,bpf-next,4/4] selftests/bpf: Add test for bpf_ksym_exists().
    https://git.kernel.org/bpf/bpf-next/c/95fdf6e313a9

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



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

* Re: [PATCH v2 bpf-next 0/4] bpf: Add detection of kfuncs.
  2023-03-17 20:19 [PATCH v2 bpf-next 0/4] bpf: Add detection of kfuncs Alexei Starovoitov
                   ` (4 preceding siblings ...)
  2023-03-17 23:00 ` [PATCH v2 bpf-next 0/4] bpf: Add detection of kfuncs patchwork-bot+netdevbpf
@ 2023-07-25 20:44 ` Matt Bobrowski
  2023-07-25 21:00   ` Alexei Starovoitov
  5 siblings, 1 reply; 11+ messages in thread
From: Matt Bobrowski @ 2023-07-25 20:44 UTC (permalink / raw)
  To: Alexei Starovoitov, andrii, ast
  Cc: davem, daniel, martin.lau, void, davemarchevsky, tj, memxor,
	netdev, bpf, kernel-team

Hey Alexei/Andrii,

On Fri, Mar 17, 2023 at 01:19:16PM -0700, Alexei Starovoitov wrote:
> From: Alexei Starovoitov <ast@kernel.org>
> 
> Allow BPF programs detect at load time whether particular kfunc exists.

So, I'm running a GCC built 6.3.7 Linux kernel and I'm attempting to
detect whether a specific kfunc i.e. bpf_rcu_read_lock/unlock() exists
using the bpf_ksym_exists() macro. However, I'm running into several
BPF verifier constraints that I'm not entirely sure how to work around
on the aforementioned Linux kernel version, and hence why I'm reaching
out for some guidance.

The first BPF verifier constraint that I'm running into is that prior
to commit 58aa2afbb1e6 ("bpf: Allow ld_imm64 instruction to point to
kfunc"), it seems that the ld_imm64 instruction with BPF_PSEUDO_BTF_ID
can only hold a ksym address for the kind KIND_VAR. However, when
attempting to use the kfuncs bpf_rcu_read_lock/unlock() from a BPF
program, the kind associated with the BPF_PSEUDO_BTF_ID is actually
KIND_FUNC, and therefore trips over this BPF verifier.

The code within the example BPF program is along the lines of the
following:
```
...
void bpf_rcu_read_lock(void) __ksym __weak;
void bpf_rcu_read_unlock(void) __ksym __weak;
...
if (bpf_ksym_exists(bpf_rcu_read_lock)) {
   bpf_rcu_read_lock();
}
...
if (bpf_ksym_exists(bpf_rcu_read_unlock)) {
   bpf_rcu_read_unlock();
}
...
```

The BPF verifier error message that is generated on a 6.3.7 Linux
kernel when attempting to load a BPF program that makes use of the
above approach is as follows:
   * "pseudo btf_id {BTF_ID} in ldimm64 isn't KIND_VAR"

The second BPF verifier constraint comes from attempting to work
around the first BPF verifier constraint that I've mentioned
above. This is trivially by dropping the conditionals that contain the
bpf_ksym_exists() check and unconditionally calling the kfuncs
bpf_rcu_read_lock/unlock().

The code within the example BPF program is along the lines of the
following:
```
...
void bpf_rcu_read_lock(void) __ksym __weak;
void bpf_rcu_read_unlock(void) __ksym __weak;
...
bpf_rcu_read_lock();
...
bpf_rcu_read_unlock();
...
```

However, in this case the BPF verifier error message that is generated
on a 6.3.7 Linux kernel is as follows:
   * "no vmlinux btf rcu tag support for kfunc bpf_rcu_read_lock"

This approach would be suboptimal anyway as the BPF program would fail
to load on older Linux kernels complaining that the kfunc is
referenced but couldn't be resolved.

Having said this, what's the best way to resolve this on a 6.3.7 Linux
kernel? The first BPF program I mentioned above making use of the
bpf_ksym_exists() macro works on a 6.4 Linux kernel with commit
58aa2afbb1e6 ("bpf: Allow ld_imm64 instruction to point to kfunc")
applied. Also, the first BPF program I mentioned above works on a
6.1.* Linux kernel...

> Patch 1: Allow ld_imm64 to point to kfunc in the kernel.
> Patch 2: Fix relocation of kfunc in ld_imm64 insn when kfunc is in kernel module.
> Patch 3: Introduce bpf_ksym_exists() macro.
> Patch 4: selftest.
> 
> NOTE: detection of kfuncs from light skeleton is not supported yet.
> 
> Alexei Starovoitov (4):
>   bpf: Allow ld_imm64 instruction to point to kfunc.
>   libbpf: Fix relocation of kfunc ksym in ld_imm64 insn.
>   libbpf: Introduce bpf_ksym_exists() macro.
>   selftests/bpf: Add test for bpf_ksym_exists().

/M

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

* Re: [PATCH v2 bpf-next 0/4] bpf: Add detection of kfuncs.
  2023-07-25 20:44 ` Matt Bobrowski
@ 2023-07-25 21:00   ` Alexei Starovoitov
  2023-07-26 12:08     ` Matt Bobrowski
  0 siblings, 1 reply; 11+ messages in thread
From: Alexei Starovoitov @ 2023-07-25 21:00 UTC (permalink / raw)
  To: Matt Bobrowski
  Cc: Andrii Nakryiko, Alexei Starovoitov, David S. Miller,
	Daniel Borkmann, Martin KaFai Lau, David Vernet, Dave Marchevsky,
	Tejun Heo, Kumar Kartikeya Dwivedi, Network Development, bpf,
	Kernel Team

On Tue, Jul 25, 2023 at 1:45 PM Matt Bobrowski <mattbobrowski@google.com> wrote:
>
> Hey Alexei/Andrii,
>
> On Fri, Mar 17, 2023 at 01:19:16PM -0700, Alexei Starovoitov wrote:
> > From: Alexei Starovoitov <ast@kernel.org>
> >
> > Allow BPF programs detect at load time whether particular kfunc exists.
>
> So, I'm running a GCC built 6.3.7 Linux kernel and I'm attempting to
> detect whether a specific kfunc i.e. bpf_rcu_read_lock/unlock() exists
> using the bpf_ksym_exists() macro. However, I'm running into several
> BPF verifier constraints that I'm not entirely sure how to work around
> on the aforementioned Linux kernel version, and hence why I'm reaching
> out for some guidance.
>
> The first BPF verifier constraint that I'm running into is that prior
> to commit 58aa2afbb1e6 ("bpf: Allow ld_imm64 instruction to point to
> kfunc"), it seems that the ld_imm64 instruction with BPF_PSEUDO_BTF_ID
> can only hold a ksym address for the kind KIND_VAR. However, when
> attempting to use the kfuncs bpf_rcu_read_lock/unlock() from a BPF
> program, the kind associated with the BPF_PSEUDO_BTF_ID is actually
> KIND_FUNC, and therefore trips over this BPF verifier.
>
> The code within the example BPF program is along the lines of the
> following:
> ```
> ...
> void bpf_rcu_read_lock(void) __ksym __weak;
> void bpf_rcu_read_unlock(void) __ksym __weak;
> ...
> if (bpf_ksym_exists(bpf_rcu_read_lock)) {
>    bpf_rcu_read_lock();
> }
> ...
> if (bpf_ksym_exists(bpf_rcu_read_unlock)) {
>    bpf_rcu_read_unlock();
> }
> ...
> ```
>
> The BPF verifier error message that is generated on a 6.3.7 Linux
> kernel when attempting to load a BPF program that makes use of the
> above approach is as follows:
>    * "pseudo btf_id {BTF_ID} in ldimm64 isn't KIND_VAR"
>
> The second BPF verifier constraint comes from attempting to work
> around the first BPF verifier constraint that I've mentioned
> above. This is trivially by dropping the conditionals that contain the
> bpf_ksym_exists() check and unconditionally calling the kfuncs
> bpf_rcu_read_lock/unlock().
>
> The code within the example BPF program is along the lines of the
> following:
> ```
> ...
> void bpf_rcu_read_lock(void) __ksym __weak;
> void bpf_rcu_read_unlock(void) __ksym __weak;
> ...
> bpf_rcu_read_lock();
> ...
> bpf_rcu_read_unlock();
> ...
> ```
>
> However, in this case the BPF verifier error message that is generated
> on a 6.3.7 Linux kernel is as follows:
>    * "no vmlinux btf rcu tag support for kfunc bpf_rcu_read_lock"
>
> This approach would be suboptimal anyway as the BPF program would fail
> to load on older Linux kernels complaining that the kfunc is
> referenced but couldn't be resolved.
>
> Having said this, what's the best way to resolve this on a 6.3.7 Linux
> kernel? The first BPF program I mentioned above making use of the
> bpf_ksym_exists() macro works on a 6.4 Linux kernel with commit
> 58aa2afbb1e6 ("bpf: Allow ld_imm64 instruction to point to kfunc")
> applied. Also, the first BPF program I mentioned above works on a
> 6.1.* Linux kernel...

Backport of that commit to 6.3.x is probably the only way.

And I don't think bpf_ksym_exists() actually works on 6.1

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

* Re: [PATCH v2 bpf-next 0/4] bpf: Add detection of kfuncs.
  2023-07-25 21:00   ` Alexei Starovoitov
@ 2023-07-26 12:08     ` Matt Bobrowski
  0 siblings, 0 replies; 11+ messages in thread
From: Matt Bobrowski @ 2023-07-26 12:08 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Andrii Nakryiko, Alexei Starovoitov, David S. Miller,
	Daniel Borkmann, Martin KaFai Lau, David Vernet, Dave Marchevsky,
	Tejun Heo, Kumar Kartikeya Dwivedi, Network Development, bpf,
	Kernel Team

On Tue, Jul 25, 2023 at 02:00:40PM -0700, Alexei Starovoitov wrote:
> On Tue, Jul 25, 2023 at 1:45 PM Matt Bobrowski <mattbobrowski@google.com> wrote:
> >
> > Hey Alexei/Andrii,
> >
> > On Fri, Mar 17, 2023 at 01:19:16PM -0700, Alexei Starovoitov wrote:
> > > From: Alexei Starovoitov <ast@kernel.org>
> > >
> > > Allow BPF programs detect at load time whether particular kfunc exists.
> >
> > So, I'm running a GCC built 6.3.7 Linux kernel and I'm attempting to
> > detect whether a specific kfunc i.e. bpf_rcu_read_lock/unlock() exists
> > using the bpf_ksym_exists() macro. However, I'm running into several
> > BPF verifier constraints that I'm not entirely sure how to work around
> > on the aforementioned Linux kernel version, and hence why I'm reaching
> > out for some guidance.
> >
> > The first BPF verifier constraint that I'm running into is that prior
> > to commit 58aa2afbb1e6 ("bpf: Allow ld_imm64 instruction to point to
> > kfunc"), it seems that the ld_imm64 instruction with BPF_PSEUDO_BTF_ID
> > can only hold a ksym address for the kind KIND_VAR. However, when
> > attempting to use the kfuncs bpf_rcu_read_lock/unlock() from a BPF
> > program, the kind associated with the BPF_PSEUDO_BTF_ID is actually
> > KIND_FUNC, and therefore trips over this BPF verifier.
> >
> > The code within the example BPF program is along the lines of the
> > following:
> > ```
> > ...
> > void bpf_rcu_read_lock(void) __ksym __weak;
> > void bpf_rcu_read_unlock(void) __ksym __weak;
> > ...
> > if (bpf_ksym_exists(bpf_rcu_read_lock)) {
> >    bpf_rcu_read_lock();
> > }
> > ...
> > if (bpf_ksym_exists(bpf_rcu_read_unlock)) {
> >    bpf_rcu_read_unlock();
> > }
> > ...
> > ```
> >
> > The BPF verifier error message that is generated on a 6.3.7 Linux
> > kernel when attempting to load a BPF program that makes use of the
> > above approach is as follows:
> >    * "pseudo btf_id {BTF_ID} in ldimm64 isn't KIND_VAR"
> >
> > The second BPF verifier constraint comes from attempting to work
> > around the first BPF verifier constraint that I've mentioned
> > above. This is trivially by dropping the conditionals that contain the
> > bpf_ksym_exists() check and unconditionally calling the kfuncs
> > bpf_rcu_read_lock/unlock().
> >
> > The code within the example BPF program is along the lines of the
> > following:
> > ```
> > ...
> > void bpf_rcu_read_lock(void) __ksym __weak;
> > void bpf_rcu_read_unlock(void) __ksym __weak;
> > ...
> > bpf_rcu_read_lock();
> > ...
> > bpf_rcu_read_unlock();
> > ...
> > ```
> >
> > However, in this case the BPF verifier error message that is generated
> > on a 6.3.7 Linux kernel is as follows:
> >    * "no vmlinux btf rcu tag support for kfunc bpf_rcu_read_lock"
> >
> > This approach would be suboptimal anyway as the BPF program would fail
> > to load on older Linux kernels complaining that the kfunc is
> > referenced but couldn't be resolved.
> >
> > Having said this, what's the best way to resolve this on a 6.3.7 Linux
> > kernel? The first BPF program I mentioned above making use of the
> > bpf_ksym_exists() macro works on a 6.4 Linux kernel with commit
> > 58aa2afbb1e6 ("bpf: Allow ld_imm64 instruction to point to kfunc")
> > applied. Also, the first BPF program I mentioned above works on a
> > 6.1.* Linux kernel...
> 
> Backport of that commit to 6.3.x is probably the only way.

Ah, that's very unfortunate. Should we consider sending this patch
series to linux-stable so that it can be considered for 6.3.x?

/M

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

end of thread, other threads:[~2023-07-26 12:08 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-17 20:19 [PATCH v2 bpf-next 0/4] bpf: Add detection of kfuncs Alexei Starovoitov
2023-03-17 20:19 ` [PATCH v2 bpf-next 1/4] bpf: Allow ld_imm64 instruction to point to kfunc Alexei Starovoitov
2023-03-17 20:19 ` [PATCH v2 bpf-next 2/4] libbpf: Fix relocation of kfunc ksym in ld_imm64 insn Alexei Starovoitov
2023-03-17 22:49   ` Andrii Nakryiko
2023-03-17 20:19 ` [PATCH v2 bpf-next 3/4] libbpf: Introduce bpf_ksym_exists() macro Alexei Starovoitov
2023-03-17 22:53   ` Andrii Nakryiko
2023-03-17 20:19 ` [PATCH v2 bpf-next 4/4] selftests/bpf: Add test for bpf_ksym_exists() Alexei Starovoitov
2023-03-17 23:00 ` [PATCH v2 bpf-next 0/4] bpf: Add detection of kfuncs patchwork-bot+netdevbpf
2023-07-25 20:44 ` Matt Bobrowski
2023-07-25 21:00   ` Alexei Starovoitov
2023-07-26 12:08     ` Matt Bobrowski

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