* [PATCH bpf-next v3 0/2] bpf: Allow void return type for global subprogs
@ 2026-02-23 21:50 Emil Tsalapatis
2026-02-23 21:50 ` [PATCH bpf-next v3 1/2] bpf: Allow void global functions in the verifier Emil Tsalapatis
2026-02-23 21:50 ` [PATCH bpf-next v3 2/2] selftests: bpf: Add tests for void global subprogs Emil Tsalapatis
0 siblings, 2 replies; 10+ messages in thread
From: Emil Tsalapatis @ 2026-02-23 21:50 UTC (permalink / raw)
To: bpf
Cc: andrii, ast, daniel, eddyz87, martin.lau, memxor, song,
yonghong.song, Emil Tsalapatis
Global subprogs are currently not allowed to return void. Adjust
verifier logic to allow global functions with a void return type.
Exception callbacks are excluded from this change, and still require
a scalar return type.
Also adjust existing selftests that were ensuring that global subprogs
with void return types fail to verify. These tests should now load
successfully.
v2 -> v3
(https://lore.kernel.org/bpf/20260210183257.217285-1-emil@etsalapatis.com/)
- Allow freplace for global programs that return void and add selftests (Kumar)
v1 -> v2
(https://lore.kernel.org/bpf/20260127231414.359283-1-emil@etsalapatis.com/)
- Only mark R0 as valid for non-void global functions (AI)
- Rename subprog_is_void to subprog_returns_void (Kumar)
- Add test that R0 is invalid after calling a void global (Alexei)
- Add check to ensure exception callbacks do not return void
and associated selftest
Cc: Kumar Kartikeya Dwivedi <memxor@gmail.com>
Emil Tsalapatis (2):
bpf: Allow void global functions in the verifier
selftests: bpf: Add tests for void global subprogs
kernel/bpf/btf.c | 7 ++-
kernel/bpf/verifier.c | 59 +++++++++++++++++--
.../selftests/bpf/prog_tests/fexit_bpf2bpf.c | 24 ++++++++
.../bpf/prog_tests/test_global_funcs.c | 2 +
.../selftests/bpf/progs/exceptions_fail.c | 19 +++++-
.../bpf/progs/freplace_int_with_void.c | 14 +++++
.../selftests/bpf/progs/freplace_void.c | 14 +++++
.../selftests/bpf/progs/test_global_func18.c | 23 ++++++++
.../selftests/bpf/progs/test_global_func7.c | 2 +-
9 files changed, 151 insertions(+), 13 deletions(-)
create mode 100644 tools/testing/selftests/bpf/progs/freplace_int_with_void.c
create mode 100644 tools/testing/selftests/bpf/progs/freplace_void.c
create mode 100644 tools/testing/selftests/bpf/progs/test_global_func18.c
--
2.49.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH bpf-next v3 1/2] bpf: Allow void global functions in the verifier
2026-02-23 21:50 [PATCH bpf-next v3 0/2] bpf: Allow void return type for global subprogs Emil Tsalapatis
@ 2026-02-23 21:50 ` Emil Tsalapatis
2026-02-24 0:09 ` Eduard Zingerman
2026-02-23 21:50 ` [PATCH bpf-next v3 2/2] selftests: bpf: Add tests for void global subprogs Emil Tsalapatis
1 sibling, 1 reply; 10+ messages in thread
From: Emil Tsalapatis @ 2026-02-23 21:50 UTC (permalink / raw)
To: bpf
Cc: andrii, ast, daniel, eddyz87, martin.lau, memxor, song,
yonghong.song, Emil Tsalapatis
Global subprogs are currently not allowed to return void. Adjust
verifier logic to allow global functions with a void return type.
Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
---
kernel/bpf/btf.c | 7 ++-
kernel/bpf/verifier.c | 59 +++++++++++++++++--
.../selftests/bpf/progs/exceptions_fail.c | 2 +-
.../selftests/bpf/progs/test_global_func7.c | 2 +-
4 files changed, 59 insertions(+), 11 deletions(-)
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 7708958e3fb8..51f8d3dece5d 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -7836,15 +7836,16 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog)
tname, nargs, MAX_BPF_FUNC_REG_ARGS);
return -EINVAL;
}
- /* check that function returns int, exception cb also requires this */
+ /* check that function is void or returns int, exception cb also requires this */
t = btf_type_by_id(btf, t->type);
while (btf_type_is_modifier(t))
t = btf_type_by_id(btf, t->type);
- if (!btf_type_is_int(t) && !btf_is_any_enum(t)) {
+ if (!btf_type_is_void(t) && !btf_type_is_int(t) && !btf_is_any_enum(t)) {
if (!is_global)
return -EINVAL;
bpf_log(log,
- "Global function %s() doesn't return scalar. Only those are supported.\n",
+ "Global function %s() return value not void or scalar. "
+ "Only those are supported.\n",
tname);
return -EINVAL;
}
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 0162f946032f..e997c3776fa7 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -444,6 +444,29 @@ static bool subprog_is_global(const struct bpf_verifier_env *env, int subprog)
return aux && aux[subprog].linkage == BTF_FUNC_GLOBAL;
}
+static bool subprog_returns_void(struct bpf_verifier_env *env, int subprog)
+{
+ const struct btf_type *type, *func, *func_proto;
+ const struct btf *btf = env->prog->aux->btf;
+ u32 btf_id;
+
+ btf_id = env->prog->aux->func_info[subprog].type_id;
+
+ func = btf_type_by_id(btf, btf_id);
+ if (verifier_bug_if(!func, env, "btf_id %u not found", btf_id))
+ return false;
+
+ func_proto = btf_type_by_id(btf, func->type);
+ if (verifier_bug_if(!func_proto, env, "btf_id %u not found", func->type))
+ return false;
+
+ type = btf_type_skip_modifiers(btf, func_proto->type, NULL);
+ if (verifier_bug_if(!type, env, "btf_id %u not found", func_proto->type))
+ return false;
+
+ return btf_type_is_void(type);
+}
+
static const char *subprog_name(const struct bpf_verifier_env *env, int subprog)
{
struct bpf_func_info *info;
@@ -10840,9 +10863,11 @@ static int check_func_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
subprog_aux(env, subprog)->called = true;
clear_caller_saved_regs(env, caller->regs);
- /* All global functions return a 64-bit SCALAR_VALUE */
- mark_reg_unknown(env, caller->regs, BPF_REG_0);
- caller->regs[BPF_REG_0].subreg_def = DEF_NOT_SUBREG;
+ /* All non-void global functions return a 64-bit SCALAR_VALUE. */
+ if (!subprog_returns_void(env, subprog)) {
+ mark_reg_unknown(env, caller->regs, BPF_REG_0);
+ caller->regs[BPF_REG_0].subreg_def = DEF_NOT_SUBREG;
+ }
/* continue with next insn after call */
return 0;
@@ -17812,6 +17837,16 @@ static int check_return_code(struct bpf_verifier_env *env, int regno, const char
/* LSM and struct_ops func-ptr's return type could be "void" */
if (!is_subprog || frame->in_exception_callback_fn) {
+
+ /*
+ * If the actual program is an extension, let it
+ * return void - attaching will succeed only if the
+ * program being replaced also returns void, and since
+ * it has passed verification its actual type doesn't matter.
+ */
+ if (env->prog->type == BPF_PROG_TYPE_EXT && subprog_returns_void(env, frame->subprogno))
+ return 0;
+
switch (prog_type) {
case BPF_PROG_TYPE_LSM:
if (prog->expected_attach_type == BPF_LSM_CGROUP)
@@ -17841,6 +17876,10 @@ static int check_return_code(struct bpf_verifier_env *env, int regno, const char
default:
break;
}
+ } else {
+ /* If this is a void global subprog, there is no return value. */
+ if (subprog_is_global(env, frame->subprogno) && subprog_returns_void(env, frame->subprogno))
+ return 0;
}
/* eBPF calling convention is such that R0 is used
@@ -24452,10 +24491,18 @@ static int do_check_common(struct bpf_verifier_env *env, int subprog)
if (subprog_is_exc_cb(env, subprog)) {
state->frame[0]->in_exception_callback_fn = true;
- /* We have already ensured that the callback returns an integer, just
- * like all global subprogs. We need to determine it only has a single
- * scalar argument.
+
+ /*
+ * Global functions are scalar or void, make sure
+ * we return a scalar.
*/
+ if (subprog_returns_void(env, subprog)) {
+ verbose(env, "exception cb cannot return void\n");
+ ret = -EINVAL;
+ goto out;
+ }
+
+ /* Also ensure the callback only has a single scalar argument. */
if (sub->arg_cnt != 1 || sub->args[0].arg_type != ARG_ANYTHING) {
verbose(env, "exception cb only supports single integer argument\n");
ret = -EINVAL;
diff --git a/tools/testing/selftests/bpf/progs/exceptions_fail.c b/tools/testing/selftests/bpf/progs/exceptions_fail.c
index 8a0fdff89927..d28ecc4ee2d0 100644
--- a/tools/testing/selftests/bpf/progs/exceptions_fail.c
+++ b/tools/testing/selftests/bpf/progs/exceptions_fail.c
@@ -51,7 +51,7 @@ __noinline int exception_cb_ok_arg_small(int a)
SEC("?tc")
__exception_cb(exception_cb_bad_ret_type)
-__failure __msg("Global function exception_cb_bad_ret_type() doesn't return scalar.")
+__failure __msg("Global function exception_cb_bad_ret_type() return value not void or scalar.")
int reject_exception_cb_type_1(struct __sk_buff *ctx)
{
bpf_throw(0);
diff --git a/tools/testing/selftests/bpf/progs/test_global_func7.c b/tools/testing/selftests/bpf/progs/test_global_func7.c
index f182febfde3c..9e59625c1c92 100644
--- a/tools/testing/selftests/bpf/progs/test_global_func7.c
+++ b/tools/testing/selftests/bpf/progs/test_global_func7.c
@@ -12,7 +12,7 @@ void foo(struct __sk_buff *skb)
}
SEC("tc")
-__failure __msg("foo() doesn't return scalar")
+__success
int global_func7(struct __sk_buff *skb)
{
foo(skb);
--
2.49.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH bpf-next v3 2/2] selftests: bpf: Add tests for void global subprogs
2026-02-23 21:50 [PATCH bpf-next v3 0/2] bpf: Allow void return type for global subprogs Emil Tsalapatis
2026-02-23 21:50 ` [PATCH bpf-next v3 1/2] bpf: Allow void global functions in the verifier Emil Tsalapatis
@ 2026-02-23 21:50 ` Emil Tsalapatis
2026-02-24 0:24 ` Eduard Zingerman
1 sibling, 1 reply; 10+ messages in thread
From: Emil Tsalapatis @ 2026-02-23 21:50 UTC (permalink / raw)
To: bpf
Cc: andrii, ast, daniel, eddyz87, martin.lau, memxor, song,
yonghong.song, Emil Tsalapatis
Add additional testing for void global functions. The tests
ensure that calls to void global functions properly keep
R0 invalid. Also make sure that exception callbacks still
require a return value.
Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
---
.../selftests/bpf/prog_tests/fexit_bpf2bpf.c | 24 +++++++++++++++++++
.../bpf/prog_tests/test_global_funcs.c | 2 ++
.../selftests/bpf/progs/exceptions_fail.c | 19 ++++++++++++---
.../bpf/progs/freplace_int_with_void.c | 14 +++++++++++
.../selftests/bpf/progs/freplace_void.c | 14 +++++++++++
.../selftests/bpf/progs/test_global_func18.c | 23 ++++++++++++++++++
6 files changed, 93 insertions(+), 3 deletions(-)
create mode 100644 tools/testing/selftests/bpf/progs/freplace_int_with_void.c
create mode 100644 tools/testing/selftests/bpf/progs/freplace_void.c
create mode 100644 tools/testing/selftests/bpf/progs/test_global_func18.c
diff --git a/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c b/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c
index f29fc789c14b..23d933f1aec6 100644
--- a/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c
+++ b/tools/testing/selftests/bpf/prog_tests/fexit_bpf2bpf.c
@@ -347,6 +347,17 @@ static void test_func_sockmap_update(void)
prog_name, false, NULL);
}
+static void test_func_replace_void(void)
+{
+ const char *prog_name[] = {
+ "freplace/foo",
+ };
+ test_fexit_bpf2bpf_common("./freplace_void.bpf.o",
+ "./test_global_func7.bpf.o",
+ ARRAY_SIZE(prog_name),
+ prog_name, false, NULL);
+}
+
static void test_obj_load_failure_common(const char *obj_file,
const char *target_obj_file,
const char *exp_msg)
@@ -432,6 +443,15 @@ static void test_func_replace_global_func(void)
prog_name, false, NULL);
}
+static void test_func_replace_int_with_void(void)
+{
+ /* Make sure we can't freplace with the wrong type */
+ test_obj_load_failure_common("freplace_int_with_void.bpf.o",
+ "./test_global_func2.bpf.o",
+ "Return type UNKNOWN of test_freplace_int_with_void()"
+ " doesn't match type INT of global_func2()");
+}
+
static int find_prog_btf_id(const char *name, __u32 attach_prog_fd)
{
struct bpf_prog_info info = {};
@@ -597,4 +617,8 @@ void serial_test_fexit_bpf2bpf(void)
test_fentry_to_cgroup_bpf();
if (test__start_subtest("func_replace_progmap"))
test_func_replace_progmap();
+ if (test__start_subtest("freplace_int_with_void"))
+ test_func_replace_int_with_void();
+ if (test__start_subtest("freplace_void"))
+ test_func_replace_void();
}
diff --git a/tools/testing/selftests/bpf/prog_tests/test_global_funcs.c b/tools/testing/selftests/bpf/prog_tests/test_global_funcs.c
index e905cbaf6b3d..45854cbcbfcf 100644
--- a/tools/testing/selftests/bpf/prog_tests/test_global_funcs.c
+++ b/tools/testing/selftests/bpf/prog_tests/test_global_funcs.c
@@ -18,6 +18,7 @@
#include "test_global_func15.skel.h"
#include "test_global_func16.skel.h"
#include "test_global_func17.skel.h"
+#include "test_global_func18.skel.h"
#include "test_global_func_ctx_args.skel.h"
#include "bpf/libbpf_internal.h"
@@ -155,6 +156,7 @@ void test_test_global_funcs(void)
RUN_TESTS(test_global_func15);
RUN_TESTS(test_global_func16);
RUN_TESTS(test_global_func17);
+ RUN_TESTS(test_global_func18);
RUN_TESTS(test_global_func_ctx_args);
if (test__start_subtest("ctx_arg_rewrite"))
diff --git a/tools/testing/selftests/bpf/progs/exceptions_fail.c b/tools/testing/selftests/bpf/progs/exceptions_fail.c
index d28ecc4ee2d0..691c16001952 100644
--- a/tools/testing/selftests/bpf/progs/exceptions_fail.c
+++ b/tools/testing/selftests/bpf/progs/exceptions_fail.c
@@ -29,11 +29,15 @@ struct {
private(A) struct bpf_spin_lock lock;
private(A) struct bpf_rb_root rbtree __contains(foo, node);
-__noinline void *exception_cb_bad_ret_type(u64 cookie)
+__noinline void *exception_cb_bad_ret_type1(u64 cookie)
{
return NULL;
}
+__noinline void exception_cb_bad_ret_type2(u64 cookie)
+{
+}
+
__noinline int exception_cb_bad_arg_0(void)
{
return 0;
@@ -50,8 +54,8 @@ __noinline int exception_cb_ok_arg_small(int a)
}
SEC("?tc")
-__exception_cb(exception_cb_bad_ret_type)
-__failure __msg("Global function exception_cb_bad_ret_type() return value not void or scalar.")
+__exception_cb(exception_cb_bad_ret_type1)
+__failure __msg("Global function exception_cb_bad_ret_type1() return value not void or scalar.")
int reject_exception_cb_type_1(struct __sk_buff *ctx)
{
bpf_throw(0);
@@ -85,6 +89,15 @@ int reject_exception_cb_type_4(struct __sk_buff *ctx)
return 0;
}
+SEC("?tc")
+__exception_cb(exception_cb_bad_ret_type2)
+__failure __msg("exception cb cannot return void")
+int reject_exception_cb_type_5(struct __sk_buff *ctx)
+{
+ bpf_throw(0);
+ return 0;
+}
+
__noinline
static int timer_cb(void *map, int *key, struct bpf_timer *timer)
{
diff --git a/tools/testing/selftests/bpf/progs/freplace_int_with_void.c b/tools/testing/selftests/bpf/progs/freplace_int_with_void.c
new file mode 100644
index 000000000000..8b6f5776cf0f
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/freplace_int_with_void.c
@@ -0,0 +1,14 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/bpf.h>
+#include <linux/pkt_cls.h>
+#include <bpf/bpf_helpers.h>
+
+volatile int data;
+
+SEC("freplace/global_func2")
+void test_freplace_int_with_void(struct __sk_buff *skb)
+{
+ data = 1;
+}
+
+char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/progs/freplace_void.c b/tools/testing/selftests/bpf/progs/freplace_void.c
new file mode 100644
index 000000000000..76707b29dc6a
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/freplace_void.c
@@ -0,0 +1,14 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+
+volatile int data;
+
+SEC("freplace/foo")
+__weak
+void test_freplace_void(struct __sk_buff *skb)
+{
+ data = 1;
+}
+
+char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/progs/test_global_func18.c b/tools/testing/selftests/bpf/progs/test_global_func18.c
new file mode 100644
index 000000000000..3dafb0dc2342
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_global_func18.c
@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <vmlinux.h>
+#include <bpf/bpf_helpers.h>
+#include "bpf_misc.h"
+
+__weak
+void foo(void)
+{
+}
+
+SEC("tc")
+__failure __msg("!read_ok")
+int global_func18(struct __sk_buff *skb)
+{
+ foo();
+
+ asm volatile(
+ "r1 = r0;"
+ :::
+ );
+
+ return 0;
+}
--
2.49.0
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH bpf-next v3 1/2] bpf: Allow void global functions in the verifier
2026-02-23 21:50 ` [PATCH bpf-next v3 1/2] bpf: Allow void global functions in the verifier Emil Tsalapatis
@ 2026-02-24 0:09 ` Eduard Zingerman
2026-02-24 18:46 ` Emil Tsalapatis
0 siblings, 1 reply; 10+ messages in thread
From: Eduard Zingerman @ 2026-02-24 0:09 UTC (permalink / raw)
To: Emil Tsalapatis, bpf
Cc: andrii, ast, daniel, martin.lau, memxor, song, yonghong.song
On Mon, 2026-02-23 at 16:50 -0500, Emil Tsalapatis wrote:
[...]
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 0162f946032f..e997c3776fa7 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -444,6 +444,29 @@ static bool subprog_is_global(const struct bpf_verifier_env *env, int subprog)
> return aux && aux[subprog].linkage == BTF_FUNC_GLOBAL;
> }
>
> +static bool subprog_returns_void(struct bpf_verifier_env *env, int subprog)
> +{
> + const struct btf_type *type, *func, *func_proto;
> + const struct btf *btf = env->prog->aux->btf;
> + u32 btf_id;
> +
> + btf_id = env->prog->aux->func_info[subprog].type_id;
> +
> + func = btf_type_by_id(btf, btf_id);
> + if (verifier_bug_if(!func, env, "btf_id %u not found", btf_id))
> + return false;
> +
> + func_proto = btf_type_by_id(btf, func->type);
> + if (verifier_bug_if(!func_proto, env, "btf_id %u not found", func->type))
> + return false;
> +
> + type = btf_type_skip_modifiers(btf, func_proto->type, NULL);
> + if (verifier_bug_if(!type, env, "btf_id %u not found", func_proto->type))
> + return false;
> +
Nit: I there there are a few unnecessary 'verifier_bug_if()' checks here,
e.g. btf.c:btf_check_all_types() guarantees that func->type and func_proto->type
would be valid.
> + return btf_type_is_void(type);
> +}
> +
> static const char *subprog_name(const struct bpf_verifier_env *env, int subprog)
> {
> struct bpf_func_info *info;
[...]
> @@ -17812,6 +17837,16 @@ static int check_return_code(struct bpf_verifier_env *env, int regno, const char
>
> /* LSM and struct_ops func-ptr's return type could be "void" */
> if (!is_subprog || frame->in_exception_callback_fn) {
> +
> + /*
> + * If the actual program is an extension, let it
> + * return void - attaching will succeed only if the
> + * program being replaced also returns void, and since
> + * it has passed verification its actual type doesn't matter.
> + */
> + if (env->prog->type == BPF_PROG_TYPE_EXT && subprog_returns_void(env, frame->subprogno))
> + return 0;
> +
> switch (prog_type) {
> case BPF_PROG_TYPE_LSM:
> if (prog->expected_attach_type == BPF_LSM_CGROUP)
> @@ -17841,6 +17876,10 @@ static int check_return_code(struct bpf_verifier_env *env, int regno, const char
> default:
> break;
> }
> + } else {
> + /* If this is a void global subprog, there is no return value. */
> + if (subprog_is_global(env, frame->subprogno) && subprog_returns_void(env, frame->subprogno))
> + return 0;
Suppose a global subprogram is verified and it calls bpf_throw().
check_return_code() is called from check_kfunc_call() in such case
with R1 as a parameter. This check acts on the return type of the
program, will it miss proper return value check for the program?
> }
>
> /* eBPF calling convention is such that R0 is used
[...]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH bpf-next v3 2/2] selftests: bpf: Add tests for void global subprogs
2026-02-23 21:50 ` [PATCH bpf-next v3 2/2] selftests: bpf: Add tests for void global subprogs Emil Tsalapatis
@ 2026-02-24 0:24 ` Eduard Zingerman
0 siblings, 0 replies; 10+ messages in thread
From: Eduard Zingerman @ 2026-02-24 0:24 UTC (permalink / raw)
To: Emil Tsalapatis, bpf
Cc: andrii, ast, daniel, martin.lau, memxor, song, yonghong.song
On Mon, 2026-02-23 at 16:50 -0500, Emil Tsalapatis wrote:
[...]
> diff --git a/tools/testing/selftests/bpf/progs/freplace_void.c b/tools/testing/selftests/bpf/progs/freplace_void.c
> new file mode 100644
> index 000000000000..76707b29dc6a
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/freplace_void.c
> @@ -0,0 +1,14 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <linux/bpf.h>
> +#include <bpf/bpf_helpers.h>
> +
> +volatile int data;
> +
> +SEC("freplace/foo")
> +__weak
Nit: __weak not needed here.
> +void test_freplace_void(struct __sk_buff *skb)
> +{
> + data = 1;
Nit: 'data' is never validated, so there is no point in 'data'
definition and this statement.
> +}
> +
> +char _license[] SEC("license") = "GPL";
> diff --git a/tools/testing/selftests/bpf/progs/test_global_func18.c b/tools/testing/selftests/bpf/progs/test_global_func18.c
> new file mode 100644
> index 000000000000..3dafb0dc2342
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/test_global_func18.c
Nit: I'd put this to verifier_global_subprogs.c
> @@ -0,0 +1,23 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +#include <vmlinux.h>
> +#include <bpf/bpf_helpers.h>
> +#include "bpf_misc.h"
> +
> +__weak
> +void foo(void)
> +{
> +}
> +
> +SEC("tc")
> +__failure __msg("!read_ok")
> +int global_func18(struct __sk_buff *skb)
> +{
> + foo();
> +
> + asm volatile(
> + "r1 = r0;"
> + :::
> + );
> +
> + return 0;
> +}
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH bpf-next v3 1/2] bpf: Allow void global functions in the verifier
2026-02-24 0:09 ` Eduard Zingerman
@ 2026-02-24 18:46 ` Emil Tsalapatis
2026-02-24 19:02 ` Eduard Zingerman
0 siblings, 1 reply; 10+ messages in thread
From: Emil Tsalapatis @ 2026-02-24 18:46 UTC (permalink / raw)
To: Eduard Zingerman, bpf
Cc: andrii, ast, daniel, martin.lau, memxor, song, yonghong.song
On Mon Feb 23, 2026 at 7:09 PM EST, Eduard Zingerman wrote:
> On Mon, 2026-02-23 at 16:50 -0500, Emil Tsalapatis wrote:
Ack on the comments for 2/2. For here:
>
> [...]
>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index 0162f946032f..e997c3776fa7 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -444,6 +444,29 @@ static bool subprog_is_global(const struct bpf_verifier_env *env, int subprog)
>> return aux && aux[subprog].linkage == BTF_FUNC_GLOBAL;
>> }
>>
>> +static bool subprog_returns_void(struct bpf_verifier_env *env, int subprog)
>> +{
>> + const struct btf_type *type, *func, *func_proto;
>> + const struct btf *btf = env->prog->aux->btf;
>> + u32 btf_id;
>> +
>> + btf_id = env->prog->aux->func_info[subprog].type_id;
>> +
>> + func = btf_type_by_id(btf, btf_id);
>> + if (verifier_bug_if(!func, env, "btf_id %u not found", btf_id))
>> + return false;
>> +
>> + func_proto = btf_type_by_id(btf, func->type);
>> + if (verifier_bug_if(!func_proto, env, "btf_id %u not found", func->type))
>> + return false;
>> +
>> + type = btf_type_skip_modifiers(btf, func_proto->type, NULL);
>> + if (verifier_bug_if(!type, env, "btf_id %u not found", func_proto->type))
>> + return false;
>> +
>
> Nit: I there there are a few unnecessary 'verifier_bug_if()' checks here,
> e.g. btf.c:btf_check_all_types() guarantees that func->type and func_proto->type
> would be valid.
>
Ack, just to make sure I got it right at all the verifier_bug_if() are
unnecessary. unnecessary because the BTF is already checked. There's no way we
have an existing type with invalid fields. We also know the subprog btf
ID is valid from check_btf_func_early that happens way before
do_check_subprogs where subprog_returns_void is used.
>> + return btf_type_is_void(type);
>> +}
>> +
>> static const char *subprog_name(const struct bpf_verifier_env *env, int subprog)
>> {
>> struct bpf_func_info *info;
>
> [...]
>
>> @@ -17812,6 +17837,16 @@ static int check_return_code(struct bpf_verifier_env *env, int regno, const char
>>
>> /* LSM and struct_ops func-ptr's return type could be "void" */
>> if (!is_subprog || frame->in_exception_callback_fn) {
>> +
>> + /*
>> + * If the actual program is an extension, let it
>> + * return void - attaching will succeed only if the
>> + * program being replaced also returns void, and since
>> + * it has passed verification its actual type doesn't matter.
>> + */
>> + if (env->prog->type == BPF_PROG_TYPE_EXT && subprog_returns_void(env, frame->subprogno))
>> + return 0;
>> +
>> switch (prog_type) {
>> case BPF_PROG_TYPE_LSM:
>> if (prog->expected_attach_type == BPF_LSM_CGROUP)
>> @@ -17841,6 +17876,10 @@ static int check_return_code(struct bpf_verifier_env *env, int regno, const char
>> default:
>> break;
>> }
>> + } else {
>> + /* If this is a void global subprog, there is no return value. */
>> + if (subprog_is_global(env, frame->subprogno) && subprog_returns_void(env, frame->subprogno))
>> + return 0;
>
> Suppose a global subprogram is verified and it calls bpf_throw().
> check_return_code() is called from check_kfunc_call() in such case
> with R1 as a parameter. This check acts on the return type of the
> program, will it miss proper return value check for the program?
>
Due to the short-circuiting herea a void global program can bpf_throw()
with no issue. This is what we want, correct? The return type we check
in that case would always be that of the u64 cookie AFAICT.
>> }
>>
>> /* eBPF calling convention is such that R0 is used
>
> [...]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH bpf-next v3 1/2] bpf: Allow void global functions in the verifier
2026-02-24 18:46 ` Emil Tsalapatis
@ 2026-02-24 19:02 ` Eduard Zingerman
2026-02-24 19:53 ` Emil Tsalapatis
0 siblings, 1 reply; 10+ messages in thread
From: Eduard Zingerman @ 2026-02-24 19:02 UTC (permalink / raw)
To: Emil Tsalapatis, bpf
Cc: andrii, ast, daniel, martin.lau, memxor, song, yonghong.song
On Tue, 2026-02-24 at 13:46 -0500, Emil Tsalapatis wrote:
> On Mon Feb 23, 2026 at 7:09 PM EST, Eduard Zingerman wrote:
> > On Mon, 2026-02-23 at 16:50 -0500, Emil Tsalapatis wrote:
>
> Ack on the comments for 2/2. For here:
> >
> > [...]
> >
> > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > > index 0162f946032f..e997c3776fa7 100644
> > > --- a/kernel/bpf/verifier.c
> > > +++ b/kernel/bpf/verifier.c
> > > @@ -444,6 +444,29 @@ static bool subprog_is_global(const struct bpf_verifier_env *env, int subprog)
> > > return aux && aux[subprog].linkage == BTF_FUNC_GLOBAL;
> > > }
> > >
> > > +static bool subprog_returns_void(struct bpf_verifier_env *env, int subprog)
> > > +{
> > > + const struct btf_type *type, *func, *func_proto;
> > > + const struct btf *btf = env->prog->aux->btf;
> > > + u32 btf_id;
> > > +
> > > + btf_id = env->prog->aux->func_info[subprog].type_id;
> > > +
> > > + func = btf_type_by_id(btf, btf_id);
> > > + if (verifier_bug_if(!func, env, "btf_id %u not found", btf_id))
> > > + return false;
> > > +
> > > + func_proto = btf_type_by_id(btf, func->type);
> > > + if (verifier_bug_if(!func_proto, env, "btf_id %u not found", func->type))
> > > + return false;
> > > +
> > > + type = btf_type_skip_modifiers(btf, func_proto->type, NULL);
> > > + if (verifier_bug_if(!type, env, "btf_id %u not found", func_proto->type))
> > > + return false;
> > > +
> >
> > Nit: I there there are a few unnecessary 'verifier_bug_if()' checks here,
> > e.g. btf.c:btf_check_all_types() guarantees that func->type and func_proto->type
> > would be valid.
> >
>
> Ack, just to make sure I got it right at all the verifier_bug_if() are
> unnecessary. unnecessary because the BTF is already checked. There's no way we
> have an existing type with invalid fields. We also know the subprog btf
> ID is valid from check_btf_func_early that happens way before
> do_check_subprogs where subprog_returns_void is used.
Certainly not needed for 'func->type' and 'func_proto->type' access.
For 'func_info[subprog].type_id', idk -- depends on how defensive you
want to be, I'd skip it as well.
>
> > > + return btf_type_is_void(type);
> > > +}
> > > +
> > > static const char *subprog_name(const struct bpf_verifier_env *env, int subprog)
> > > {
> > > struct bpf_func_info *info;
> >
> > [...]
> >
> > > @@ -17812,6 +17837,16 @@ static int check_return_code(struct bpf_verifier_env *env, int regno, const char
> > >
> > > /* LSM and struct_ops func-ptr's return type could be "void" */
> > > if (!is_subprog || frame->in_exception_callback_fn) {
> > > +
> > > + /*
> > > + * If the actual program is an extension, let it
> > > + * return void - attaching will succeed only if the
> > > + * program being replaced also returns void, and since
> > > + * it has passed verification its actual type doesn't matter.
> > > + */
> > > + if (env->prog->type == BPF_PROG_TYPE_EXT && subprog_returns_void(env, frame->subprogno))
> > > + return 0;
> > > +
> > > switch (prog_type) {
> > > case BPF_PROG_TYPE_LSM:
> > > if (prog->expected_attach_type == BPF_LSM_CGROUP)
> > > @@ -17841,6 +17876,10 @@ static int check_return_code(struct bpf_verifier_env *env, int regno, const char
> > > default:
> > > break;
> > > }
> > > + } else {
> > > + /* If this is a void global subprog, there is no return value. */
> > > + if (subprog_is_global(env, frame->subprogno) && subprog_returns_void(env, frame->subprogno))
> > > + return 0;
> >
> > Suppose a global subprogram is verified and it calls bpf_throw().
> > check_return_code() is called from check_kfunc_call() in such case
> > with R1 as a parameter. This check acts on the return type of the
> > program, will it miss proper return value check for the program?
> >
>
> Due to the short-circuiting herea a void global program can bpf_throw()
> with no issue. This is what we want, correct? The return type we check
> in that case would always be that of the u64 cookie AFAICT.
I mean the following situation:
SEC(<I want some special return code>)
int foo(...) {
... bar(...) ...;
return ...;
}
// global func
void bar(...) {
... bpf_throw(<bad return code>) ...
}
In this case 'bar' would correspond to 'frame' in the check above and
<bad return code> won't be checked.
>
> > > }
> > >
> > > /* eBPF calling convention is such that R0 is used
> >
> > [...]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH bpf-next v3 1/2] bpf: Allow void global functions in the verifier
2026-02-24 19:02 ` Eduard Zingerman
@ 2026-02-24 19:53 ` Emil Tsalapatis
2026-02-24 20:33 ` Eduard Zingerman
0 siblings, 1 reply; 10+ messages in thread
From: Emil Tsalapatis @ 2026-02-24 19:53 UTC (permalink / raw)
To: Eduard Zingerman, bpf
Cc: andrii, ast, daniel, martin.lau, memxor, song, yonghong.song
On Tue Feb 24, 2026 at 2:02 PM EST, Eduard Zingerman wrote:
> On Tue, 2026-02-24 at 13:46 -0500, Emil Tsalapatis wrote:
>> On Mon Feb 23, 2026 at 7:09 PM EST, Eduard Zingerman wrote:
>> > On Mon, 2026-02-23 at 16:50 -0500, Emil Tsalapatis wrote:
>>
>> Ack on the comments for 2/2. For here:
>> >
>> > [...]
>> >
>> > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> > > index 0162f946032f..e997c3776fa7 100644
>> > > --- a/kernel/bpf/verifier.c
>> > > +++ b/kernel/bpf/verifier.c
>> > > @@ -444,6 +444,29 @@ static bool subprog_is_global(const struct bpf_verifier_env *env, int subprog)
>> > > return aux && aux[subprog].linkage == BTF_FUNC_GLOBAL;
>> > > }
>> > >
>> > > +static bool subprog_returns_void(struct bpf_verifier_env *env, int subprog)
>> > > +{
>> > > + const struct btf_type *type, *func, *func_proto;
>> > > + const struct btf *btf = env->prog->aux->btf;
>> > > + u32 btf_id;
>> > > +
>> > > + btf_id = env->prog->aux->func_info[subprog].type_id;
>> > > +
>> > > + func = btf_type_by_id(btf, btf_id);
>> > > + if (verifier_bug_if(!func, env, "btf_id %u not found", btf_id))
>> > > + return false;
>> > > +
>> > > + func_proto = btf_type_by_id(btf, func->type);
>> > > + if (verifier_bug_if(!func_proto, env, "btf_id %u not found", func->type))
>> > > + return false;
>> > > +
>> > > + type = btf_type_skip_modifiers(btf, func_proto->type, NULL);
>> > > + if (verifier_bug_if(!type, env, "btf_id %u not found", func_proto->type))
>> > > + return false;
>> > > +
>> >
>> > Nit: I there there are a few unnecessary 'verifier_bug_if()' checks here,
>> > e.g. btf.c:btf_check_all_types() guarantees that func->type and func_proto->type
>> > would be valid.
>> >
>>
>> Ack, just to make sure I got it right at all the verifier_bug_if() are
>> unnecessary. unnecessary because the BTF is already checked. There's no way we
>> have an existing type with invalid fields. We also know the subprog btf
>> ID is valid from check_btf_func_early that happens way before
>> do_check_subprogs where subprog_returns_void is used.
>
> Certainly not needed for 'func->type' and 'func_proto->type' access.
> For 'func_info[subprog].type_id', idk -- depends on how defensive you
> want to be, I'd skip it as well.
>
>>
>> > > + return btf_type_is_void(type);
>> > > +}
>> > > +
>> > > static const char *subprog_name(const struct bpf_verifier_env *env, int subprog)
>> > > {
>> > > struct bpf_func_info *info;
>> >
>> > [...]
>> >
>> > > @@ -17812,6 +17837,16 @@ static int check_return_code(struct bpf_verifier_env *env, int regno, const char
>> > >
>> > > /* LSM and struct_ops func-ptr's return type could be "void" */
>> > > if (!is_subprog || frame->in_exception_callback_fn) {
>> > > +
>> > > + /*
>> > > + * If the actual program is an extension, let it
>> > > + * return void - attaching will succeed only if the
>> > > + * program being replaced also returns void, and since
>> > > + * it has passed verification its actual type doesn't matter.
>> > > + */
>> > > + if (env->prog->type == BPF_PROG_TYPE_EXT && subprog_returns_void(env, frame->subprogno))
>> > > + return 0;
>> > > +
>> > > switch (prog_type) {
>> > > case BPF_PROG_TYPE_LSM:
>> > > if (prog->expected_attach_type == BPF_LSM_CGROUP)
>> > > @@ -17841,6 +17876,10 @@ static int check_return_code(struct bpf_verifier_env *env, int regno, const char
>> > > default:
>> > > break;
>> > > }
>> > > + } else {
>> > > + /* If this is a void global subprog, there is no return value. */
>> > > + if (subprog_is_global(env, frame->subprogno) && subprog_returns_void(env, frame->subprogno))
>> > > + return 0;
>> >
>> > Suppose a global subprogram is verified and it calls bpf_throw().
>> > check_return_code() is called from check_kfunc_call() in such case
>> > with R1 as a parameter. This check acts on the return type of the
>> > program, will it miss proper return value check for the program?
>> >
>>
>> Due to the short-circuiting herea a void global program can bpf_throw()
>> with no issue. This is what we want, correct? The return type we check
>> in that case would always be that of the u64 cookie AFAICT.
>
> I mean the following situation:
>
> SEC(<I want some special return code>)
> int foo(...) {
> ... bar(...) ...;
> return ...;
> }
>
> // global func
> void bar(...) {
> ... bpf_throw(<bad return code>) ...
> }
>
> In this case 'bar' would correspond to 'frame' in the check above and
> <bad return code> won't be checked.
>
I see, yes this spuriously passes. Since the check_error_code only runs
for BPF_EXIT and bpf_throw, can we just check if we're inspecting
bpf_throw()'s return value like so?
bool is_bpf_throw = regno == BPF_REG_1;
...
if (subprog_is_global(env, frame->subprogno) && subprog_returns_void(env, frame->subprogno)
&& !is_bpf_throw)
...
We only need to use this check in two places in check_error_code
to solve the problem. With the fix we can also still throw from
void global() functions.
>>
>> > > }
>> > >
>> > > /* eBPF calling convention is such that R0 is used
>> >
>> > [...]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH bpf-next v3 1/2] bpf: Allow void global functions in the verifier
2026-02-24 19:53 ` Emil Tsalapatis
@ 2026-02-24 20:33 ` Eduard Zingerman
2026-02-25 0:55 ` Emil Tsalapatis
0 siblings, 1 reply; 10+ messages in thread
From: Eduard Zingerman @ 2026-02-24 20:33 UTC (permalink / raw)
To: Emil Tsalapatis, bpf
Cc: andrii, ast, daniel, martin.lau, memxor, song, yonghong.song
On Tue, 2026-02-24 at 14:53 -0500, Emil Tsalapatis wrote:
> On Tue Feb 24, 2026 at 2:02 PM EST, Eduard Zingerman wrote:
> > On Tue, 2026-02-24 at 13:46 -0500, Emil Tsalapatis wrote:
> > > On Mon Feb 23, 2026 at 7:09 PM EST, Eduard Zingerman wrote:
> > > > On Mon, 2026-02-23 at 16:50 -0500, Emil Tsalapatis wrote:
> > >
> > > Ack on the comments for 2/2. For here:
> > > >
> > > > [...]
> > > >
> > > > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > > > > index 0162f946032f..e997c3776fa7 100644
> > > > > --- a/kernel/bpf/verifier.c
> > > > > +++ b/kernel/bpf/verifier.c
> > > > > @@ -444,6 +444,29 @@ static bool subprog_is_global(const struct bpf_verifier_env *env, int subprog)
> > > > > return aux && aux[subprog].linkage == BTF_FUNC_GLOBAL;
> > > > > }
> > > > >
> > > > > +static bool subprog_returns_void(struct bpf_verifier_env *env, int subprog)
> > > > > +{
> > > > > + const struct btf_type *type, *func, *func_proto;
> > > > > + const struct btf *btf = env->prog->aux->btf;
> > > > > + u32 btf_id;
> > > > > +
> > > > > + btf_id = env->prog->aux->func_info[subprog].type_id;
> > > > > +
> > > > > + func = btf_type_by_id(btf, btf_id);
> > > > > + if (verifier_bug_if(!func, env, "btf_id %u not found", btf_id))
> > > > > + return false;
> > > > > +
> > > > > + func_proto = btf_type_by_id(btf, func->type);
> > > > > + if (verifier_bug_if(!func_proto, env, "btf_id %u not found", func->type))
> > > > > + return false;
> > > > > +
> > > > > + type = btf_type_skip_modifiers(btf, func_proto->type, NULL);
> > > > > + if (verifier_bug_if(!type, env, "btf_id %u not found", func_proto->type))
> > > > > + return false;
> > > > > +
> > > >
> > > > Nit: I there there are a few unnecessary 'verifier_bug_if()' checks here,
> > > > e.g. btf.c:btf_check_all_types() guarantees that func->type and func_proto->type
> > > > would be valid.
> > > >
> > >
> > > Ack, just to make sure I got it right at all the verifier_bug_if() are
> > > unnecessary. unnecessary because the BTF is already checked. There's no way we
> > > have an existing type with invalid fields. We also know the subprog btf
> > > ID is valid from check_btf_func_early that happens way before
> > > do_check_subprogs where subprog_returns_void is used.
> >
> > Certainly not needed for 'func->type' and 'func_proto->type' access.
> > For 'func_info[subprog].type_id', idk -- depends on how defensive you
> > want to be, I'd skip it as well.
> >
> > >
> > > > > + return btf_type_is_void(type);
> > > > > +}
> > > > > +
> > > > > static const char *subprog_name(const struct bpf_verifier_env *env, int subprog)
> > > > > {
> > > > > struct bpf_func_info *info;
> > > >
> > > > [...]
> > > >
> > > > > @@ -17812,6 +17837,16 @@ static int check_return_code(struct bpf_verifier_env *env, int regno, const char
> > > > >
> > > > > /* LSM and struct_ops func-ptr's return type could be "void" */
> > > > > if (!is_subprog || frame->in_exception_callback_fn) {
> > > > > +
> > > > > + /*
> > > > > + * If the actual program is an extension, let it
> > > > > + * return void - attaching will succeed only if the
> > > > > + * program being replaced also returns void, and since
> > > > > + * it has passed verification its actual type doesn't matter.
> > > > > + */
> > > > > + if (env->prog->type == BPF_PROG_TYPE_EXT && subprog_returns_void(env, frame->subprogno))
> > > > > + return 0;
> > > > > +
> > > > > switch (prog_type) {
> > > > > case BPF_PROG_TYPE_LSM:
> > > > > if (prog->expected_attach_type == BPF_LSM_CGROUP)
> > > > > @@ -17841,6 +17876,10 @@ static int check_return_code(struct bpf_verifier_env *env, int regno, const char
> > > > > default:
> > > > > break;
> > > > > }
> > > > > + } else {
> > > > > + /* If this is a void global subprog, there is no return value. */
> > > > > + if (subprog_is_global(env, frame->subprogno) && subprog_returns_void(env, frame->subprogno))
> > > > > + return 0;
> > > >
> > > > Suppose a global subprogram is verified and it calls bpf_throw().
> > > > check_return_code() is called from check_kfunc_call() in such case
> > > > with R1 as a parameter. This check acts on the return type of the
> > > > program, will it miss proper return value check for the program?
> > > >
> > >
> > > Due to the short-circuiting herea a void global program can bpf_throw()
> > > with no issue. This is what we want, correct? The return type we check
> > > in that case would always be that of the u64 cookie AFAICT.
> >
> > I mean the following situation:
> >
> > SEC(<I want some special return code>)
> > int foo(...) {
> > ... bar(...) ...;
> > return ...;
> > }
> >
> > // global func
> > void bar(...) {
> > ... bpf_throw(<bad return code>) ...
> > }
> >
> > In this case 'bar' would correspond to 'frame' in the check above and
> > <bad return code> won't be checked.
> >
>
> I see, yes this spuriously passes. Since the check_error_code only runs
> for BPF_EXIT and bpf_throw, can we just check if we're inspecting
> bpf_throw()'s return value like so?
>
> bool is_bpf_throw = regno == BPF_REG_1;
> ...
> if (subprog_is_global(env, frame->subprogno) && subprog_returns_void(env, frame->subprogno)
> && !is_bpf_throw)
> ...
>
> We only need to use this check in two places in check_error_code
> to solve the problem. With the fix we can also still throw from
> void global() functions.
Would it be possible to split check_return_code() in two:
- check_return_code() that checks program return code
- check_throw_return_code() with throw specific logic,
referring back to check_return_code() if necessary?
> > >
> > > > > }
> > > > >
> > > > > /* eBPF calling convention is such that R0 is used
> > > >
> > > > [...]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH bpf-next v3 1/2] bpf: Allow void global functions in the verifier
2026-02-24 20:33 ` Eduard Zingerman
@ 2026-02-25 0:55 ` Emil Tsalapatis
0 siblings, 0 replies; 10+ messages in thread
From: Emil Tsalapatis @ 2026-02-25 0:55 UTC (permalink / raw)
To: Eduard Zingerman, bpf
Cc: andrii, ast, daniel, martin.lau, memxor, song, yonghong.song
On Tue Feb 24, 2026 at 3:33 PM EST, Eduard Zingerman wrote:
> On Tue, 2026-02-24 at 14:53 -0500, Emil Tsalapatis wrote:
>> On Tue Feb 24, 2026 at 2:02 PM EST, Eduard Zingerman wrote:
>> > On Tue, 2026-02-24 at 13:46 -0500, Emil Tsalapatis wrote:
>> > > On Mon Feb 23, 2026 at 7:09 PM EST, Eduard Zingerman wrote:
>> > > > On Mon, 2026-02-23 at 16:50 -0500, Emil Tsalapatis wrote:
>> > >
>> > > Ack on the comments for 2/2. For here:
>> > > >
>> > > > [...]
>> > > >
>> > > > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> > > > > index 0162f946032f..e997c3776fa7 100644
>> > > > > --- a/kernel/bpf/verifier.c
>> > > > > +++ b/kernel/bpf/verifier.c
>> > > > > @@ -444,6 +444,29 @@ static bool subprog_is_global(const struct bpf_verifier_env *env, int subprog)
>> > > > > return aux && aux[subprog].linkage == BTF_FUNC_GLOBAL;
>> > > > > }
>> > > > >
>> > > > > +static bool subprog_returns_void(struct bpf_verifier_env *env, int subprog)
>> > > > > +{
>> > > > > + const struct btf_type *type, *func, *func_proto;
>> > > > > + const struct btf *btf = env->prog->aux->btf;
>> > > > > + u32 btf_id;
>> > > > > +
>> > > > > + btf_id = env->prog->aux->func_info[subprog].type_id;
>> > > > > +
>> > > > > + func = btf_type_by_id(btf, btf_id);
>> > > > > + if (verifier_bug_if(!func, env, "btf_id %u not found", btf_id))
>> > > > > + return false;
>> > > > > +
>> > > > > + func_proto = btf_type_by_id(btf, func->type);
>> > > > > + if (verifier_bug_if(!func_proto, env, "btf_id %u not found", func->type))
>> > > > > + return false;
>> > > > > +
>> > > > > + type = btf_type_skip_modifiers(btf, func_proto->type, NULL);
>> > > > > + if (verifier_bug_if(!type, env, "btf_id %u not found", func_proto->type))
>> > > > > + return false;
>> > > > > +
>> > > >
>> > > > Nit: I there there are a few unnecessary 'verifier_bug_if()' checks here,
>> > > > e.g. btf.c:btf_check_all_types() guarantees that func->type and func_proto->type
>> > > > would be valid.
>> > > >
>> > >
>> > > Ack, just to make sure I got it right at all the verifier_bug_if() are
>> > > unnecessary. unnecessary because the BTF is already checked. There's no way we
>> > > have an existing type with invalid fields. We also know the subprog btf
>> > > ID is valid from check_btf_func_early that happens way before
>> > > do_check_subprogs where subprog_returns_void is used.
>> >
>> > Certainly not needed for 'func->type' and 'func_proto->type' access.
>> > For 'func_info[subprog].type_id', idk -- depends on how defensive you
>> > want to be, I'd skip it as well.
>> >
>> > >
>> > > > > + return btf_type_is_void(type);
>> > > > > +}
>> > > > > +
>> > > > > static const char *subprog_name(const struct bpf_verifier_env *env, int subprog)
>> > > > > {
>> > > > > struct bpf_func_info *info;
>> > > >
>> > > > [...]
>> > > >
>> > > > > @@ -17812,6 +17837,16 @@ static int check_return_code(struct bpf_verifier_env *env, int regno, const char
>> > > > >
>> > > > > /* LSM and struct_ops func-ptr's return type could be "void" */
>> > > > > if (!is_subprog || frame->in_exception_callback_fn) {
>> > > > > +
>> > > > > + /*
>> > > > > + * If the actual program is an extension, let it
>> > > > > + * return void - attaching will succeed only if the
>> > > > > + * program being replaced also returns void, and since
>> > > > > + * it has passed verification its actual type doesn't matter.
>> > > > > + */
>> > > > > + if (env->prog->type == BPF_PROG_TYPE_EXT && subprog_returns_void(env, frame->subprogno))
>> > > > > + return 0;
>> > > > > +
>> > > > > switch (prog_type) {
>> > > > > case BPF_PROG_TYPE_LSM:
>> > > > > if (prog->expected_attach_type == BPF_LSM_CGROUP)
>> > > > > @@ -17841,6 +17876,10 @@ static int check_return_code(struct bpf_verifier_env *env, int regno, const char
>> > > > > default:
>> > > > > break;
>> > > > > }
>> > > > > + } else {
>> > > > > + /* If this is a void global subprog, there is no return value. */
>> > > > > + if (subprog_is_global(env, frame->subprogno) && subprog_returns_void(env, frame->subprogno))
>> > > > > + return 0;
>> > > >
>> > > > Suppose a global subprogram is verified and it calls bpf_throw().
>> > > > check_return_code() is called from check_kfunc_call() in such case
>> > > > with R1 as a parameter. This check acts on the return type of the
>> > > > program, will it miss proper return value check for the program?
>> > > >
>> > >
>> > > Due to the short-circuiting herea a void global program can bpf_throw()
>> > > with no issue. This is what we want, correct? The return type we check
>> > > in that case would always be that of the u64 cookie AFAICT.
>> >
>> > I mean the following situation:
>> >
>> > SEC(<I want some special return code>)
>> > int foo(...) {
>> > ... bar(...) ...;
>> > return ...;
>> > }
>> >
>> > // global func
>> > void bar(...) {
>> > ... bpf_throw(<bad return code>) ...
>> > }
>> >
>> > In this case 'bar' would correspond to 'frame' in the check above and
>> > <bad return code> won't be checked.
>> >
>>
>> I see, yes this spuriously passes. Since the check_error_code only runs
>> for BPF_EXIT and bpf_throw, can we just check if we're inspecting
>> bpf_throw()'s return value like so?
>>
>> bool is_bpf_throw = regno == BPF_REG_1;
>> ...
>> if (subprog_is_global(env, frame->subprogno) && subprog_returns_void(env, frame->subprogno)
>> && !is_bpf_throw)
>> ...
>>
>> We only need to use this check in two places in check_error_code
>> to solve the problem. With the fix we can also still throw from
>> void global() functions.
>
> Would it be possible to split check_return_code() in two:
> - check_return_code() that checks program return code
> - check_throw_return_code() with throw specific logic,
> referring back to check_return_code() if necessary?
>
I will do that, check_return_code will need some refactoring but
the overall code will be cleaner (e.g., the retval_range() calculation
can be factored out into a pure function). I will tack on the patches
with the non-functional changes at the front of this series.
>> > >
>> > > > > }
>> > > > >
>> > > > > /* eBPF calling convention is such that R0 is used
>> > > >
>> > > > [...]
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-02-25 0:55 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-23 21:50 [PATCH bpf-next v3 0/2] bpf: Allow void return type for global subprogs Emil Tsalapatis
2026-02-23 21:50 ` [PATCH bpf-next v3 1/2] bpf: Allow void global functions in the verifier Emil Tsalapatis
2026-02-24 0:09 ` Eduard Zingerman
2026-02-24 18:46 ` Emil Tsalapatis
2026-02-24 19:02 ` Eduard Zingerman
2026-02-24 19:53 ` Emil Tsalapatis
2026-02-24 20:33 ` Eduard Zingerman
2026-02-25 0:55 ` Emil Tsalapatis
2026-02-23 21:50 ` [PATCH bpf-next v3 2/2] selftests: bpf: Add tests for void global subprogs Emil Tsalapatis
2026-02-24 0:24 ` Eduard Zingerman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox