Re: [PATCH bpf-next v3 1/2] bpf: Allow void global functions in the verifier

["Emil Tsalapatis" <emil@etsalapatis.com>]

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
>> > 
>> > [...]