Re: [PATCH bpf-next v4 07/13] bpf: Add verifier support for 16-byte returns in R0:R2

[Eduard Zingerman <eddyz87@gmail.com>]

On Thu, 2026-08-13 at 11:20 -0700, Yonghong Song wrote:

...

> > > @@ -19366,6 +19452,22 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
> > >   			return -EOPNOTSUPP;
> > >   		}
> > >
> > > +		/*
> > > +		 * An extension replaces the target outright, so it has to match
> > > +		 * the target's return convention. Its own return value is capped
> > > +		 * at 8 bytes (a >8 byte program return is rejected at BPF_EXIT),
> > > +		 * so it can never fill the R0:R2 pair the target's callers read.
> > > +		 * This cannot be left to btf_check_type_match() above, which
> > > +		 * compares return types by btf_type->info only: an int carries no
> > > +		 * vlen, so a 16-byte __int128 and an 8-byte long compare equal.
> > > +		 */
> > Should the btf_check_type_match() be fixed?
>
> The function btf_check_type_match() calls btf_check_func_type_match().
> In btf_check_func_type_match(), we have
>
>
>          t1 = btf_type_skip_modifiers(btf1, t1->type, NULL);
>          t2 = btf_type_skip_modifiers(btf2, t2->type, NULL);
>          if (t1->info != t2->info) {
>                  bpf_log(log,
>                          "Return type %s of %s() doesn't match type %s of %s()\n",
>                          btf_type_str(t1), fn1,
>                          btf_type_str(t2), fn2);
>                  return -EINVAL;
>          }
>
>          for (i = 0; i < nargs1; i++) {
> 		...
> 	}
>
> It only checked the t1->info vs. t2->info. For example t1->info and t2->info both
> have kind INT. But t1 and t2 may have different INT type (e.g. int vs. long)
> and this is allowed in btf_check_func_type_match().
> The same thing it also allows int vs. int128.
> The same for other kinds e.g. struct (some struct has smaller size and some struct
> has larger size).
>
> So I didn't use btf_check_type_match() and rather use tgt_info->fmodel.ret_size > 8
> where reject any prog returning more than 8 bytes.

Yes, I understand the mechanics. The question is whether
btf_check_type_match() needs adjustment. This function is used by a
single caller, bpf_check_attach_target():

	if (prog_extension &&
	    btf_check_type_match(log, prog, btf, t))
		return -EINVAL;

And just a few lines below this (and v5 of the series) adds:

+	if (prog_extension && tgt_info->fmodel.ret_size > 8) {
+		bpf_log(log,
+			"Cannot replace function %s with a >8 byte return value\n",
+			tname);
+		return -EOPNOTSUPP;
+	}

Given that the sole reason for btf_check_type_match() to exist is to
answer the question if an extension BPF subprogram is compatible with
the subprogram being extended, I thing this new check has to be moved
inside btf_check_type_match(). (E.g. by deeming that integers below
8-bytes are compatible between each other, but larger integers are not).