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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAEf4Bza84H=FL-KxJEFAn6pFpVBQVnvrpif6_gtf_SWHH4pRJQ@mail.gmail.com>
Date: Wed, 14 Jan 2026 10:56:16 -0800
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Menglong Dong <menglong.dong@...ux.dev>
Cc: Menglong Dong <menglong8.dong@...il.com>, ast@...nel.org, andrii@...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, 
	davem@...emloft.net, dsahern@...nel.org, tglx@...utronix.de, mingo@...hat.com, 
	jiang.biao@...ux.dev, bp@...en8.de, dave.hansen@...ux.intel.com, 
	x86@...nel.org, hpa@...or.com, bpf@...r.kernel.org, netdev@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH bpf-next v9 01/11] bpf: add fsession support

On Tue, Jan 13, 2026 at 6:11 PM Menglong Dong <menglong.dong@...ux.dev> wrote:
>
> On 2026/1/14 09:22 Andrii Nakryiko <andrii.nakryiko@...il.com> write:
> > On Sat, Jan 10, 2026 at 6:11 AM Menglong Dong <menglong8.dong@...il.com> wrote:
> > >
> > > The fsession 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.
> > >
> [...]
> > > --- a/kernel/bpf/btf.c
> > > +++ b/kernel/bpf/btf.c
> > > @@ -6107,6 +6107,7 @@ static int btf_validate_prog_ctx_type(struct bpf_verifier_log *log, const struct
> > >                 case BPF_TRACE_FENTRY:
> > >                 case BPF_TRACE_FEXIT:
> > >                 case BPF_MODIFY_RETURN:
> > > +               case BPF_TRACE_FSESSION:
> > >                         /* allow u64* as ctx */
> > >                         if (btf_is_int(t) && t->size == 8)
> > >                                 return 0;
> > > @@ -6704,6 +6705,7 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
> > >                         fallthrough;
> > >                 case BPF_LSM_CGROUP:
> > >                 case BPF_TRACE_FEXIT:
> > > +               case BPF_TRACE_FSESSION:
> >
> > According to the comment below we make this exception due to LSM.
> > FSESSION won't be using FSESSION programs, no? So this is not
> > necessary?
>
> The comment describe the LSM case here, but the code
> here is not only for LSM. It is also for FEXIT, which makes
> sure that we can get the return value with "ctx[nr_args]".
> So I think we still need it here, as we need to access the
> return value with "ctx[nr_args]" too.

please update the comment then as well

>
> >
> > >                         /* When LSM programs are attached to void LSM hooks
> > >                          * they use FEXIT trampolines and when attached to
> > >                          * int LSM hooks, they use MODIFY_RETURN trampolines.
> >
> > [...]
> >
> > > @@ -4350,6 +4365,7 @@ attach_type_to_prog_type(enum bpf_attach_type attach_type)
> > >         case BPF_TRACE_RAW_TP:
> > >         case BPF_TRACE_FENTRY:
> > >         case BPF_TRACE_FEXIT:
> > > +       case BPF_TRACE_FSESSION:
> > >         case BPF_MODIFY_RETURN:
> > >                 return BPF_PROG_TYPE_TRACING;
> > >         case BPF_LSM_MAC:
> > > diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> > > index 2a125d063e62..11e043049d68 100644
> > > --- a/kernel/bpf/trampoline.c
> > > +++ b/kernel/bpf/trampoline.c
> > > @@ -111,7 +111,7 @@ bool bpf_prog_has_trampoline(const struct bpf_prog *prog)
> > >
> > >         return (ptype == BPF_PROG_TYPE_TRACING &&
> > >                 (eatype == BPF_TRACE_FENTRY || eatype == BPF_TRACE_FEXIT ||
> > > -                eatype == BPF_MODIFY_RETURN)) ||
> > > +                eatype == BPF_MODIFY_RETURN || eatype == BPF_TRACE_FSESSION)) ||
> > >                 (ptype == BPF_PROG_TYPE_LSM && eatype == BPF_LSM_MAC);
> >
> > this is getting crazy, switch to the switch (lol) maybe?
>
> ACK
>
> >
> > >  }
> > >
> > > @@ -559,6 +559,8 @@ static enum bpf_tramp_prog_type bpf_attach_type_to_tramp(struct bpf_prog *prog)
> > >                 return BPF_TRAMP_MODIFY_RETURN;
> > >         case BPF_TRACE_FEXIT:
> > >                 return BPF_TRAMP_FEXIT;
> > > +       case BPF_TRACE_FSESSION:
> > > +               return BPF_TRAMP_FSESSION;
> > >         case BPF_LSM_MAC:
> > >                 if (!prog->aux->attach_func_proto->type)
> > >                         /* The function returns void, we cannot modify its
> > > @@ -596,6 +598,8 @@ static int __bpf_trampoline_link_prog(struct bpf_tramp_link *link,
> > >  {
> > >         enum bpf_tramp_prog_type kind;
> > >         struct bpf_tramp_link *link_exiting;
> > > +       struct bpf_fsession_link *fslink;
> >
> > initialize to NULL to avoid compiler (falsely, but still) complaining
> > about potentially using uninitialized value
>
> ACK
>
> >
> > > +       struct hlist_head *prog_list;
> > >         int err = 0;
> > >         int cnt = 0, i;
> > >
> >
> > [...]
> >
> > > -       hlist_add_head(&link->tramp_hlist, &tr->progs_hlist[kind]);
> > > -       tr->progs_cnt[kind]++;
> > > +       hlist_add_head(&link->tramp_hlist, prog_list);
> > > +       if (kind == BPF_TRAMP_FSESSION) {
> > > +               tr->progs_cnt[BPF_TRAMP_FENTRY]++;
> > > +               fslink = container_of(link, struct bpf_fsession_link, link.link);
> > > +               hlist_add_head(&fslink->fexit.tramp_hlist,
> > > +                              &tr->progs_hlist[BPF_TRAMP_FEXIT]);
> >
> > fits under 100 characters? keep on a single line then
>
> ACK
>
> >
> > > +               tr->progs_cnt[BPF_TRAMP_FEXIT]++;
> > > +       } else {
> > > +               tr->progs_cnt[kind]++;
> > > +       }
> > >         err = bpf_trampoline_update(tr, true /* lock_direct_mutex */);
> > >         if (err) {
> > >                 hlist_del_init(&link->tramp_hlist);
> > > -               tr->progs_cnt[kind]--;
> > > +               if (kind == BPF_TRAMP_FSESSION) {
> > > +                       tr->progs_cnt[BPF_TRAMP_FENTRY]--;
> > > +                       hlist_del_init(&fslink->fexit.tramp_hlist);
> > > +                       tr->progs_cnt[BPF_TRAMP_FEXIT]--;
> > > +               } else {
> > > +                       tr->progs_cnt[kind]--;
> > > +               }
> > >         }
> > >         return err;
> > >  }
> > > @@ -659,6 +683,7 @@ static int __bpf_trampoline_unlink_prog(struct bpf_tramp_link *link,
> > >                                         struct bpf_trampoline *tr,
> > >                                         struct bpf_prog *tgt_prog)
> > >  {
> > > +       struct bpf_fsession_link *fslink;
> >
> > used in only one branch, move declaration there?
>
> ACK
>
> Thanks!
> Menglong Dong
>
> >
> > >         enum bpf_tramp_prog_type kind;
> > >         int err;
> > >
> > > @@ -672,6 +697,11 @@ static int __bpf_trampoline_unlink_prog(struct bpf_tramp_link *link,
> > >                 guard(mutex)(&tgt_prog->aux->ext_mutex);
> > >                 tgt_prog->aux->is_extended = false;
> > >                 return err;
> > > +       } else if (kind == BPF_TRAMP_FSESSION) {
> > > +               fslink = container_of(link, struct bpf_fsession_link, link.link);
> > > +               hlist_del_init(&fslink->fexit.tramp_hlist);
> > > +               tr->progs_cnt[BPF_TRAMP_FEXIT]--;
> > > +               kind = BPF_TRAMP_FENTRY;
> > >         }
> > >         hlist_del_init(&link->tramp_hlist);
> > >         tr->progs_cnt[kind]--;
> >
> > [...]
> >
>
>
>
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