[PATCH bpf-next 0/3] Verify global subprogs lazily

[PATCH bpf-next 0/3] Verify global subprogs lazily

[Andrii Nakryiko]

See patch #2 for justification. In few words, current eager verification of
global func prevents BPF CO-RE approaches to be applied to global functions.

Patch #1 is just a nicety to emit global subprog names in verifier logs.

Patch #3 adds selftests validating new lazy semantics.

Andrii Nakryiko (3):
  bpf: emit global subprog name in verifier logs
  bpf: validate global subprogs lazily
  selftests/bpf: add lazy global subprog validation tests

 include/linux/bpf.h                           |  2 +
 kernel/bpf/verifier.c                         | 88 ++++++++++++++----
 .../selftests/bpf/prog_tests/verifier.c       |  2 +
 .../selftests/bpf/progs/test_global_func12.c  |  4 +-
 .../bpf/progs/verifier_global_subprogs.c      | 92 +++++++++++++++++++
 .../bpf/progs/verifier_subprog_precision.c    |  4 +-
 6 files changed, 168 insertions(+), 24 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/progs/verifier_global_subprogs.c

-- 
2.34.1

[PATCH bpf-next 1/3] bpf: emit global subprog name in verifier logs

[Andrii Nakryiko]

We have the name, instead of emitting just func#N to identify global
subprog, augment verifier log messages with actual function name to make
it more user-friendly.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 kernel/bpf/verifier.c | 39 +++++++++++++++++++++++++--------------
 1 file changed, 25 insertions(+), 14 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 1340921ea311..fe2fd76d6ec4 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -339,6 +339,11 @@ struct bpf_kfunc_call_arg_meta {
 
 struct btf *btf_vmlinux;
 
+static const char *btf_type_name(const struct btf *btf, u32 id)
+{
+	return btf_name_by_offset(btf, btf_type_by_id(btf, id)->name_off);
+}
+
 static DEFINE_MUTEX(bpf_verifier_lock);
 static DEFINE_MUTEX(bpf_percpu_ma_lock);
 
@@ -418,6 +423,17 @@ static bool subprog_is_global(const struct bpf_verifier_env *env, int subprog)
 	return aux && aux[subprog].linkage == BTF_FUNC_GLOBAL;
 }
 
