* [PATCH bpf-next v4 0/4] bpf: Allow void return type for global subprogs
@ 2026-02-25 3:33 Emil Tsalapatis
2026-02-25 3:33 ` [PATCH bpf-next v4 1/4] bpf: Factor out program return value calculation Emil Tsalapatis
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Emil Tsalapatis @ 2026-02-25 3:33 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.
Adding additional logic for void globals significantly complicates
check_return_code(), mainly because of how the new feature interacts
with bpf_throw(). To avoid complicating the code, refactor the return
value range validation logic into two separate functions, one for
program exits through BPF_EXIT and one for program exits through
bpf_throw(). The changes are non-functional.
Also adjust existing selftests that were ensuring that global subprogs
with void return types fail to verify. These tests should now load
successfully.
Patches 1-2 refactor check_return_code into two separate functions, one
for BPF_EXIT and one for bpf_throw(). No functional changes.
Patches 3-4 enable void globals in the verifier and add additional
selftests.
v3 -> v4
(https://lore.kernel.org/bpf/20260223215046.1706110-1-emil@etsalapatis.com/)
- Factor out bpf_throw() return value validation logic into its own
function (Eduard)
- Validate the range of bpf_throw() return values from void globals (Eduard)
- Move new selftest to verifier_global_subprogs.c (Eduard)
- Remove unnecessary verifier_bug_if() statements (Eduard)
- Add new test to ensure void globals can call bpf_throw
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 (4):
bpf: Factor out program return value calculation
bpf: Separate bpf_throw() and bpf_exit() return value validation
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 | 372 ++++++++++++------
.../selftests/bpf/prog_tests/exceptions.c | 1 +
.../selftests/bpf/prog_tests/fexit_bpf2bpf.c | 24 ++
.../testing/selftests/bpf/progs/exceptions.c | 14 +
.../selftests/bpf/progs/exceptions_fail.c | 35 +-
.../bpf/progs/freplace_int_with_void.c | 14 +
.../selftests/bpf/progs/freplace_void.c | 12 +
.../selftests/bpf/progs/test_global_func7.c | 2 +-
.../bpf/progs/verifier_global_subprogs.c | 19 +
10 files changed, 381 insertions(+), 119 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
--
2.49.0
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH bpf-next v4 1/4] bpf: Factor out program return value calculation
2026-02-25 3:33 [PATCH bpf-next v4 0/4] bpf: Allow void return type for global subprogs Emil Tsalapatis
@ 2026-02-25 3:33 ` Emil Tsalapatis
2026-02-25 23:31 ` Eduard Zingerman
2026-02-25 3:33 ` [PATCH bpf-next v4 2/4] bpf: Separate bpf_throw() and bpf_exit() return value validation Emil Tsalapatis
` (2 subsequent siblings)
3 siblings, 1 reply; 11+ messages in thread
From: Emil Tsalapatis @ 2026-02-25 3:33 UTC (permalink / raw)
To: bpf
Cc: andrii, ast, daniel, eddyz87, martin.lau, memxor, song,
yonghong.song, Emil Tsalapatis
Factor the return value range calculation logic in check_return_code
out of the function in preparation for separating the return value
validation logic for BPF_EXIT and bpf_throw().
Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
---
kernel/bpf/verifier.c | 205 +++++++++++++++++++++++-------------------
1 file changed, 114 insertions(+), 91 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index edf5342b982f..96ec27a36b32 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -17837,82 +17837,14 @@ static int check_ld_abs(struct bpf_verifier_env *env, struct bpf_insn *insn)
return 0;
}
-static int check_return_code(struct bpf_verifier_env *env, int regno, const char *reg_name)
+
+static int return_retval_range(struct bpf_verifier_env *env, struct bpf_retval_range *range,
+ bool *return_32bit)
{
- const char *exit_ctx = "At program exit";
- struct tnum enforce_attach_type_range = tnum_unknown;
- const struct bpf_prog *prog = env->prog;
- struct bpf_reg_state *reg = reg_state(env, regno);
- struct bpf_retval_range range = retval_range(0, 1);
enum bpf_prog_type prog_type = resolve_prog_type(env->prog);
- int err;
- struct bpf_func_state *frame = env->cur_state->frame[0];
- const bool is_subprog = frame->subprogno;
- bool return_32bit = false;
- const struct btf_type *reg_type, *ret_type = NULL;
-
- /* LSM and struct_ops func-ptr's return type could be "void" */
- if (!is_subprog || frame->in_exception_callback_fn) {
- switch (prog_type) {
- case BPF_PROG_TYPE_LSM:
- if (prog->expected_attach_type == BPF_LSM_CGROUP)
- /* See below, can be 0 or 0-1 depending on hook. */
- break;
- if (!prog->aux->attach_func_proto->type)
- return 0;
- break;
- case BPF_PROG_TYPE_STRUCT_OPS:
- if (!prog->aux->attach_func_proto->type)
- return 0;
-
- if (frame->in_exception_callback_fn)
- break;
-
- /* Allow a struct_ops program to return a referenced kptr if it
- * matches the operator's return type and is in its unmodified
- * form. A scalar zero (i.e., a null pointer) is also allowed.
- */
- reg_type = reg->btf ? btf_type_by_id(reg->btf, reg->btf_id) : NULL;
- ret_type = btf_type_resolve_ptr(prog->aux->attach_btf,
- prog->aux->attach_func_proto->type,
- NULL);
- if (ret_type && ret_type == reg_type && reg->ref_obj_id)
- return __check_ptr_off_reg(env, reg, regno, false);
- break;
- default:
- break;
- }
- }
- /* eBPF calling convention is such that R0 is used
- * to return the value from eBPF program.
- * Make sure that it's readable at this time
- * of bpf_exit, which means that program wrote
- * something into it earlier
- */
- err = check_reg_arg(env, regno, SRC_OP);
- if (err)
- return err;
-
- if (is_pointer_value(env, regno)) {
- verbose(env, "R%d leaks addr as return value\n", regno);
- return -EACCES;
- }
-
- if (frame->in_async_callback_fn) {
- exit_ctx = "At async callback return";
- range = frame->callback_ret_range;
- goto enforce_retval;
- }
-
- if (is_subprog && !frame->in_exception_callback_fn) {
- if (reg->type != SCALAR_VALUE) {
- verbose(env, "At subprogram exit the register R%d is not a scalar value (%s)\n",
- regno, reg_type_str(env, reg->type));
- return -EINVAL;
- }
- return 0;
- }
+ /* Default return value range. */
+ *range = retval_range(0, 1);
switch (prog_type) {
case BPF_PROG_TYPE_CGROUP_SOCK_ADDR:
@@ -17925,16 +17857,14 @@ static int check_return_code(struct bpf_verifier_env *env, int regno, const char
env->prog->expected_attach_type == BPF_CGROUP_INET4_GETSOCKNAME ||
env->prog->expected_attach_type == BPF_CGROUP_INET6_GETSOCKNAME ||
env->prog->expected_attach_type == BPF_CGROUP_UNIX_GETSOCKNAME)
- range = retval_range(1, 1);
+ *range = retval_range(1, 1);
if (env->prog->expected_attach_type == BPF_CGROUP_INET4_BIND ||
env->prog->expected_attach_type == BPF_CGROUP_INET6_BIND)
- range = retval_range(0, 3);
+ *range = retval_range(0, 3);
break;
case BPF_PROG_TYPE_CGROUP_SKB:
- if (env->prog->expected_attach_type == BPF_CGROUP_INET_EGRESS) {
- range = retval_range(0, 3);
- enforce_attach_type_range = tnum_range(2, 3);
- }
+ if (env->prog->expected_attach_type == BPF_CGROUP_INET_EGRESS)
+ *range = retval_range(0, 3);
break;
case BPF_PROG_TYPE_CGROUP_SOCK:
case BPF_PROG_TYPE_SOCK_OPS:
@@ -17945,14 +17875,14 @@ static int check_return_code(struct bpf_verifier_env *env, int regno, const char
case BPF_PROG_TYPE_RAW_TRACEPOINT:
if (!env->prog->aux->attach_btf_id)
return 0;
- range = retval_range(0, 0);
+ *range = retval_range(0, 0);
break;
case BPF_PROG_TYPE_TRACING:
switch (env->prog->expected_attach_type) {
case BPF_TRACE_FENTRY:
case BPF_TRACE_FEXIT:
case BPF_TRACE_FSESSION:
- range = retval_range(0, 0);
+ *range = retval_range(0, 0);
break;
case BPF_TRACE_RAW_TP:
case BPF_MODIFY_RETURN:
@@ -17967,40 +17897,37 @@ static int check_return_code(struct bpf_verifier_env *env, int regno, const char
switch (env->prog->expected_attach_type) {
case BPF_TRACE_KPROBE_SESSION:
case BPF_TRACE_UPROBE_SESSION:
- range = retval_range(0, 1);
break;
default:
return 0;
}
break;
case BPF_PROG_TYPE_SK_LOOKUP:
- range = retval_range(SK_DROP, SK_PASS);
+ *range = retval_range(SK_DROP, SK_PASS);
break;
case BPF_PROG_TYPE_LSM:
if (env->prog->expected_attach_type != BPF_LSM_CGROUP) {
/* no range found, any return value is allowed */
- if (!get_func_retval_range(env->prog, &range))
+ if (!get_func_retval_range(env->prog, range))
return 0;
/* no restricted range, any return value is allowed */
- if (range.minval == S32_MIN && range.maxval == S32_MAX)
+ if (range->minval == S32_MIN && range->maxval == S32_MAX)
return 0;
- return_32bit = true;
+ *return_32bit = true;
} else if (!env->prog->aux->attach_func_proto->type) {
/* Make sure programs that attach to void
* hooks don't try to modify return value.
*/
- range = retval_range(1, 1);
+ *range = retval_range(1, 1);
}
break;
case BPF_PROG_TYPE_NETFILTER:
- range = retval_range(NF_DROP, NF_ACCEPT);
+ *range = retval_range(NF_DROP, NF_ACCEPT);
break;
case BPF_PROG_TYPE_STRUCT_OPS:
- if (!ret_type)
- return 0;
- range = retval_range(0, 0);
+ *range = retval_range(0, 0);
break;
case BPF_PROG_TYPE_EXT:
/* freplace program can return anything as its return value
@@ -18010,6 +17937,102 @@ static int check_return_code(struct bpf_verifier_env *env, int regno, const char
return 0;
}
+ /* Continue calculating. */
+
+ return 1;
+}
+
+static int check_return_code(struct bpf_verifier_env *env, int regno, const char *reg_name)
+{
+ const char *exit_ctx = "At program exit";
+ struct tnum enforce_attach_type_range = tnum_unknown;
+ const struct bpf_prog *prog = env->prog;
+ struct bpf_reg_state *reg = reg_state(env, regno);
+ struct bpf_retval_range range = retval_range(0, 1);
+ enum bpf_prog_type prog_type = resolve_prog_type(env->prog);
+ int ret, err;
+ struct bpf_func_state *frame = env->cur_state->frame[0];
+ const bool is_subprog = frame->subprogno;
+ bool return_32bit = false;
+ const struct btf_type *reg_type, *ret_type = NULL;
+
+ /* LSM and struct_ops func-ptr's return type could be "void" */
+ if (!is_subprog || frame->in_exception_callback_fn) {
+ switch (prog_type) {
+ case BPF_PROG_TYPE_LSM:
+ if (prog->expected_attach_type == BPF_LSM_CGROUP)
+ /* See return_retval_range, can be 0 or 0-1 depending on hook. */
+ break;
+ if (!prog->aux->attach_func_proto->type)
+ return 0;
+ break;
+ case BPF_PROG_TYPE_STRUCT_OPS:
+ if (!prog->aux->attach_func_proto->type)
+ return 0;
+
+ if (frame->in_exception_callback_fn)
+ break;
+
+ /* Allow a struct_ops program to return a referenced kptr if it
+ * matches the operator's return type and is in its unmodified
+ * form. A scalar zero (i.e., a null pointer) is also allowed.
+ */
+ reg_type = reg->btf ? btf_type_by_id(reg->btf, reg->btf_id) : NULL;
+ ret_type = btf_type_resolve_ptr(prog->aux->attach_btf,
+ prog->aux->attach_func_proto->type,
+ NULL);
+ if (ret_type && ret_type == reg_type && reg->ref_obj_id)
+ return __check_ptr_off_reg(env, reg, regno, false);
+ break;
+ default:
+ break;
+ }
+ }
+
+ /* eBPF calling convention is such that R0 is used
+ * to return the value from eBPF program.
+ * Make sure that it's readable at this time
+ * of bpf_exit, which means that program wrote
+ * something into it earlier
+ */
+ err = check_reg_arg(env, regno, SRC_OP);
+ if (err)
+ return err;
+
+ if (is_pointer_value(env, regno)) {
+ verbose(env, "R%d leaks addr as return value\n", regno);
+ return -EACCES;
+ }
+
+ if (frame->in_async_callback_fn) {
+ exit_ctx = "At async callback return";
+ range = frame->callback_ret_range;
+ goto enforce_retval;
+ }
+
+ if (is_subprog && !frame->in_exception_callback_fn) {
+ if (reg->type != SCALAR_VALUE) {
+ verbose(env, "At subprogram exit the register R%d is not a scalar value (%s)\n",
+ regno, reg_type_str(env, reg->type));
+ return -EINVAL;
+ }
+ return 0;
+ }
+
+ if (prog_type == BPF_PROG_TYPE_STRUCT_OPS && !ret_type)
+ return 0;
+
+ if (prog_type == BPF_PROG_TYPE_CGROUP_SKB && (env->prog->expected_attach_type == BPF_CGROUP_INET_EGRESS))
+ enforce_attach_type_range = tnum_range(2, 3);
+
+ ret = return_retval_range(env, &range, &return_32bit);
+ if (ret < 0)
+ return ret;
+
+ /* Check if we even need to validate. */
+ if (!ret)
+ return ret;
+
enforce_retval:
if (reg->type != SCALAR_VALUE) {
verbose(env, "%s the register R%d is not a known value (%s)\n",
--
2.49.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH bpf-next v4 2/4] bpf: Separate bpf_throw() and bpf_exit() return value validation
2026-02-25 3:33 [PATCH bpf-next v4 0/4] bpf: Allow void return type for global subprogs Emil Tsalapatis
2026-02-25 3:33 ` [PATCH bpf-next v4 1/4] bpf: Factor out program return value calculation Emil Tsalapatis
@ 2026-02-25 3:33 ` Emil Tsalapatis
2026-02-25 4:23 ` bot+bpf-ci
2026-02-26 4:56 ` Eduard Zingerman
2026-02-25 3:33 ` [PATCH bpf-next v4 3/4] bpf: Allow void global functions in the verifier Emil Tsalapatis
2026-02-25 3:33 ` [PATCH bpf-next v4 4/4] selftests: bpf: Add tests for void global subprogs Emil Tsalapatis
3 siblings, 2 replies; 11+ messages in thread
From: Emil Tsalapatis @ 2026-02-25 3:33 UTC (permalink / raw)
To: bpf
Cc: andrii, ast, daniel, eddyz87, martin.lau, memxor, song,
yonghong.song, Emil Tsalapatis
Both bpf_throw() and BPF_EXIT use the same return value validation
function, even though their validation logic is slightly different.
Using the same code path for both complicates the logic and makes
it difficult to follow, so create a new check_throw_return_code()
function just for bpf_throw()-based exits. Also factor code needed
for both functions out of check_return_code().
No functional changes.
Suggested-by: Eduard Zingerman <eddyz87@gmail.com>
Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
---
kernel/bpf/verifier.c | 120 ++++++++++++++++++++++++++++++++++--------
1 file changed, 99 insertions(+), 21 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 96ec27a36b32..8ed5060f6ea8 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -14029,7 +14029,8 @@ static int check_special_kfunc(struct bpf_verifier_env *env, struct bpf_kfunc_ca
return 1;
}
-static int check_return_code(struct bpf_verifier_env *env, int regno, const char *reg_name);
+static int check_throw_return_code(struct bpf_verifier_env *env);
+static int check_return_code(struct bpf_verifier_env *env);
static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
int *insn_idx_p)
@@ -14257,7 +14258,7 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
* to bpf_throw becomes the return value of the program.
*/
if (!env->exception_callback_subprog) {
- err = check_return_code(env, BPF_REG_1, "R1");
+ err = check_throw_return_code(env);
if (err < 0)
return err;
}
@@ -17942,12 +17943,98 @@ static int return_retval_range(struct bpf_verifier_env *env, struct bpf_retval_r
return 1;
}
-static int check_return_code(struct bpf_verifier_env *env, int regno, const char *reg_name)
+static void
+maybe_enforce_expected_attach_type(struct bpf_verifier_env *env, struct bpf_reg_state *reg)
+{
+ struct tnum enforce_attach_type_range;
+ struct bpf_prog *prog = env->prog;
+ enum bpf_prog_type prog_type = resolve_prog_type(prog);
+
+ /* Throw or not throw, still limit the attach type. */
+ if (prog_type != BPF_PROG_TYPE_CGROUP_SKB || prog->expected_attach_type != BPF_CGROUP_INET_EGRESS)
+ return;
+
+ enforce_attach_type_range = tnum_range(2, 3);
+
+ if (tnum_in(enforce_attach_type_range, reg->var_off))
+ prog->enforce_expected_attach_type = 1;
+}
+
+static int
+check_retval_is_bounded(struct bpf_verifier_env *env, struct bpf_retval_range range, struct bpf_reg_state *reg,
+ const char *reg_name, bool return_32bit, bool is_subprog, const char *exit_ctx)
+{
+ const struct bpf_prog *prog = env->prog;
+ enum bpf_prog_type prog_type = resolve_prog_type(prog);
+
+ if (retval_range_within(range, reg, return_32bit))
+ return 0;
+
+ verbose_invalid_scalar(env, reg, range, exit_ctx, reg_name);
+ if (!is_subprog &&
+ prog->expected_attach_type == BPF_LSM_CGROUP &&
+ prog_type == BPF_PROG_TYPE_LSM &&
+ !prog->aux->attach_func_proto->type)
+ verbose(env, "Note, BPF_LSM_CGROUP that attach to void LSM hooks can't modify return value!\n");
+
+ return -EINVAL;
+}
+
+static int check_throw_return_code(struct bpf_verifier_env *env)
{
const char *exit_ctx = "At program exit";
- struct tnum enforce_attach_type_range = tnum_unknown;
const struct bpf_prog *prog = env->prog;
- struct bpf_reg_state *reg = reg_state(env, regno);
+ struct bpf_reg_state *reg = reg_state(env, BPF_REG_1);
+ struct bpf_retval_range range = retval_range(0, 1);
+ enum bpf_prog_type prog_type = resolve_prog_type(env->prog);
+ struct bpf_func_state *frame = env->cur_state->frame[0];
+ const bool is_subprog = frame->subprogno;
+ bool return_32bit = false;
+ int err, ret;
+
+ /*
+ * BPF_LSM_CGROUP LSM programs must not modify the return value of a void program
+ * (see the logic in return_retval_range()).
+ */
+ if (prog_type == BPF_PROG_TYPE_LSM && prog->expected_attach_type != BPF_LSM_CGROUP &&
+ !prog->aux->attach_func_proto->type)
+ return 0;
+
+ ret = return_retval_range(env, &range, &return_32bit);
+ if (ret < 0)
+ return ret;
+
+ /*
+ * Specifically for STRUCT_OPS programs, we never need
+ * to validate the cookie.
+ */
+ if (prog_type == BPF_PROG_TYPE_STRUCT_OPS)
+ return 0;
+
+ /* Check if we even need to validate. */
+ if (!ret)
+ return ret;
+
+ err = mark_chain_precision(env, BPF_REG_1);
+ if (err)
+ return err;
+
+ err = check_retval_is_bounded(env, range, reg, "R1",
+ return_32bit, is_subprog, exit_ctx);
+ if (err)
+ return err;
+
+ maybe_enforce_expected_attach_type(env, reg);
+
+ return 0;
+}
+
+static int check_return_code(struct bpf_verifier_env *env)
+{
+ const int regno = BPF_REG_0;
+ const char *exit_ctx = "At program exit";
+ const struct bpf_prog *prog = env->prog;
+ struct bpf_reg_state *reg = reg_state(env, BPF_REG_0);
struct bpf_retval_range range = retval_range(0, 1);
enum bpf_prog_type prog_type = resolve_prog_type(env->prog);
int ret, err;
@@ -18022,9 +18109,6 @@ static int check_return_code(struct bpf_verifier_env *env, int regno, const char
if (prog_type == BPF_PROG_TYPE_STRUCT_OPS && !ret_type)
return 0;
- if (prog_type == BPF_PROG_TYPE_CGROUP_SKB && (env->prog->expected_attach_type == BPF_CGROUP_INET_EGRESS))
- enforce_attach_type_range = tnum_range(2, 3);
-
ret = return_retval_range(env, &range, &return_32bit);
if (ret < 0)
return ret;
@@ -18044,19 +18128,13 @@ static int check_return_code(struct bpf_verifier_env *env, int regno, const char
if (err)
return err;
- if (!retval_range_within(range, reg, return_32bit)) {
- verbose_invalid_scalar(env, reg, range, exit_ctx, reg_name);
- if (!is_subprog &&
- prog->expected_attach_type == BPF_LSM_CGROUP &&
- prog_type == BPF_PROG_TYPE_LSM &&
- !prog->aux->attach_func_proto->type)
- verbose(env, "Note, BPF_LSM_CGROUP that attach to void LSM hooks can't modify return value!\n");
- return -EINVAL;
- }
+ err = check_retval_is_bounded(env, range, reg, "R0",
+ return_32bit, is_subprog, exit_ctx);
+ if (err)
+ return err;
+
+ maybe_enforce_expected_attach_type(env, reg);
- if (!tnum_is_unknown(enforce_attach_type_range) &&
- tnum_in(enforce_attach_type_range, reg->var_off))
- env->prog->enforce_expected_attach_type = 1;
return 0;
}
@@ -20888,7 +20966,7 @@ static int process_bpf_exit_full(struct bpf_verifier_env *env,
return 0;
}
- err = check_return_code(env, BPF_REG_0, "R0");
+ err = check_return_code(env);
if (err)
return err;
return PROCESS_BPF_EXIT;
--
2.49.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH bpf-next v4 3/4] bpf: Allow void global functions in the verifier
2026-02-25 3:33 [PATCH bpf-next v4 0/4] bpf: Allow void return type for global subprogs Emil Tsalapatis
2026-02-25 3:33 ` [PATCH bpf-next v4 1/4] bpf: Factor out program return value calculation Emil Tsalapatis
2026-02-25 3:33 ` [PATCH bpf-next v4 2/4] bpf: Separate bpf_throw() and bpf_exit() return value validation Emil Tsalapatis
@ 2026-02-25 3:33 ` Emil Tsalapatis
2026-02-25 3:33 ` [PATCH bpf-next v4 4/4] selftests: bpf: Add tests for void global subprogs Emil Tsalapatis
3 siblings, 0 replies; 11+ messages in thread
From: Emil Tsalapatis @ 2026-02-25 3:33 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 8ed5060f6ea8..be597b90116c 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 (!func_proto)
+ return false;
+
+ type = btf_type_skip_modifiers(btf, func_proto->type, NULL);
+ if (!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;
@@ -10857,9 +10880,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;
@@ -18045,6 +18070,16 @@ static int check_return_code(struct bpf_verifier_env *env)
/* 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)
@@ -18074,6 +18109,10 @@ static int check_return_code(struct bpf_verifier_env *env)
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
@@ -24603,10 +24642,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] 11+ messages in thread
* [PATCH bpf-next v4 4/4] selftests: bpf: Add tests for void global subprogs
2026-02-25 3:33 [PATCH bpf-next v4 0/4] bpf: Allow void return type for global subprogs Emil Tsalapatis
` (2 preceding siblings ...)
2026-02-25 3:33 ` [PATCH bpf-next v4 3/4] bpf: Allow void global functions in the verifier Emil Tsalapatis
@ 2026-02-25 3:33 ` Emil Tsalapatis
3 siblings, 0 replies; 11+ messages in thread
From: Emil Tsalapatis @ 2026-02-25 3:33 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/exceptions.c | 1 +
.../selftests/bpf/prog_tests/fexit_bpf2bpf.c | 24 +++++++++++++
.../testing/selftests/bpf/progs/exceptions.c | 14 ++++++++
.../selftests/bpf/progs/exceptions_fail.c | 35 +++++++++++++++++--
.../bpf/progs/freplace_int_with_void.c | 14 ++++++++
.../selftests/bpf/progs/freplace_void.c | 12 +++++++
.../bpf/progs/verifier_global_subprogs.c | 19 ++++++++++
7 files changed, 116 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
diff --git a/tools/testing/selftests/bpf/prog_tests/exceptions.c b/tools/testing/selftests/bpf/prog_tests/exceptions.c
index 516f4a13013c..84ab73e08b0e 100644
--- a/tools/testing/selftests/bpf/prog_tests/exceptions.c
+++ b/tools/testing/selftests/bpf/prog_tests/exceptions.c
@@ -83,6 +83,7 @@ static void test_exceptions_success(void)
RUN_SUCCESS(exception_assert_range_with, 1);
RUN_SUCCESS(exception_bad_assert_range, 0);
RUN_SUCCESS(exception_bad_assert_range_with, 10);
+ RUN_SUCCESS(exception_throw_from_void_global, 11);
#define RUN_EXT(load_ret, attach_err, expr, msg, after_link) \
{ \
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/progs/exceptions.c b/tools/testing/selftests/bpf/progs/exceptions.c
index f09cd14d8e04..4206f59d7b86 100644
--- a/tools/testing/selftests/bpf/progs/exceptions.c
+++ b/tools/testing/selftests/bpf/progs/exceptions.c
@@ -109,6 +109,20 @@ int exception_tail_call(struct __sk_buff *ctx) {
return ret + 8;
}
+__weak
+void throw_11(void)
+{
+ bpf_throw(11);
+}
+
+SEC("tc")
+int exception_throw_from_void_global(struct __sk_buff *ctx)
+{
+ throw_11();
+
+ return 0;
+}
+
__noinline int exception_ext_global(struct __sk_buff *ctx)
{
volatile int ret = 0;
diff --git a/tools/testing/selftests/bpf/progs/exceptions_fail.c b/tools/testing/selftests/bpf/progs/exceptions_fail.c
index d28ecc4ee2d0..275ad6fe4a04 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)
{
@@ -346,4 +359,20 @@ int reject_exception_throw_cb_diff(struct __sk_buff *ctx)
return 0;
}
+__weak
+void foo(void)
+{
+ bpf_throw(1);
+}
+
+SEC("?fentry/bpf_check")
+__failure __msg("At program exit the register R1 has smin=1 smax=1 should")
+int reject_out_of_range_global_throw(struct __sk_buff *skb)
+{
+ foo();
+
+ return 0;
+}
+
+
char _license[] SEC("license") = "GPL";
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..cfa4af00991d
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/freplace_void.c
@@ -0,0 +1,12 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+
+volatile int data;
+
+SEC("freplace/foo")
+void test_freplace_void(struct __sk_buff *skb)
+{
+}
+
+char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/progs/verifier_global_subprogs.c b/tools/testing/selftests/bpf/progs/verifier_global_subprogs.c
index 20904cd2baa2..f02012a2fbaa 100644
--- a/tools/testing/selftests/bpf/progs/verifier_global_subprogs.c
+++ b/tools/testing/selftests/bpf/progs/verifier_global_subprogs.c
@@ -388,4 +388,23 @@ int arg_tag_dynptr(struct xdp_md *ctx)
return subprog_dynptr(&dptr);
}
+__weak
+void foo(void)
+{
+}
+
+SEC("?tc")
+__failure __msg("R0 !read_ok")
+int return_from_void_global(struct __sk_buff *skb)
+{
+ foo();
+
+ asm volatile(
+ "r1 = r0;"
+ :::
+ );
+
+ return 0;
+}
+
char _license[] SEC("license") = "GPL";
--
2.49.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next v4 2/4] bpf: Separate bpf_throw() and bpf_exit() return value validation
2026-02-25 3:33 ` [PATCH bpf-next v4 2/4] bpf: Separate bpf_throw() and bpf_exit() return value validation Emil Tsalapatis
@ 2026-02-25 4:23 ` bot+bpf-ci
2026-02-26 1:48 ` Emil Tsalapatis
2026-02-26 4:56 ` Eduard Zingerman
1 sibling, 1 reply; 11+ messages in thread
From: bot+bpf-ci @ 2026-02-25 4:23 UTC (permalink / raw)
To: emil, bpf
Cc: andrii, ast, daniel, eddyz87, martin.lau, memxor, song,
yonghong.song, emil, ast, andrii, daniel, martin.lau, eddyz87,
yonghong.song, clm, ihor.solodrai
[-- Attachment #1: Type: text/plain, Size: 2560 bytes --]
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
[ ... ]
> +static int check_throw_return_code(struct bpf_verifier_env *env)
> +{
> + const char *exit_ctx = "At program exit";
> + const struct bpf_prog *prog = env->prog;
> + struct bpf_reg_state *reg = reg_state(env, BPF_REG_1);
> + struct bpf_retval_range range = retval_range(0, 1);
> + enum bpf_prog_type prog_type = resolve_prog_type(env->prog);
> + struct bpf_func_state *frame = env->cur_state->frame[0];
> + const bool is_subprog = frame->subprogno;
> + bool return_32bit = false;
> + int err, ret;
> +
> + /*
> + * BPF_LSM_CGROUP LSM programs must not modify the return value of a void program
> + * (see the logic in return_retval_range()).
> + */
> + if (prog_type == BPF_PROG_TYPE_LSM && prog->expected_attach_type != BPF_LSM_CGROUP &&
> + !prog->aux->attach_func_proto->type)
> + return 0;
Is the comment above this condition a bit misleading? It describes
why BPF_LSM_CGROUP programs need validation, but the condition
itself handles the opposite case -- non-CGROUP void LSM programs
that skip validation.
A reader would naturally associate the comment with the code that
follows it, and could be confused by the mismatch between the
comment mentioning BPF_LSM_CGROUP and the condition checking
!= BPF_LSM_CGROUP.
In the old check_return_code(), the per-case comment was:
if (prog->expected_attach_type == BPF_LSM_CGROUP)
/* See return_retval_range, can be 0 or 0-1 depending on hook. */
break;
Would something like the following be clearer here?
/*
* Non-CGROUP LSM programs with void return type do not need
* cookie validation. BPF_LSM_CGROUP programs need validation
* even for void hooks (see return_retval_range()).
*/
[ ... ]
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/22381317682
AI-authorship-score: low
AI-authorship-explanation: Standard refactoring suggested by reviewer Eduard Zingerman during patch review, with natural kernel coding patterns and commit message prose consistent with human authorship.
issues-found: 1
issue-severity-score: low
issue-severity-explanation: Misleading comment in new check_throw_return_code() describes BPF_LSM_CGROUP behavior before a condition handling non-CGROUP programs, creating readability confusion but no runtime impact.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next v4 1/4] bpf: Factor out program return value calculation
2026-02-25 3:33 ` [PATCH bpf-next v4 1/4] bpf: Factor out program return value calculation Emil Tsalapatis
@ 2026-02-25 23:31 ` Eduard Zingerman
2026-02-26 21:17 ` Emil Tsalapatis
0 siblings, 1 reply; 11+ messages in thread
From: Eduard Zingerman @ 2026-02-25 23:31 UTC (permalink / raw)
To: Emil Tsalapatis, bpf
Cc: andrii, ast, daniel, martin.lau, memxor, song, yonghong.song
On Tue, 2026-02-24 at 22:33 -0500, Emil Tsalapatis wrote:
> Factor the return value range calculation logic in check_return_code
> out of the function in preparation for separating the return value
> validation logic for BPF_EXIT and bpf_throw().
>
> Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
> ---
I like this refactoring, thank you! The logic seem to be preserved.
A few nits below.
Acked-by: Eduard Zingerman <eddyz87@gmail.com>
> kernel/bpf/verifier.c | 205 +++++++++++++++++++++++-------------------
> 1 file changed, 114 insertions(+), 91 deletions(-)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index edf5342b982f..96ec27a36b32 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -17837,82 +17837,14 @@ static int check_ld_abs(struct bpf_verifier_env *env, struct bpf_insn
> *insn)
> return 0;
> }
>
> -static int check_return_code(struct bpf_verifier_env *env, int regno, const char *reg_name)
> +
> +static int return_retval_range(struct bpf_verifier_env *env, struct bpf_retval_range *range,
> + bool *return_32bit)
Nit: maybe make this function bool?
It does not seem to return any errors.
> {
> - const char *exit_ctx = "At program exit";
> - struct tnum enforce_attach_type_range = tnum_unknown;
> - const struct bpf_prog *prog = env->prog;
> - struct bpf_reg_state *reg = reg_state(env, regno);
> - struct bpf_retval_range range = retval_range(0, 1);
> enum bpf_prog_type prog_type = resolve_prog_type(env->prog);
> - int err;
> - struct bpf_func_state *frame = env->cur_state->frame[0];
> - const bool is_subprog = frame->subprogno;
> - bool return_32bit = false;
> - const struct btf_type *reg_type, *ret_type = NULL;
> -
> - /* LSM and struct_ops func-ptr's return type could be "void" */
> - if (!is_subprog || frame->in_exception_callback_fn) {
> - switch (prog_type) {
> - case BPF_PROG_TYPE_LSM:
> - if (prog->expected_attach_type == BPF_LSM_CGROUP)
> - /* See below, can be 0 or 0-1 depending on hook. */
> - break;
> - if (!prog->aux->attach_func_proto->type)
> - return 0;
> - break;
> - case BPF_PROG_TYPE_STRUCT_OPS:
> - if (!prog->aux->attach_func_proto->type)
> - return 0;
> -
> - if (frame->in_exception_callback_fn)
> - break;
> -
> - /* Allow a struct_ops program to return a referenced kptr if it
> - * matches the operator's return type and is in its unmodified
> - * form. A scalar zero (i.e., a null pointer) is also allowed.
> - */
> - reg_type = reg->btf ? btf_type_by_id(reg->btf, reg->btf_id) : NULL;
> - ret_type = btf_type_resolve_ptr(prog->aux->attach_btf,
> - prog->aux->attach_func_proto->type,
> - NULL);
> - if (ret_type && ret_type == reg_type && reg->ref_obj_id)
> - return __check_ptr_off_reg(env, reg, regno, false);
> - break;
> - default:
> - break;
> - }
> - }
>
> - /* eBPF calling convention is such that R0 is used
> - * to return the value from eBPF program.
> - * Make sure that it's readable at this time
> - * of bpf_exit, which means that program wrote
> - * something into it earlier
> - */
> - err = check_reg_arg(env, regno, SRC_OP);
> - if (err)
> - return err;
> -
> - if (is_pointer_value(env, regno)) {
> - verbose(env, "R%d leaks addr as return value\n", regno);
> - return -EACCES;
> - }
> -
> - if (frame->in_async_callback_fn) {
> - exit_ctx = "At async callback return";
> - range = frame->callback_ret_range;
> - goto enforce_retval;
> - }
> -
> - if (is_subprog && !frame->in_exception_callback_fn) {
> - if (reg->type != SCALAR_VALUE) {
> - verbose(env, "At subprogram exit the register R%d is not a scalar value
> (%s)\n",
> - regno, reg_type_str(env, reg->type));
> - return -EINVAL;
> - }
> - return 0;
> - }
> + /* Default return value range. */
> + *range = retval_range(0, 1);
>
> switch (prog_type) {
> case BPF_PROG_TYPE_CGROUP_SOCK_ADDR:
> @@ -17925,16 +17857,14 @@ static int check_return_code(struct bpf_verifier_env *env, int regno, const
> char
> env->prog->expected_attach_type == BPF_CGROUP_INET4_GETSOCKNAME ||
> env->prog->expected_attach_type == BPF_CGROUP_INET6_GETSOCKNAME ||
> env->prog->expected_attach_type == BPF_CGROUP_UNIX_GETSOCKNAME)
> - range = retval_range(1, 1);
> + *range = retval_range(1, 1);
> if (env->prog->expected_attach_type == BPF_CGROUP_INET4_BIND ||
> env->prog->expected_attach_type == BPF_CGROUP_INET6_BIND)
> - range = retval_range(0, 3);
> + *range = retval_range(0, 3);
> break;
> case BPF_PROG_TYPE_CGROUP_SKB:
> - if (env->prog->expected_attach_type == BPF_CGROUP_INET_EGRESS) {
> - range = retval_range(0, 3);
> - enforce_attach_type_range = tnum_range(2, 3);
> - }
> + if (env->prog->expected_attach_type == BPF_CGROUP_INET_EGRESS)
> + *range = retval_range(0, 3);
> break;
> case BPF_PROG_TYPE_CGROUP_SOCK:
> case BPF_PROG_TYPE_SOCK_OPS:
> @@ -17945,14 +17875,14 @@ static int check_return_code(struct bpf_verifier_env *env, int regno, const
> char
> case BPF_PROG_TYPE_RAW_TRACEPOINT:
> if (!env->prog->aux->attach_btf_id)
> return 0;
> - range = retval_range(0, 0);
> + *range = retval_range(0, 0);
> break;
> case BPF_PROG_TYPE_TRACING:
> switch (env->prog->expected_attach_type) {
> case BPF_TRACE_FENTRY:
> case BPF_TRACE_FEXIT:
> case BPF_TRACE_FSESSION:
> - range = retval_range(0, 0);
> + *range = retval_range(0, 0);
> break;
> case BPF_TRACE_RAW_TP:
> case BPF_MODIFY_RETURN:
> @@ -17967,40 +17897,37 @@ static int check_return_code(struct bpf_verifier_env *env, int regno, const
> char
> switch (env->prog->expected_attach_type) {
> case BPF_TRACE_KPROBE_SESSION:
> case BPF_TRACE_UPROBE_SESSION:
> - range = retval_range(0, 1);
> break;
> default:
> return 0;
> }
> break;
> case BPF_PROG_TYPE_SK_LOOKUP:
> - range = retval_range(SK_DROP, SK_PASS);
> + *range = retval_range(SK_DROP, SK_PASS);
> break;
>
> case BPF_PROG_TYPE_LSM:
> if (env->prog->expected_attach_type != BPF_LSM_CGROUP) {
> /* no range found, any return value is allowed */
> - if (!get_func_retval_range(env->prog, &range))
> + if (!get_func_retval_range(env->prog, range))
> return 0;
> /* no restricted range, any return value is allowed */
> - if (range.minval == S32_MIN && range.maxval == S32_MAX)
> + if (range->minval == S32_MIN && range->maxval == S32_MAX)
Tangential to this refactoring. Looking at get_func_retval_range() it
seems that S32_{MIN,MAX} special case can never happen.
It defers to bpf_lsm_get_retval_range() which either does not set the
range or it to [0, 1] or [-MAX_ERRNO, 0].
Maybe remove it as a separate patch?
> return 0;
> - return_32bit = true;
> + *return_32bit = true;
Nit: maybe make this a special case in check_return_code(),
as with enforce_attach_type_range?
Or push 'return_32bit' to be a field in retval_range?
> } else if (!env->prog->aux->attach_func_proto->type) {
> /* Make sure programs that attach to void
> * hooks don't try to modify return value.
> */
> - range = retval_range(1, 1);
> + *range = retval_range(1, 1);
> }
> break;
>
> case BPF_PROG_TYPE_NETFILTER:
> - range = retval_range(NF_DROP, NF_ACCEPT);
> + *range = retval_range(NF_DROP, NF_ACCEPT);
> break;
> case BPF_PROG_TYPE_STRUCT_OPS:
> - if (!ret_type)
> - return 0;
> - range = retval_range(0, 0);
> + *range = retval_range(0, 0);
> break;
> case BPF_PROG_TYPE_EXT:
> /* freplace program can return anything as its return value
[...]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next v4 2/4] bpf: Separate bpf_throw() and bpf_exit() return value validation
2026-02-25 4:23 ` bot+bpf-ci
@ 2026-02-26 1:48 ` Emil Tsalapatis
0 siblings, 0 replies; 11+ messages in thread
From: Emil Tsalapatis @ 2026-02-26 1:48 UTC (permalink / raw)
To: bot+bpf-ci, bpf
Cc: andrii, ast, daniel, eddyz87, martin.lau, memxor, song,
yonghong.song, clm, ihor.solodrai
On Tue Feb 24, 2026 at 11:23 PM EST, bot+bpf-ci wrote:
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>
> [ ... ]
>
>> +static int check_throw_return_code(struct bpf_verifier_env *env)
>> +{
>> + const char *exit_ctx = "At program exit";
>> + const struct bpf_prog *prog = env->prog;
>> + struct bpf_reg_state *reg = reg_state(env, BPF_REG_1);
>> + struct bpf_retval_range range = retval_range(0, 1);
>> + enum bpf_prog_type prog_type = resolve_prog_type(env->prog);
>> + struct bpf_func_state *frame = env->cur_state->frame[0];
>> + const bool is_subprog = frame->subprogno;
>> + bool return_32bit = false;
>> + int err, ret;
>> +
>> + /*
>> + * BPF_LSM_CGROUP LSM programs must not modify the return value of a void program
>> + * (see the logic in return_retval_range()).
>> + */
>> + if (prog_type == BPF_PROG_TYPE_LSM && prog->expected_attach_type != BPF_LSM_CGROUP &&
>> + !prog->aux->attach_func_proto->type)
>> + return 0;
>
> Is the comment above this condition a bit misleading? It describes
> why BPF_LSM_CGROUP programs need validation, but the condition
> itself handles the opposite case -- non-CGROUP void LSM programs
> that skip validation.
>
> A reader would naturally associate the comment with the code that
> follows it, and could be confused by the mismatch between the
> comment mentioning BPF_LSM_CGROUP and the condition checking
> != BPF_LSM_CGROUP.
>
> In the old check_return_code(), the per-case comment was:
>
> if (prog->expected_attach_type == BPF_LSM_CGROUP)
> /* See return_retval_range, can be 0 or 0-1 depending on hook. */
> break;
>
> Would something like the following be clearer here?
>
> /*
> * Non-CGROUP LSM programs with void return type do not need
> * cookie validation. BPF_LSM_CGROUP programs need validation
> * even for void hooks (see return_retval_range()).
> */
>
I mildly disagree with the AI here, I think it's a small leap from
reading that BPF_LSM_CGROUP void programs must still be verified to
understanding why we only return 0 for non-BPF_LSM_CGROUP programs. We
also refer to return_retval_range() for a more definitive explanation
instead of duplicating it here. Ofc if human reviewers prefer the proposed
version I will update it.
> [ ... ]
>
>
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
>
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/22381317682
>
> AI-authorship-score: low
> AI-authorship-explanation: Standard refactoring suggested by reviewer Eduard Zingerman during patch review, with natural kernel coding patterns and commit message prose consistent with human authorship.
> issues-found: 1
> issue-severity-score: low
> issue-severity-explanation: Misleading comment in new check_throw_return_code() describes BPF_LSM_CGROUP behavior before a condition handling non-CGROUP programs, creating readability confusion but no runtime impact.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next v4 2/4] bpf: Separate bpf_throw() and bpf_exit() return value validation
2026-02-25 3:33 ` [PATCH bpf-next v4 2/4] bpf: Separate bpf_throw() and bpf_exit() return value validation Emil Tsalapatis
2026-02-25 4:23 ` bot+bpf-ci
@ 2026-02-26 4:56 ` Eduard Zingerman
2026-02-26 20:35 ` Emil Tsalapatis
1 sibling, 1 reply; 11+ messages in thread
From: Eduard Zingerman @ 2026-02-26 4:56 UTC (permalink / raw)
To: Emil Tsalapatis, bpf
Cc: andrii, ast, daniel, martin.lau, memxor, song, yonghong.song
On Tue, 2026-02-24 at 22:33 -0500, Emil Tsalapatis wrote:
> Both bpf_throw() and BPF_EXIT use the same return value validation
> function, even though their validation logic is slightly different.
> Using the same code path for both complicates the logic and makes
> it difficult to follow, so create a new check_throw_return_code()
> function just for bpf_throw()-based exits. Also factor code needed
> for both functions out of check_return_code().
>
> No functional changes.
>
> Suggested-by: Eduard Zingerman <eddyz87@gmail.com>
> Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
> ---
I think I suggested the wrong split.
A better border would be between checking return code for subprograms
and for programs. Wdyt about the following branch:
https://github.com/eddyz87/bpf/tree/check-subprogram-return-code
(fexit_bpf2bpf/freplace_void fails,
but I think it could be fixed after some debugging).
[...]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next v4 2/4] bpf: Separate bpf_throw() and bpf_exit() return value validation
2026-02-26 4:56 ` Eduard Zingerman
@ 2026-02-26 20:35 ` Emil Tsalapatis
0 siblings, 0 replies; 11+ messages in thread
From: Emil Tsalapatis @ 2026-02-26 20:35 UTC (permalink / raw)
To: Eduard Zingerman, bpf
Cc: andrii, ast, daniel, martin.lau, memxor, song, yonghong.song
On Wed Feb 25, 2026 at 11:56 PM EST, Eduard Zingerman wrote:
> On Tue, 2026-02-24 at 22:33 -0500, Emil Tsalapatis wrote:
>> Both bpf_throw() and BPF_EXIT use the same return value validation
>> function, even though their validation logic is slightly different.
>> Using the same code path for both complicates the logic and makes
>> it difficult to follow, so create a new check_throw_return_code()
>> function just for bpf_throw()-based exits. Also factor code needed
>> for both functions out of check_return_code().
>>
>> No functional changes.
>>
>> Suggested-by: Eduard Zingerman <eddyz87@gmail.com>
>> Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
>> ---
>
> I think I suggested the wrong split.
> A better border would be between checking return code for subprograms
> and for programs. Wdyt about the following branch:
> https://github.com/eddyz87/bpf/tree/check-subprogram-return-code
> (fexit_bpf2bpf/freplace_void fails,
> but I think it could be fixed after some debugging).
>
I see, yeah this split avoids duplication and removes the constant
is_subprog checks. I'm going through the two patches rn and will
send out the new version with them.
> [...]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH bpf-next v4 1/4] bpf: Factor out program return value calculation
2026-02-25 23:31 ` Eduard Zingerman
@ 2026-02-26 21:17 ` Emil Tsalapatis
0 siblings, 0 replies; 11+ messages in thread
From: Emil Tsalapatis @ 2026-02-26 21:17 UTC (permalink / raw)
To: Eduard Zingerman, bpf
Cc: andrii, ast, daniel, martin.lau, memxor, song, yonghong.song
On Wed Feb 25, 2026 at 6:31 PM EST, Eduard Zingerman wrote:
> On Tue, 2026-02-24 at 22:33 -0500, Emil Tsalapatis wrote:
>> Factor the return value range calculation logic in check_return_code
>> out of the function in preparation for separating the return value
>> validation logic for BPF_EXIT and bpf_throw().
>>
>> Signed-off-by: Emil Tsalapatis <emil@etsalapatis.com>
>> ---
>
> I like this refactoring, thank you! The logic seem to be preserved.
> A few nits below.
>
> Acked-by: Eduard Zingerman <eddyz87@gmail.com>
>
>> kernel/bpf/verifier.c | 205 +++++++++++++++++++++++-------------------
>> 1 file changed, 114 insertions(+), 91 deletions(-)
>>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index edf5342b982f..96ec27a36b32 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -17837,82 +17837,14 @@ static int check_ld_abs(struct bpf_verifier_env *env, struct bpf_insn
>> *insn)
>> return 0;
>> }
>>
>> -static int check_return_code(struct bpf_verifier_env *env, int regno, const char *reg_name)
>> +
>> +static int return_retval_range(struct bpf_verifier_env *env, struct bpf_retval_range *range,
>> + bool *return_32bit)
>
> Nit: maybe make this function bool?
> It does not seem to return any errors.
Annoyingly, there is a _single_ -EOPNOTSUPP for TRACING programs that do
not have one of 6 attach types (FENTRY/FEXIT/FSESSION/RAW_TP/MODIFY_RETURN/TRACE_ITER).
IIUC This is actually an exhaustive list of all attach types for a TRACING program, so the
-EOPNOTSUPP is redundant - we check the attach type is valid at attach/link creation time
with bpf_prog_attach_check_attach_type. We can have any invalid attach types for TRACING
progs fall through to the default range and turn the whole function into a bool.
>
>> {
>> - const char *exit_ctx = "At program exit";
>> - struct tnum enforce_attach_type_range = tnum_unknown;
>> - const struct bpf_prog *prog = env->prog;
>> - struct bpf_reg_state *reg = reg_state(env, regno);
>> - struct bpf_retval_range range = retval_range(0, 1);
>> enum bpf_prog_type prog_type = resolve_prog_type(env->prog);
>> - int err;
>> - struct bpf_func_state *frame = env->cur_state->frame[0];
>> - const bool is_subprog = frame->subprogno;
>> - bool return_32bit = false;
>> - const struct btf_type *reg_type, *ret_type = NULL;
>> -
>> - /* LSM and struct_ops func-ptr's return type could be "void" */
>> - if (!is_subprog || frame->in_exception_callback_fn) {
>> - switch (prog_type) {
>> - case BPF_PROG_TYPE_LSM:
>> - if (prog->expected_attach_type == BPF_LSM_CGROUP)
>> - /* See below, can be 0 or 0-1 depending on hook. */
>> - break;
>> - if (!prog->aux->attach_func_proto->type)
>> - return 0;
>> - break;
>> - case BPF_PROG_TYPE_STRUCT_OPS:
>> - if (!prog->aux->attach_func_proto->type)
>> - return 0;
>> -
>> - if (frame->in_exception_callback_fn)
>> - break;
>> -
>> - /* Allow a struct_ops program to return a referenced kptr if it
>> - * matches the operator's return type and is in its unmodified
>> - * form. A scalar zero (i.e., a null pointer) is also allowed.
>> - */
>> - reg_type = reg->btf ? btf_type_by_id(reg->btf, reg->btf_id) : NULL;
>> - ret_type = btf_type_resolve_ptr(prog->aux->attach_btf,
>> - prog->aux->attach_func_proto->type,
>> - NULL);
>> - if (ret_type && ret_type == reg_type && reg->ref_obj_id)
>> - return __check_ptr_off_reg(env, reg, regno, false);
>> - break;
>> - default:
>> - break;
>> - }
>> - }
>>
>> - /* eBPF calling convention is such that R0 is used
>> - * to return the value from eBPF program.
>> - * Make sure that it's readable at this time
>> - * of bpf_exit, which means that program wrote
>> - * something into it earlier
>> - */
>> - err = check_reg_arg(env, regno, SRC_OP);
>> - if (err)
>> - return err;
>> -
>> - if (is_pointer_value(env, regno)) {
>> - verbose(env, "R%d leaks addr as return value\n", regno);
>> - return -EACCES;
>> - }
>> -
>> - if (frame->in_async_callback_fn) {
>> - exit_ctx = "At async callback return";
>> - range = frame->callback_ret_range;
>> - goto enforce_retval;
>> - }
>> -
>> - if (is_subprog && !frame->in_exception_callback_fn) {
>> - if (reg->type != SCALAR_VALUE) {
>> - verbose(env, "At subprogram exit the register R%d is not a scalar value
>> (%s)\n",
>> - regno, reg_type_str(env, reg->type));
>> - return -EINVAL;
>> - }
>> - return 0;
>> - }
>> + /* Default return value range. */
>> + *range = retval_range(0, 1);
>>
>> switch (prog_type) {
>> case BPF_PROG_TYPE_CGROUP_SOCK_ADDR:
>> @@ -17925,16 +17857,14 @@ static int check_return_code(struct bpf_verifier_env *env, int regno, const
>> char
>> env->prog->expected_attach_type == BPF_CGROUP_INET4_GETSOCKNAME ||
>> env->prog->expected_attach_type == BPF_CGROUP_INET6_GETSOCKNAME ||
>> env->prog->expected_attach_type == BPF_CGROUP_UNIX_GETSOCKNAME)
>> - range = retval_range(1, 1);
>> + *range = retval_range(1, 1);
>> if (env->prog->expected_attach_type == BPF_CGROUP_INET4_BIND ||
>> env->prog->expected_attach_type == BPF_CGROUP_INET6_BIND)
>> - range = retval_range(0, 3);
>> + *range = retval_range(0, 3);
>> break;
>> case BPF_PROG_TYPE_CGROUP_SKB:
>> - if (env->prog->expected_attach_type == BPF_CGROUP_INET_EGRESS) {
>> - range = retval_range(0, 3);
>> - enforce_attach_type_range = tnum_range(2, 3);
>> - }
>> + if (env->prog->expected_attach_type == BPF_CGROUP_INET_EGRESS)
>> + *range = retval_range(0, 3);
>> break;
>> case BPF_PROG_TYPE_CGROUP_SOCK:
>> case BPF_PROG_TYPE_SOCK_OPS:
>> @@ -17945,14 +17875,14 @@ static int check_return_code(struct bpf_verifier_env *env, int regno, const
>> char
>> case BPF_PROG_TYPE_RAW_TRACEPOINT:
>> if (!env->prog->aux->attach_btf_id)
>> return 0;
>> - range = retval_range(0, 0);
>> + *range = retval_range(0, 0);
>> break;
>> case BPF_PROG_TYPE_TRACING:
>> switch (env->prog->expected_attach_type) {
>> case BPF_TRACE_FENTRY:
>> case BPF_TRACE_FEXIT:
>> case BPF_TRACE_FSESSION:
>> - range = retval_range(0, 0);
>> + *range = retval_range(0, 0);
>> break;
>> case BPF_TRACE_RAW_TP:
>> case BPF_MODIFY_RETURN:
>> @@ -17967,40 +17897,37 @@ static int check_return_code(struct bpf_verifier_env *env, int regno, const
>> char
>> switch (env->prog->expected_attach_type) {
>> case BPF_TRACE_KPROBE_SESSION:
>> case BPF_TRACE_UPROBE_SESSION:
>> - range = retval_range(0, 1);
>> break;
>> default:
>> return 0;
>> }
>> break;
>> case BPF_PROG_TYPE_SK_LOOKUP:
>> - range = retval_range(SK_DROP, SK_PASS);
>> + *range = retval_range(SK_DROP, SK_PASS);
>> break;
>>
>> case BPF_PROG_TYPE_LSM:
>> if (env->prog->expected_attach_type != BPF_LSM_CGROUP) {
>> /* no range found, any return value is allowed */
>> - if (!get_func_retval_range(env->prog, &range))
>> + if (!get_func_retval_range(env->prog, range))
>> return 0;
>> /* no restricted range, any return value is allowed */
>> - if (range.minval == S32_MIN && range.maxval == S32_MAX)
>> + if (range->minval == S32_MIN && range->maxval == S32_MAX)
>
> Tangential to this refactoring. Looking at get_func_retval_range() it
> seems that S32_{MIN,MAX} special case can never happen.
> It defers to bpf_lsm_get_retval_range() which either does not set the
> range or it to [0, 1] or [-MAX_ERRNO, 0].
> Maybe remove it as a separate patch?
>
Ack, will do.
>> return 0;
>> - return_32bit = true;
>> + *return_32bit = true;
>
> Nit: maybe make this a special case in check_return_code(),
> as with enforce_attach_type_range?
> Or push 'return_32bit' to be a field in retval_range?
Ack, I think return_32bit as a field is cleaner conceptually. I will
keep the default value false to avoid churn on all retval_range()
call sites.
>
>> } else if (!env->prog->aux->attach_func_proto->type) {
>> /* Make sure programs that attach to void
>> * hooks don't try to modify return value.
>> */
>> - range = retval_range(1, 1);
>> + *range = retval_range(1, 1);
>> }
>> break;
>>
>> case BPF_PROG_TYPE_NETFILTER:
>> - range = retval_range(NF_DROP, NF_ACCEPT);
>> + *range = retval_range(NF_DROP, NF_ACCEPT);
>> break;
>> case BPF_PROG_TYPE_STRUCT_OPS:
>> - if (!ret_type)
>> - return 0;
>> - range = retval_range(0, 0);
>> + *range = retval_range(0, 0);
>> break;
>> case BPF_PROG_TYPE_EXT:
>> /* freplace program can return anything as its return value
>
> [...]
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-02-26 21:17 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-25 3:33 [PATCH bpf-next v4 0/4] bpf: Allow void return type for global subprogs Emil Tsalapatis
2026-02-25 3:33 ` [PATCH bpf-next v4 1/4] bpf: Factor out program return value calculation Emil Tsalapatis
2026-02-25 23:31 ` Eduard Zingerman
2026-02-26 21:17 ` Emil Tsalapatis
2026-02-25 3:33 ` [PATCH bpf-next v4 2/4] bpf: Separate bpf_throw() and bpf_exit() return value validation Emil Tsalapatis
2026-02-25 4:23 ` bot+bpf-ci
2026-02-26 1:48 ` Emil Tsalapatis
2026-02-26 4:56 ` Eduard Zingerman
2026-02-26 20:35 ` Emil Tsalapatis
2026-02-25 3:33 ` [PATCH bpf-next v4 3/4] bpf: Allow void global functions in the verifier Emil Tsalapatis
2026-02-25 3:33 ` [PATCH bpf-next v4 4/4] selftests: bpf: Add tests for void global subprogs Emil Tsalapatis
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox