All of lore.kernel.org
 help / color / mirror / Atom feed
From: Slava Imameev <slava.imameev@crowdstrike.com>
To: <eddyz87@gmail.com>
Cc: <andrii@kernel.org>, <ast@kernel.org>, <bpf@vger.kernel.org>,
	<daniel@iogearbox.net>, <davem@davemloft.net>,
	<edumazet@google.com>, <haoluo@google.com>, <horms@kernel.org>,
	<john.fastabend@gmail.com>, <jolsa@kernel.org>,
	<kpsingh@kernel.org>, <kuba@kernel.org>,
	<linux-kernel@vger.kernel.org>, <linux-kselftest@vger.kernel.org>,
	<linux-open-source@crowdstrike.com>, <martin.lau@linux.dev>,
	<netdev@vger.kernel.org>, <pabeni@redhat.com>, <sdf@fomichev.me>,
	<shuah@kernel.org>, <slava.imameev@crowdstrike.com>,
	<song@kernel.org>, <yonghong.song@linux.dev>
Subject: Re: [PATCH bpf-next v4 1/2] bpf: Support new pointer param types via SCALAR_VALUE for trampolines
Date: Tue, 10 Mar 2026 23:16:59 +1100	[thread overview]
Message-ID: <20260310121659.25801-1-slava.imameev@crowdstrike.com> (raw)
In-Reply-To: <c4f4e99f83a98179c3413dd7afe5e7e73a98e4d3.camel@gmail.com>

On Tue, 03 Mar 2026 16:38:57 -0800, Eduard Zingerman wrote:
> On Wed, 2026-03-04 at 11:22 +1100, Slava Imameev wrote:
> > On Tue, 03 Mar 2026 14:43:01, Eduard Zingerman wrote:
> > > On Wed, 2026-03-04 at 08:49 +1100, Slava Imameev wrote:
> > > > 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;
> > > > > ?
> > > 
> > > Uh-oh, I copy-pasted the wrong snippet, sorry.
> > > The correct snippet is:
> > > 
> > >          if (btf_type_is_struct_ptr(btf, t))
> > >                   return true;
> > > 
> > > With it the selftests pass (except for `float` tests noted earlier).
> > > And regardless of selftests, the code below this point will
> > > error out if `t` is not a pointer to struct.
> > 
> > I think you tested with
> > 
> > 	if (!btf_type_is_struct_ptr(btf, t))
> > 		return true;
> > 
> > I decided on a narrower condition, as
> > 
> > - if (!btf_type_is_struct_ptr(btf, t)) -
> 
> Yes, sorry again.
> 
> > changes the existing selection condition from "treat only these types
> > as scalar" to "treat as scalar any type that is not a pointer to
> > structure". Technically both approaches cover the problem I'm trying
> > to solve - multilevel pointer support for structures, but the latter is
> > open-ended and changes the current approach, which checks for pointers
> > to int and void. So I'm extending this to int, void, enum 32/64,
> > function, and corresponding multilevel pointers to these types and
> > multilevel pointers to structures.
> 
> BTF is defined for the following non-modifier types:
> - void        [allowed already]
> - int         [allowed already]
> - ptr         [multi-level pointers allowed by your patch]
> - array       [disallowed?]
> - struct      [single level pointers allowed already,
> - union		   multi-level allowed by your patch]
> - enum/enum64 [allowed by your patch]
> - func_proto  [allowed by your patch]
> - float       [disallowed]
> 
> And a few not reachable from function fields (I think BTF validation
> checks that these can't be met, but would be good to double-check.
> If it doesn't, it should):
> - func
> - var
> - datasec
> 
> So, effectively you disallow reading from tracing context fields of
> type: struct (non-pointer), array, float and a few types that can't be
> specified for struct fields.
> 
> Does not seem necessary, tbh.

I verified whether PTR->FUNC, PTR->DATASEC, PTR->VAR can be passed to
btf_ctx_access() in the current mainline.

I added helpers that inject PTR->FUNC, PTR->DATASEC, PTR->VAR as pre or
post calls to btf_check_meta(). In all cases, the BPF program load
failed with errors "arg0 type FUNC / DATASEC / VAR is not a struct",
which indicates that btf_check_meta() can indeed be called with
PTR->FUNC, PTR->DATASEC, PTR->VAR.

If the condition for pointer check is changed to
`if (!btf_type_is_struct_ptr(btf, t))`, these BPF programs will load
successfully with arguments set to scalar().

Do we accept this change in behavior?

Test case with invalid BTF types injection:
https://github.com/slava-at-cs/bpf/commit/c49af6500ace4e4aceee01c570e3b067aae7e48c

Branch:
https://github.com/slava-at-cs/bpf/commits/inject-invalid-btf/

To run test:
./vmtest.sh -- ./test_progs -t verifier_btf_ctx_access

The verifier log:
=============
0: R1=ctx() R10=fp0
; asm volatile ("					\ @ verifier_btf_ctx_access.c:85
0: (79) r2 = *(u64 *)(r1 +0)
func 'bpf_fentry_test_invalid_ptr_func' arg0 type FUNC is not a struct
invalid bpf_context access off=0 size=8
processed 1 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0
=============

=============
0: R1=ctx() R10=fp0
; asm volatile ("					\ @ verifier_btf_ctx_access.c:85
0: (79) r2 = *(u64 *)(r1 +0)
func 'bpf_fentry_test_invalid_ptr_func' arg0 type DATASEC is not a struct
invalid bpf_context access off=0 size=8
processed 1 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0
=============

=============
0: R1=ctx() R10=fp0
; asm volatile ("					\ @ verifier_btf_ctx_access.c:85
0: (79) r2 = *(u64 *)(r1 +0)
func 'bpf_fentry_test_invalid_ptr_func' arg0 type VAR is not a struct
invalid bpf_context access off=0 size=8
processed 1 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0
=============

  reply	other threads:[~2026-03-10 12:17 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-03  9:54 [PATCH bpf-next v4 0/2] bpf: Add multi-level pointer parameter support for trampolines Slava Imameev
2026-03-03  9:54 ` [PATCH bpf-next v4 1/2] bpf: Support new pointer param types via SCALAR_VALUE " Slava Imameev
2026-03-03 20:05   ` Eduard Zingerman
2026-03-03 21:49     ` Slava Imameev
2026-03-03 22:43       ` Eduard Zingerman
2026-03-04  0:22         ` Slava Imameev
2026-03-04  0:36           ` Alexei Starovoitov
2026-03-04  0:38           ` Eduard Zingerman
2026-03-10 12:16             ` Slava Imameev [this message]
2026-03-10 18:52               ` Eduard Zingerman
2026-03-11 13:07                 ` Slava Imameev
2026-03-11 16:31                   ` Eduard Zingerman
2026-03-03  9:54 ` [PATCH bpf-next v4 2/2] selftests/bpf: Add trampolines single and multi-level pointer params test coverage Slava Imameev
2026-03-03 20:08   ` Eduard Zingerman
2026-03-03 22:14     ` Slava Imameev

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260310121659.25801-1-slava.imameev@crowdstrike.com \
    --to=slava.imameev@crowdstrike.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=eddyz87@gmail.com \
    --cc=edumazet@google.com \
    --cc=haoluo@google.com \
    --cc=horms@kernel.org \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=kpsingh@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-open-source@crowdstrike.com \
    --cc=martin.lau@linux.dev \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=sdf@fomichev.me \
    --cc=shuah@kernel.org \
    --cc=song@kernel.org \
    --cc=yonghong.song@linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.