Re: [PATCH bpf-next v1 03/10] bpf: Verifier support for KF_IMPLICIT_ARGS

[Ihor Solodrai <ihor.solodrai@linux.dev>]

On 1/13/26 12:39 PM, Eduard Zingerman wrote:
> On Fri, 2026-01-09 at 10:48 -0800, Ihor Solodrai wrote:
>> A kernel function bpf_foo marked with KF_IMPLICIT_ARGS flag is
>> expected to have two associated types in BTF:
>>   * `bpf_foo` with a function prototype that omits implicit arguments
>>   * `bpf_foo_impl` with a function prototype that matches the kernel
>>      declaration of `bpf_foo`, but doesn't have a ksym associated with
>>      its name
>>
>> In order to support kfuncs with implicit arguments, the verifier has
>> to know how to resolve a call of `bpf_foo` to the correct BTF function
>> prototype and address.
>>
>> To implement this, in add_kfunc_call() kfunc flags are checked for
>> KF_IMPLICIT_ARGS. For such kfuncs a BTF func prototype is adjusted to
>> the one found for `bpf_foo_impl` (func_name + "_impl" suffix, by
>> convention) function in BTF.
>>
>> This effectively changes the signature of the `bpf_foo` kfunc in the
>> context of verification: from one without implicit args to the one
>> with full argument list.
>>
>> Whether a kfunc argument is implicit or not is determined by
>> is_kfunc_arg_implicit(). The values of implicit arguments by design
>> are provided by the verifier, and so they can only be of particular
>> types. In this patch the only allowed implicit arg type is a pointer
>> to struct bpf_prog_aux. The __prog args (usually void *) are also
>> considered implicit for backwards compatibility.
>>
>> In order to enable the verifier to correctly set an implicit
>> bpf_prog_aux arg value at runtime, is_kfunc_arg_prog() is extended to
>> check for the arg type. At a point when prog arg is determined in
>> check_kfunc_args() the kfunc with implicit args already has a
>> prototype with full argument list, so the existing value patch
>> mechanism just works.
>>
>> If a new kfunc with KF_IMPLICIT_ARG is declared for an existing kfunc
>> that uses a __prog argument (a legacy case), the prototype
>> substitution works in exactly the same way, assuming the kfunc follows
>> the _impl naming convention. The difference is only in how _impl
>> prototype is added to the BTF, which is not the verifier's
>> concern. See a subsequent resolve_btfids patch for details.
>>
>> In check_kfunc_call() reset the subreg_def of registers holding
>> implicit arguments to correctly track zero extensions.
>>
>> Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
>> ---
> 
> Overall lgtm.
> 
> [...]
> 
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> 
> [...]
> 
>> @@ -14303,6 +14358,17 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
>>  	for (i = 0; i < nargs; i++) {
>>  		u32 regno = i + 1;
>>  
>> +		/*
>> +		 * Implicit kfunc arguments are set after main verification pass.
>> +		 * For correct tracking of zero-extensions we have to reset subreg_def for such
>> +		 * args. Otherwise mark_btf_func_reg_size() will be inspecting subreg_def of regs
>> +		 * from an earlier (irrelevant) point in the program, which may lead to an error
>> +		 * in opt_subreg_zext_lo32_rnd_hi32().
>> +		 */
>> +		if (unlikely(KF_IMPLICIT_ARGS & meta.kfunc_flags
>> +				&& is_kfunc_arg_implicit(desc_btf, &args[i])))
>> +			regs[regno].subreg_def = DEF_NOT_SUBREG;
>> +
> 
> Did you try doing this in `mark_reg_not_init()`?
> This function is called for R1-R5 some time prior this hunk.

> Did you try doing this in `mark_reg_not_init()`?

Just tried, it doesn't work because REG0 is considered a caller saved
register, and so it breaks the zext tracking:

        #define CALLER_SAVED_REGS 6
        static const int caller_saved[CALLER_SAVED_REGS] = {
	     BPF_REG_0, BPF_REG_1, BPF_REG_2, BPF_REG_3, BPF_REG_4, BPF_REG_5
        };

        [...]

	for (i = 0; i < CALLER_SAVED_REGS; i++)
		mark_reg_not_init(env, regs, caller_saved[i]);

CI run for the diff below (on top of this series):
https://github.com/kernel-patches/bpf/actions/runs/20972520708


diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index b4e40b87e8fa..8bbcd1466815 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -2784,6 +2784,8 @@ static void __reg_assign_32_into_64(struct bpf_reg_state *reg)
        }
 }
 