+static const char *subprog_name(const struct bpf_verifier_env *env, int subprog)
+{
+	struct bpf_func_info *info;
+
+	if (!env->prog->aux->func_info)
+		return "";
+
+	info = &env->prog->aux->func_info[subprog];
+	return btf_type_name(env->prog->aux->btf, info->type_id);
+}
+
 static bool reg_may_point_to_spin_lock(const struct bpf_reg_state *reg)
 {
 	return btf_record_has_field(reg_btf_record(reg), BPF_SPIN_LOCK);
@@ -576,11 +592,6 @@ static int iter_get_spi(struct bpf_verifier_env *env, struct bpf_reg_state *reg,
 	return stack_slot_obj_get_spi(env, reg, "iter", nr_slots);
 }
 
-static const char *btf_type_name(const struct btf *btf, u32 id)
-{
-	return btf_name_by_offset(btf, btf_type_by_id(btf, id)->name_off);
-}
-
 static enum bpf_dynptr_type arg_to_dynptr_type(enum bpf_arg_type arg_type)
 {
 	switch (arg_type & DYNPTR_TYPE_FLAG_MASK) {
@@ -9128,15 +9139,16 @@ static int __check_func_call(struct bpf_verifier_env *env, struct bpf_insn *insn
 	if (err == -EFAULT)
 		return err;
 	if (subprog_is_global(env, subprog)) {
+		const char *sub_name = subprog_name(env, subprog);
+
 		if (err) {
-			verbose(env, "Caller passes invalid args into func#%d\n",
-				subprog);
+			verbose(env, "Caller passes invalid args into func#%d ('%s')\n",
+				subprog, sub_name);
 			return err;
 		} else {
-			if (env->log.level & BPF_LOG_LEVEL)
-				verbose(env,
-					"Func#%d is global and valid. Skipping.\n",
-					subprog);
+			verbose(env, "Func#%d ('%s') is global and assumed valid.\n",
+				subprog, sub_name);
+
 			clear_caller_saved_regs(env, caller->regs);
 
 			/* All global functions return a 64-bit SCALAR_VALUE */
@@ -19763,9 +19775,8 @@ static int do_check_subprogs(struct bpf_verifier_env *env)
 		if (ret) {
 			return ret;
 		} else if (env->log.level & BPF_LOG_LEVEL) {
-			verbose(env,
-				"Func#%d is safe for any args that match its prototype\n",
-				i);
+			verbose(env, "Func#%d ('%s') is safe for any args that match its prototype\n",
+				i, subprog_name(env, i));
 		}
 	}
 	return 0;
-- 
2.34.1

Re: [PATCH bpf-next 1/3] bpf: emit global subprog name in verifier logs

[Eduard Zingerman]

On Wed, 2023-11-22 at 13:31 -0800, Andrii Nakryiko wrote:
> We have the name, instead of emitting just func#N to identify global
> subprog, augment verifier log messages with actual function name to make
> it more user-friendly.
> 
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> ---

Acked-by: Eduard Zingerman <eddyz87@gmail.com>

[...]

[PATCH bpf-next 2/3] bpf: validate global subprogs lazily

[Andrii Nakryiko]

Slightly change BPF verifier logic around eagerness and order of global
subprog validation. Instead of going over every global subprog eagerly
and validating it before main (entry) BPF program is verified, turn it
around. Validate main program first, mark subprogs that were called from
main program for later verification, but otherwise assume it is valid.
Afterwards, go over marked global subprogs and validate those,
potentially marking some more global functions as being called. Continue
this process until all (transitively) callable global subprogs are
validated. It's a BFS traversal at its heart and will always converge.

This is an important change because it allows to feature-gate some
subprograms that might not be verifiable on some older kernel, depending
on supported set of features.

E.g., at some point, global functions were allowed to accept a pointer
to memory, which size is identified by user-provided type.
Unfortunately, older kernels don't support this feature. With BPF CO-RE
approach, the natural way would be to still compile BPF object file once
and guard calls to this global subprog with some CO-RE check or using
.rodata variables. That's what people do to guard usage of new helpers
or kfuncs, and any other new BPF-side feature that might be missing on
old kernels.

That's currently impossible to do with global subprogs, unfortunately,
because they are eagerly and unconditionally validated. This patch set
aims to change this, so that in the future when global funcs gain new
features, those can be guarded using BPF CO-RE techniques in the same
fashion as any other new kernel feature.

Two selftests had to be adjusted in sync with these changes.

test_global_func12 relied on eager global subprog validation failing
before main program failure is detected (unknown return value). Fix by
making sure that main program is always valid.

verifier_subprog_precision's parent_stack_slot_precise subtest relied on
verifier checkpointing heuristic to do a checkpoint at instruction #5,
but that's no longer true because we don't have enough jumps validated
before reaching insn #5 due to global subprogs being validated later.

Other than that, no changes, as one would expect.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 include/linux/bpf.h                           |  2 +
 kernel/bpf/verifier.c                         | 49 ++++++++++++++++---
 .../selftests/bpf/progs/test_global_func12.c  |  4 +-
 .../bpf/progs/verifier_subprog_precision.c    |  4 +-
 4 files changed, 49 insertions(+), 10 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 258ba232e302..eb447b0a9423 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1347,6 +1347,8 @@ static inline bool bpf_prog_has_trampoline(const struct bpf_prog *prog)
 struct bpf_func_info_aux {
 	u16 linkage;
 	bool unreliable;
+	bool called : 1;
+	bool verified : 1;
 };
 
 enum bpf_jit_poke_reason {
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index fe2fd76d6ec4..e06e06d61fc7 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -434,6 +434,11 @@ static const char *subprog_name(const struct bpf_verifier_env *env, int subprog)
 	return btf_type_name(env->prog->aux->btf, info->type_id);
 }
 
+static struct bpf_func_info_aux *subprog_aux(const struct bpf_verifier_env *env, int subprog)
+{
+	return &env->prog->aux->func_info_aux[subprog];
+}
+
 static bool reg_may_point_to_spin_lock(const struct bpf_reg_state *reg)
 {
 	return btf_record_has_field(reg_btf_record(reg), BPF_SPIN_LOCK);
@@ -9149,6 +9154,9 @@ static int __check_func_call(struct bpf_verifier_env *env, struct bpf_insn *insn
 			verbose(env, "Func#%d ('%s') is global and assumed valid.\n",
 				subprog, sub_name);
 
+			/* mark global subprog for verifying after main prog */
+			subprog_aux(env, subprog)->called = true;
+
 			clear_caller_saved_regs(env, caller->regs);
 
 			/* All global functions return a 64-bit SCALAR_VALUE */
@@ -19741,8 +19749,11 @@ static int do_check_common(struct bpf_verifier_env *env, int subprog, bool is_ex
 	return ret;
 }
 
-/* Verify all global functions in a BPF program one by one based on their BTF.
- * All global functions must pass verification. Otherwise the whole program is rejected.
+/* Lazily verify all global functions based on their BTF, if they are called
+ * from main BPF program or any of subprograms transitively.
+ * BPF global subprogs called from dead code are not validated.
+ * All callable global functions must pass verification.
+ * Otherwise the whole program is rejected.
  * Consider:
  * int bar(int);
  * int foo(int f)
@@ -19761,14 +19772,26 @@ static int do_check_common(struct bpf_verifier_env *env, int subprog, bool is_ex
 static int do_check_subprogs(struct bpf_verifier_env *env)
 {
 	struct bpf_prog_aux *aux = env->prog->aux;
-	int i, ret;
+	struct bpf_func_info_aux *sub_aux;
+	int i, ret, new_cnt;
 
 	if (!aux->func_info)
 		return 0;
 
+	/* exception callback is presumed to be always called */
+	if (env->exception_callback_subprog)
+		subprog_aux(env, env->exception_callback_subprog)->called = true;
+
+again:
+	new_cnt = 0;
 	for (i = 1; i < env->subprog_cnt; i++) {
-		if (aux->func_info_aux[i].linkage != BTF_FUNC_GLOBAL)
+		if (!subprog_is_global(env, i))
+			continue;
+
+		sub_aux = subprog_aux(env, i);
+		if (!sub_aux->called || sub_aux->verified)
 			continue;
+
 		env->insn_idx = env->subprog_info[i].start;
 		WARN_ON_ONCE(env->insn_idx == 0);
 		ret = do_check_common(env, i, env->exception_callback_subprog == i);
@@ -19778,7 +19801,21 @@ static int do_check_subprogs(struct bpf_verifier_env *env)
 			verbose(env, "Func#%d ('%s') is safe for any args that match its prototype\n",
 				i, subprog_name(env, i));
 		}
+
+		/* We verified new global subprog, it might have called some
+		 * more global subprogs that we haven't verified yet, so we
+		 * need to do another pass over subprogs to verify those.
+		 */
+		sub_aux->verified = true;
+		new_cnt++;
 	}
+
+	/* We can't loop forever as we verify at least one global subprog on
+	 * each pass.
+	 */
+	if (new_cnt)
+		goto again;
+
 	return 0;
 }
 
@@ -20424,8 +20461,8 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr, bpfptr_t uattr, __u3
 	if (ret < 0)
 		goto skip_full_check;
 
-	ret = do_check_subprogs(env);
-	ret = ret ?: do_check_main(env);
+	ret = do_check_main(env);
+	ret = ret ?: do_check_subprogs(env);
 
 	if (ret == 0 && bpf_prog_is_offloaded(env->prog->aux))
 		ret = bpf_prog_offload_finalize(env);
diff --git a/tools/testing/selftests/bpf/progs/test_global_func12.c b/tools/testing/selftests/bpf/progs/test_global_func12.c
index 7f159d83c6f6..6e03d42519a6 100644
--- a/tools/testing/selftests/bpf/progs/test_global_func12.c
+++ b/tools/testing/selftests/bpf/progs/test_global_func12.c
@@ -19,5 +19,7 @@ int global_func12(struct __sk_buff *skb)
 {
 	const struct S s = {.x = skb->len };
 
-	return foo(&s);
+	foo(&s);
+
+	return 1;
 }
diff --git a/tools/testing/selftests/bpf/progs/verifier_subprog_precision.c b/tools/testing/selftests/bpf/progs/verifier_subprog_precision.c
index db6b3143338b..23b8f2836a67 100644
--- a/tools/testing/selftests/bpf/progs/verifier_subprog_precision.c
+++ b/tools/testing/selftests/bpf/progs/verifier_subprog_precision.c
@@ -328,12 +328,10 @@ __naked int parent_stack_slot_precise(void)
 SEC("?raw_tp")
 __success __log_level(2)
 __msg("9: (0f) r1 += r6")
-__msg("mark_precise: frame0: last_idx 9 first_idx 6")
+__msg("mark_precise: frame0: last_idx 9 first_idx 0")
 __msg("mark_precise: frame0: regs=r6 stack= before 8: (bf) r1 = r7")
 __msg("mark_precise: frame0: regs=r6 stack= before 7: (27) r6 *= 4")
 __msg("mark_precise: frame0: regs=r6 stack= before 6: (79) r6 = *(u64 *)(r10 -8)")
-__msg("mark_precise: frame0: parent state regs= stack=-8:")
-__msg("mark_precise: frame0: last_idx 5 first_idx 0")
 __msg("mark_precise: frame0: regs= stack=-8 before 5: (85) call pc+6")
 __msg("mark_precise: frame0: regs= stack=-8 before 4: (b7) r1 = 0")
 __msg("mark_precise: frame0: regs= stack=-8 before 3: (7b) *(u64 *)(r10 -8) = r6")
-- 
2.34.1

Re: [PATCH bpf-next 2/3] bpf: validate global subprogs lazily

[Eduard Zingerman]

On Wed, 2023-11-22 at 13:31 -0800, Andrii Nakryiko wrote:
> Slightly change BPF verifier logic around eagerness and order of global
> subprog validation. Instead of going over every global subprog eagerly
> and validating it before main (entry) BPF program is verified, turn it
> around. Validate main program first, mark subprogs that were called from
> main program for later verification, but otherwise assume it is valid.
> Afterwards, go over marked global subprogs and validate those,
> potentially marking some more global functions as being called. Continue
> this process until all (transitively) callable global subprogs are
> validated. It's a BFS traversal at its heart and will always converge.
> 
> This is an important change because it allows to feature-gate some
> subprograms that might not be verifiable on some older kernel, depending
> on supported set of features.
>
[...]
> 
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>

Acked-by: Eduard Zingerman <eddyz87@gmai.com>

[...]

> @@ -19761,14 +19772,26 @@ static int do_check_common(struct bpf_verifier_env *env, int subprog, bool is_ex
>  static int do_check_subprogs(struct bpf_verifier_env *env)
>  {
>  	struct bpf_prog_aux *aux = env->prog->aux;
> -	int i, ret;
> +	struct bpf_func_info_aux *sub_aux;
> +	int i, ret, new_cnt;
>  
>  	if (!aux->func_info)
>  		return 0;
>  
> +	/* exception callback is presumed to be always called */
> +	if (env->exception_callback_subprog)
> +		subprog_aux(env, env->exception_callback_subprog)->called = true;
> +
> +again:

Nit: I'd use an explicit loop and a separate function here,
     but kernel people like their gotos...

[...]

[PATCH bpf-next 3/3] selftests/bpf: add lazy global subprog validation tests

[Andrii Nakryiko]

Add a few test that validate BPF verifier's lazy approach to validating
global subprogs.

We check that global subprogs that are called transitively through
another global subprog is validated.

We also check that invalid global subprog is not validated, if it's not
called from the main program.

And we also check that main program is always validated first, before
any of the subprogs.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 .../selftests/bpf/prog_tests/verifier.c       |  2 +
 .../bpf/progs/verifier_global_subprogs.c      | 92 +++++++++++++++++++
 2 files changed, 94 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/progs/verifier_global_subprogs.c

diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c
index e5c61aa6604a..1700e3c4b3f7 100644
--- a/tools/testing/selftests/bpf/prog_tests/verifier.c
+++ b/tools/testing/selftests/bpf/prog_tests/verifier.c
@@ -25,6 +25,7 @@
 #include "verifier_direct_stack_access_wraparound.skel.h"
 #include "verifier_div0.skel.h"
 #include "verifier_div_overflow.skel.h"
+#include "verifier_global_subprogs.skel.h"
 #include "verifier_gotol.skel.h"
 #include "verifier_helper_access_var_len.skel.h"
 #include "verifier_helper_packet_access.skel.h"
@@ -133,6 +134,7 @@ void test_verifier_direct_packet_access(void) { RUN(verifier_direct_packet_acces
 void test_verifier_direct_stack_access_wraparound(void) { RUN(verifier_direct_stack_access_wraparound); }
 void test_verifier_div0(void)                 { RUN(verifier_div0); }
 void test_verifier_div_overflow(void)         { RUN(verifier_div_overflow); }
+void test_verifier_global_subprogs(void)      { RUN(verifier_global_subprogs); }
 void test_verifier_gotol(void)                { RUN(verifier_gotol); }
 void test_verifier_helper_access_var_len(void) { RUN(verifier_helper_access_var_len); }
 void test_verifier_helper_packet_access(void) { RUN(verifier_helper_packet_access); }
diff --git a/tools/testing/selftests/bpf/progs/verifier_global_subprogs.c b/tools/testing/selftests/bpf/progs/verifier_global_subprogs.c
new file mode 100644
index 000000000000..a0a5efd1caa1
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/verifier_global_subprogs.c
@@ -0,0 +1,92 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2023 Meta Platforms, Inc. and affiliates. */
+
+#include <stdbool.h>
+#include <errno.h>
+#include <string.h>
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+#include "bpf_misc.h"
+
+int arr[1];
+int unkn_idx;
+
+__noinline long global_bad(void)
+{
+	return arr[unkn_idx]; /* BOOM */
+}
+
+__noinline long global_good(void)
+{
+	return arr[0];
+}
+
+__noinline long global_calls_bad(void)
+{
+	return global_good() + global_bad() /* does BOOM indirectly */;
+}
+
+__noinline long global_calls_good_only(void)
+{
+	return global_good();
+}
+
+SEC("?raw_tp")
+__success __log_level(2)
+/* main prog is validated completely first */
+__msg("('global_calls_good_only') is global and assumed valid.")
+__msg("1: (95) exit")
+/* eventually global_good() is transitively validated as well */
+__msg("Validating global_good() func")
+__msg("('global_good') is safe for any args that match its prototype")
+int chained_global_func_calls_success(void)
+{
+	return global_calls_good_only();
+}
+
+SEC("?raw_tp")
+__failure __log_level(2)
+/* main prog validated successfully first */
+__msg("1: (95) exit")
+/* eventually we validate global_bad() and fail */
+__msg("Validating global_bad() func")
+__msg("math between map_value pointer and register") /* BOOM */
+int chained_global_func_calls_bad(void)
+{
+	return global_calls_bad();
+}
+
+/* do out of bounds access forcing verifier to fail verification if this
+ * global func is called
+ */
+__noinline int global_unsupp(const int *mem)
+{
+	if (!mem)
+		return 0;
+	return mem[100]; /* BOOM */
+}
+
+const volatile bool skip_unsupp_global = true;
+
+SEC("?raw_tp")
+__success
+int guarded_unsupp_global_called(void)
+{
+	if (!skip_unsupp_global)
+		return global_unsupp(NULL);
+	return 0;
+}
+
+SEC("?raw_tp")
+__failure __log_level(2)
+__msg("Func#1 ('global_unsupp') is global and assumed valid.")
+__msg("Validating global_unsupp() func#1...")
+__msg("value is outside of the allowed memory range")
+int unguarded_unsupp_global_called(void)
+{
+	int x = 0;
+
+	return global_unsupp(&x);
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.34.1

Re: [PATCH bpf-next 3/3] selftests/bpf: add lazy global subprog validation tests

[Eduard Zingerman]

On Wed, 2023-11-22 at 13:31 -0800, Andrii Nakryiko wrote:
> Add a few test that validate BPF verifier's lazy approach to validating
> global subprogs.
> 
> We check that global subprogs that are called transitively through
> another global subprog is validated.
> 
> We also check that invalid global subprog is not validated, if it's not
> called from the main program.
> 
> And we also check that main program is always validated first, before
> any of the subprogs.
> 
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> ---

Acked-by: Eduard Zingerman <eddyz87@gmail.com>

Re: [PATCH bpf-next 0/3] Verify global subprogs lazily

[Daniel Borkmann]

On 11/22/23 10:31 PM, Andrii Nakryiko wrote:
> See patch #2 for justification. In few words, current eager verification of
> global func prevents BPF CO-RE approaches to be applied to global functions.
> 
> Patch #1 is just a nicety to emit global subprog names in verifier logs.
> 
> Patch #3 adds selftests validating new lazy semantics.
> 
> Andrii Nakryiko (3):
>    bpf: emit global subprog name in verifier logs
>    bpf: validate global subprogs lazily
>    selftests/bpf: add lazy global subprog validation tests
> 
>   include/linux/bpf.h                           |  2 +
>   kernel/bpf/verifier.c                         | 88 ++++++++++++++----
>   .../selftests/bpf/prog_tests/verifier.c       |  2 +
>   .../selftests/bpf/progs/test_global_func12.c  |  4 +-
>   .../bpf/progs/verifier_global_subprogs.c      | 92 +++++++++++++++++++
>   .../bpf/progs/verifier_subprog_precision.c    |  4 +-
>   6 files changed, 168 insertions(+), 24 deletions(-)
>   create mode 100644 tools/testing/selftests/bpf/progs/verifier_global_subprogs.c

Series looks good to me, ACK. It needs a rebase however after the fast
forward of the bpf-next tree from today.

 > With BPF CO-RE approach, the natural way would be to still compile BPF
 > object file once and guard calls to this global subprog with some CO-RE
 > check or using .rodata variables. That's what people do to guard usage
 > of new helpers or kfuncs, and any other new BPF-side feature that might
 > be missing on old kernels.

I was wondering for selftests, could we also add something similar to the
above to guard calls via co-re? Just to have this use-case covered in CI.
Also perhaps one global_bad function which is dead-code would be nice to
have.

Thanks,
Daniel

Re: [PATCH bpf-next 0/3] Verify global subprogs lazily

[Andrii Nakryiko]

On Thu, Nov 23, 2023 at 2:48 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 11/22/23 10:31 PM, Andrii Nakryiko wrote:
> > See patch #2 for justification. In few words, current eager verification of
> > global func prevents BPF CO-RE approaches to be applied to global functions.
> >
> > Patch #1 is just a nicety to emit global subprog names in verifier logs.
> >
> > Patch #3 adds selftests validating new lazy semantics.
> >
> > Andrii Nakryiko (3):
> >    bpf: emit global subprog name in verifier logs
> >    bpf: validate global subprogs lazily
> >    selftests/bpf: add lazy global subprog validation tests
> >
> >   include/linux/bpf.h                           |  2 +
> >   kernel/bpf/verifier.c                         | 88 ++++++++++++++----
> >   .../selftests/bpf/prog_tests/verifier.c       |  2 +
> >   .../selftests/bpf/progs/test_global_func12.c  |  4 +-
> >   .../bpf/progs/verifier_global_subprogs.c      | 92 +++++++++++++++++++
> >   .../bpf/progs/verifier_subprog_precision.c    |  4 +-
> >   6 files changed, 168 insertions(+), 24 deletions(-)
> >   create mode 100644 tools/testing/selftests/bpf/progs/verifier_global_subprogs.c
>
> Series looks good to me, ACK. It needs a rebase however after the fast
> forward of the bpf-next tree from today.

Sure, no problem.

>
>  > With BPF CO-RE approach, the natural way would be to still compile BPF
>  > object file once and guard calls to this global subprog with some CO-RE
>  > check or using .rodata variables. That's what people do to guard usage
>  > of new helpers or kfuncs, and any other new BPF-side feature that might
>  > be missing on old kernels.
>
> I was wondering for selftests, could we also add something similar to the
> above to guard calls via co-re? Just to have this use-case covered in CI.
> Also perhaps one global_bad function which is dead-code would be nice to
> have.
>

We already have that in patch #3. See `const volatile bool
skip_unsupp_global = true;` and then

if (!skip_unsupp_global)
    return global_unsupp(NULL);

Because skip_unsupp_global is true, we won't call global_unsupp() from
that main program and it will be dead-code eliminated.

This `const volatile` value can actually be set to false before
loading BPF program, though, and in that case global function will be
actually callable.

Keep in mind that each main (entry) BPF program gets its own copies of
each subprogram (global or static, doesn't matter). So global_bad() is
dead-code for guarded_unsupp_global_called main program, but is not a
dead-code for unguarded_unsupp_global_called prog, in which we
unconditionally call global_bad().

So this should cover all the use cases, I think.


> Thanks,
> Daniel
>