| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <3387829.aeNJFYEL58@7940hx>
Date: Fri, 19 Dec 2025 09:24:00 +0800
From: Menglong Dong <menglong.dong@...ux.dev>
To: Menglong Dong <menglong8.dong@...il.com>,
Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc: ast@...nel.org, andrii@...nel.org, davem@...emloft.net,
dsahern@...nel.org, daniel@...earbox.net, martin.lau@...ux.dev,
eddyz87@...il.com, song@...nel.org, yonghong.song@...ux.dev,
john.fastabend@...il.com, kpsingh@...nel.org, sdf@...ichev.me,
haoluo@...gle.com, jolsa@...nel.org, tglx@...utronix.de, mingo@...hat.com,
bp@...en8.de, dave.hansen@...ux.intel.com, x86@...nel.org, hpa@...or.com,
netdev@...r.kernel.org, bpf@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH bpf-next v4 1/9] bpf: add tracing session support
On 2025/12/19 08:55 Andrii Nakryiko <andrii.nakryiko@...il.com> write:
> On Wed, Dec 17, 2025 at 1:55 AM Menglong Dong <menglong8.dong@...il.com> wrote:
> >
> > The tracing session is something that similar to kprobe session. It allow
> > to attach a single BPF program to both the entry and the exit of the
> > target functions.
> >
> > Introduce the struct bpf_fsession_link, which allows to add the link to
> > both the fentry and fexit progs_hlist of the trampoline.
> >
> > Signed-off-by: Menglong Dong <dongml2@...natelecom.cn>
> > Co-developed-by: Leon Hwang <leon.hwang@...ux.dev>
> > Signed-off-by: Leon Hwang <leon.hwang@...ux.dev>
> > ---
> > v4:
> > - instead of adding a new hlist to progs_hlist in trampoline, add the bpf
> > program to both the fentry hlist and the fexit hlist.
> > ---
> > include/linux/bpf.h | 20 +++++++++++
> > include/uapi/linux/bpf.h | 1 +
> > kernel/bpf/btf.c | 2 ++
> > kernel/bpf/syscall.c | 18 +++++++++-
> > kernel/bpf/trampoline.c | 36 +++++++++++++++----
> > kernel/bpf/verifier.c | 12 +++++--
> > net/bpf/test_run.c | 1 +
> > net/core/bpf_sk_storage.c | 1 +
> > tools/include/uapi/linux/bpf.h | 1 +
> > .../bpf/prog_tests/tracing_failure.c | 2 +-
> > 10 files changed, 83 insertions(+), 11 deletions(-)
> >
>
> [...]
>
> > int bpf_prog_ctx_arg_info_init(struct bpf_prog *prog,
> > const struct bpf_ctx_arg_aux *info, u32 cnt);
> >
> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > index 84ced3ed2d21..696a7d37db0e 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -1145,6 +1145,7 @@ enum bpf_attach_type {
> > BPF_NETKIT_PEER,
> > BPF_TRACE_KPROBE_SESSION,
> > BPF_TRACE_UPROBE_SESSION,
> > + BPF_TRACE_SESSION,
>
> FSESSION for consistency with FENTRY and FEXIT
OK
>
> > __MAX_BPF_ATTACH_TYPE
> > };
> >
>
> [...]
>
> > {
> > - enum bpf_tramp_prog_type kind;
> > - struct bpf_tramp_link *link_exiting;
> > + enum bpf_tramp_prog_type kind, okind;
> > + struct bpf_tramp_link *link_existing;
> > + struct bpf_fsession_link *fslink;
> > int err = 0;
> > int cnt = 0, i;
> >
> > - kind = bpf_attach_type_to_tramp(link->link.prog);
> > + okind = kind = bpf_attach_type_to_tramp(link->link.prog);
> > if (tr->extension_prog)
> > /* cannot attach fentry/fexit if extension prog is attached.
> > * cannot overwrite extension prog either.
> > @@ -621,13 +624,18 @@ static int __bpf_trampoline_link_prog(struct bpf_tramp_link *link,
> > BPF_MOD_JUMP, NULL,
> > link->link.prog->bpf_func);
> > }
> > + if (kind == BPF_TRAMP_SESSION) {
> > + /* deal with fsession as fentry by default */
> > + kind = BPF_TRAMP_FENTRY;
> > + cnt++;
> > + }
>
> this "pretend we are BPF_TRAMP_FENTRY" looks a bit hacky and is very
> hard to follow. I think it would be cleaner to have explicit small
> special cases for BPF_TRAMP_SESSION, and then generalize
> hlist_for_each_entry case by using a local variable for storing
> &tr->progs_hlist[kind] (which for TRAMP_SESSION you'll set to
> &tr->progs_hlist[BPF_TRAMP_FENTRY]). You'll then just do extra
> hlist_add_head/hlist_del_init and count manipulation. IMO, it's better
> than keeping in head what kind and okind is...
Ah, the way now does seem a little hacky. I tried to make this series
looks less complex by modifying the code as less as possible.
I'll use a explicit way here as you advised.
>
>
> > if (cnt >= BPF_MAX_TRAMP_LINKS)
> > return -E2BIG;
> > if (!hlist_unhashed(&link->tramp_hlist))
> > /* prog already linked */
> > return -EBUSY;
> > - hlist_for_each_entry(link_exiting, &tr->progs_hlist[kind], tramp_hlist) {
> > - if (link_exiting->link.prog != link->link.prog)
> > + hlist_for_each_entry(link_existing, &tr->progs_hlist[kind], tramp_hlist) {
> > + if (link_existing->link.prog != link->link.prog)
> > continue;
> > /* prog already linked */
> > return -EBUSY;
>
> [...]
>
> > @@ -23298,6 +23299,7 @@ static int do_misc_fixups(struct bpf_verifier_env *env)
> > if (prog_type == BPF_PROG_TYPE_TRACING &&
> > insn->imm == BPF_FUNC_get_func_ret) {
> > if (eatype == BPF_TRACE_FEXIT ||
> > + eatype == BPF_TRACE_SESSION ||
> > eatype == BPF_MODIFY_RETURN) {
> > /* Load nr_args from ctx - 8 */
> > insn_buf[0] = BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, -8);
> > @@ -24242,7 +24244,8 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
> > if (tgt_prog->type == BPF_PROG_TYPE_TRACING &&
> > prog_extension &&
> > (tgt_prog->expected_attach_type == BPF_TRACE_FENTRY ||
> > - tgt_prog->expected_attach_type == BPF_TRACE_FEXIT)) {
> > + tgt_prog->expected_attach_type == BPF_TRACE_FEXIT ||
> > + tgt_prog->expected_attach_type == BPF_TRACE_SESSION)) {
> > /* Program extensions can extend all program types
> > * except fentry/fexit. The reason is the following.
> > * The fentry/fexit programs are used for performance
> > @@ -24257,7 +24260,7 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
> > * beyond reasonable stack size. Hence extending fentry
> > * is not allowed.
> > */
> > - bpf_log(log, "Cannot extend fentry/fexit\n");
> > + bpf_log(log, "Cannot extend fentry/fexit/session\n");
>
> fsession?
OK
Thanks!
Menglong Dong
>
> > return -EINVAL;
> > }
> > } else {
> > @@ -24341,6 +24344,7 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
> > case BPF_LSM_CGROUP:
> > case BPF_TRACE_FENTRY:
> > case BPF_TRACE_FEXIT:
> > + case BPF_TRACE_SESSION:
> > if (!btf_type_is_func(t)) {
> > bpf_log(log, "attach_btf_id %u is not a function\n",
> > btf_id);
>
> [...]
>
Powered by blists - more mailing lists