+#define DEF_NOT_SUBREG (0)
+
 /* Mark a register as having a completely unknown (scalar) value. */
 static void __mark_reg_unknown_imprecise(struct bpf_reg_state *reg)
 {
@@ -2798,6 +2800,7 @@ static void __mark_reg_unknown_imprecise(struct bpf_reg_state *reg)
        reg->var_off = tnum_unknown;
        reg->frameno = 0;
        reg->precise = false;
+       reg->subreg_def = DEF_NOT_SUBREG;
        __mark_reg_unbounded(reg);
 }
 
@@ -2892,7 +2895,6 @@ static int mark_btf_ld_reg(struct bpf_verifier_env *env,
        }
 }
 
-#define DEF_NOT_SUBREG (0)
 static void init_reg_state(struct bpf_verifier_env *env,
                           struct bpf_func_state *state)
 {
@@ -14363,17 +14365,6 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
        for (i = 0; i < nargs; i++) {
                u32 regno = i + 1;
 
-               /*
-                * Implicit kfunc arguments are set after main verification pass.
-                * For correct tracking of zero-extensions we have to reset subreg_def for such
-                * args. Otherwise mark_btf_func_reg_size() will be inspecting subreg_def of regs
-                * from an earlier (irrelevant) point in the program, which may lead to an error
-                * in opt_subreg_zext_lo32_rnd_hi32().
-                */
-               if (unlikely(KF_IMPLICIT_ARGS & meta.kfunc_flags
-                               && is_kfunc_arg_implicit(desc_btf, &args[i])))
-                       regs[regno].subreg_def = DEF_NOT_SUBREG;
-
                t = btf_type_skip_modifiers(desc_btf, args[i].type, NULL);
                if (btf_type_is_ptr(t))
                        mark_btf_func_reg_size(env, regno, sizeof(void *));

---

Resetting all reg args appears to be working however (see below).
CI: https://github.com/kernel-patches/bpf/actions/runs/20973490221

Should I send this as a separate patch?

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 8bbcd1466815..9dfcf3149841 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -2800,7 +2800,6 @@ static void __mark_reg_unknown_imprecise(struct bpf_reg_state *reg)
        reg->var_off = tnum_unknown;
        reg->frameno = 0;
        reg->precise = false;
-       reg->subreg_def = DEF_NOT_SUBREG;
        __mark_reg_unbounded(reg);
 }
 
@@ -14241,6 +14240,11 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
        for (i = 0; i < CALLER_SAVED_REGS; i++)
                mark_reg_not_init(env, regs, caller_saved[i]);
 
+       for (i = 0; i < MAX_BPF_FUNC_REG_ARGS; i++) {
+               u32 regno = i + 1;
+               regs[regno].subreg_def = DEF_NOT_SUBREG;
+       }
+
        /* Check return type */
        t = btf_type_skip_modifiers(desc_btf, meta.func_proto->type, NULL);


> What I don't like from structural point of view is:
> - `is_kfunc_arg_implicit()` depends on KF_IMPLICIT_ARGS, but that
>   check is done externally. Hence, the naming is misleading or 'meta'
>   should be passed to `is_kfunc_arg_implicit()`.
> - doing DEF_NOT_SUBREG logically has not much to do with implicit args,
>   so it is a bit confusing that is pre-conditioned like that.
> 
>>  		t = btf_type_skip_modifiers(desc_btf, args[i].type, NULL);
>>  		if (btf_type_is_ptr(t))
>>  			mark_btf_func_reg_size(env, regno, sizeof(void *));