Re: [PATCH bpf-next v2 06/18] bpf: tracing: add support to record and check the accessed args

[Menglong Dong <menglong.dong@linux.dev>]

[-- Attachment #1: Type: text/plain, Size: 2126 bytes --]

On Wednesday, July 16, 2025 1:11 AM Andrii Nakryiko <andrii.nakryiko@gmail.com> write:
> On Mon, Jul 14, 2025 at 4:45 PM Menglong Dong <menglong.dong@linux.dev> wrote:
> >
> >
> > On 2025/7/15 06:07, Andrii Nakryiko wrote:
> > > On Thu, Jul 3, 2025 at 5:20 AM Menglong Dong <menglong8.dong@gmail.com> wrote:
> > >> In this commit, we add the 'accessed_args' field to struct bpf_prog_aux,
> > >> which is used to record the accessed index of the function args in
> > >> btf_ctx_access().
> > > Do we need to bother giving access to arguments through direct ctx[i]
> > > access for these multi-fentry/fexit programs? We have
> > > bpf_get_func_arg_cnt() and bpf_get_func_arg() which can be used to get
> > > any given argument at runtime.
> >
> >
> > Hi Andrii. This commit is not for that purpose. We remember all the accessed
> > args to bpf_prog_aux->accessed_args. And when we attach the tracing-multi
> > prog to the kernel functions, we will check if the accessed arguments are
> > consistent between all the target functions.
> >
> > The bpf_prog_aux->accessed_args will be used in
> > https://lore.kernel.org/bpf/20250703121521.1874196-12-dongml2@chinatelecom.cn/
> >
> > in bpf_tracing_check_multi() to do such checking.
> >
> > With such checking, the target functions don't need to have
> > the same prototype, which makes tracing-multi more flexible.
> 
> Yeah, and my point is why even track this at verifier level. If we
> don't allow direct ctx[i] access and only access arguments through
> bpf_get_func_arg(), we can check actual number of arguments at runtime
> and if program is trying to access something that's not there, we'll
> just return error code, so user can handle this generically.
> 
> I'm just not sure if there is a need to do anything more than that.

This commit is for the ctx[i] direct access, and we can use
bpf_core_cast() instead, as you said in
https://lore.kernel.org/bpf/CADxym3Zrqb6MxoV6mg4ioQMCiR+Cden9tmD5YHj8DtRFjn14HA@mail.gmail.com/T/#m7daa262d423c0e8bb1c7033e51099ef06180d2c5

Which means that we don't need this commit any more.



[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 488 bytes --]