Re: [PATCH bpf-next v9 01/11] bpf: add fsession support

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

On 2026/1/14 09:22 Andrii Nakryiko <andrii.nakryiko@gmail.com> write:
> On Sat, Jan 10, 2026 at 6:11 AM Menglong Dong <menglong8.dong@gmail.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.

> 
> >                         /* 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]--;
> 
> [...]
>