On 2026-03-03 20:05 UTC, Eduard Zingerman wrote:
> > @@ -6902,11 +6921,7 @@ bool btf_ctx_access(int off, int size, enum
> > bpf_access_type type,
> > }
> > }
> >
> > - /*
> > - * If it's a pointer to void, it's the same as scalar from the
> > verifier
> > - * safety POV. Either way, no futher pointer walking is allowed.
> > - */
> > - if (is_void_or_int_ptr(btf, t))
> > + if (is_ptr_treated_as_scalar(btf, t))
> > return true;
>
> I'm probably missing a point here, but what's wrong with Alexei's
> suggestion to do this instead:
>
> if (is_ptr_treated_as_scalar(btf, t))
> return true;
> ?
This reflects my belief in a cautious approach: adding support
only for selected types with tests added for each new type. That said,
I can add the suggested broader condition and make it pass the tests,
but I cannot be sure it will be future-proof against conflicts.
I think the broader check like
/* skip modifiers */
tt = t;
while (btf_type_is_modifier(tt))
tt = btf_type_by_id(btf, tt->type);
if (!btf_type_is_struct(tt))
return true;
might have some incompatibility with future changes, compared to
explicit type checks for selected types. This condition is
open-ended, including anything instead of selecting specific types.
This broader check also needs to be moved down closer to the exit
from btf_ctx_access; otherwise, btf_ctx_access can exit early
without executing the following code. In my case, this resulted in
existing test failures if the above !btf_type_is_struct(tt) replaces
current master's branch condition
if (is_void_or_int_ptr(btf, t))
return true;
The result for:
./vmtest.sh -- ./test_progs
was:
Summary: 617/5770 PASSED, 80 SKIPPED, 82 FAILED
with a lot of:
unexpected_load_success
Compared to:
Summary: 692/6045 PASSED, 80 SKIPPED, 7 FAILED
for the master branch.
As I noted this diff, closer to the exit from btf_ctx_access,
makes tests to pass:
if (!btf_type_is_struct(t)) {
- bpf_log(log,
- "func '%s' arg%d type %s is not a struct\n",
- tname, arg, btf_type_str(t));
- return false;
+ info->reg_type = SCALAR_VALUE;
+ return true;
}
> Only two new tests fail:
> - #554/62 verifier_ctx_ptr_param/fentry/pointer to float - invalid ctx
> access:FAIL
> - #554/63 verifier_ctx_ptr_param/fentry/double pointer to float - invalid
> ctx access:FAIL
> But I'd say this shouldn't matter.
> This will also make selftests much simpler.
Yes, I decided not to add support for pointers to float.
