| 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: <2251274.irdbgypaU6@7940hx>
Date: Fri, 02 Jan 2026 17:21:42 +0800
From: Menglong Dong <menglong.dong@...ux.dev>
To: Menglong Dong <menglong8.dong@...il.com>, Jiri Olsa <olsajiri@...il.com>
Cc: 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, 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 v5 01/10] bpf: add fsession support
On 2026/1/1 21:52 Jiri Olsa <olsajiri@...il.com> write:
> On Wed, Dec 24, 2025 at 09:07:26PM +0800, Menglong Dong wrote:
>
> SNIP
Hi, Jiri. Happy New Year!
>
> > +struct bpf_fsession_link {
> > + struct bpf_tracing_link link;
> > + struct bpf_tramp_link fexit;
> > +};
> > +
> > struct bpf_raw_tp_link {
> > struct bpf_link link;
> > struct bpf_raw_event_map *btp;
> > @@ -2114,6 +2120,20 @@ static inline void bpf_struct_ops_desc_release(struct bpf_struct_ops_desc *st_op
> >
> > #endif
> >
> > +static inline int bpf_fsession_cnt(struct bpf_tramp_links *links)
> > +{
> > + struct bpf_tramp_links fentries = links[BPF_TRAMP_FENTRY];
> > + int cnt = 0;
> > +
> > + for (int i = 0; i < links[BPF_TRAMP_FENTRY].nr_links; i++) {
> > + if (fentries.links[i]->link.prog->expected_attach_type ==
> > + BPF_TRACE_FSESSION)
>
> let's keep it on the single line ?
OK
>
> > + cnt++;
> > + }
> > +
> > + return cnt;
> > +}
> > +
[......]
> > @@ -3628,7 +3629,21 @@ static int bpf_tracing_prog_attach(struct bpf_prog *prog,
> > key = bpf_trampoline_compute_key(tgt_prog, NULL, btf_id);
> > }
> >
> > - link = kzalloc(sizeof(*link), GFP_USER);
> > + if (prog->expected_attach_type == BPF_TRACE_FSESSION) {
> > + struct bpf_fsession_link *fslink;
> > +
> > + fslink = kzalloc(sizeof(*fslink), GFP_USER);
> > + if (fslink) {
> > + bpf_link_init(&fslink->fexit.link, BPF_LINK_TYPE_TRACING,
> > + &bpf_tracing_link_lops, prog, attach_type);
>
> I don't think we need the extra exit struct bpf_link, we just need
> hlist_node hook for exit program, so this should perhaps be:
>
> struct bpf_fsession_link {
> struct bpf_tracing_link link;
> struct hlist_node tramp_hlist;
> };
I think we can't do it this way according to how we manager
the bpf_link in trampoline, as you can see in
bpf_trampoline_get_progs() and the struct of bpf_tramp_links.
In bpf_trampoline_get_progs(), it will lookup all the bpf_link
in the trampoline. If we simply add the bpf_fsession_link->tramp_hlist,
the struct in the progs_hlist will be inconsistent.
>
>
> SNIP
>
> > @@ -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;
> > + struct hlist_head *prog_list;
> > int err = 0;
> > int cnt = 0, i;
> >
> > @@ -621,24 +625,44 @@ static int __bpf_trampoline_link_prog(struct bpf_tramp_link *link,
> > BPF_MOD_JUMP, NULL,
> > link->link.prog->bpf_func);
> > }
> > + if (kind == BPF_TRAMP_FSESSION) {
> > + prog_list = &tr->progs_hlist[BPF_TRAMP_FENTRY];
> > + cnt++;
> > + } else {
> > + prog_list = &tr->progs_hlist[kind];
> > + }
> > 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) {
> > + hlist_for_each_entry(link_exiting, prog_list, tramp_hlist) {
> > if (link_exiting->link.prog != link->link.prog)
> > continue;
> > /* prog already linked */
> > return -EBUSY;
> > }
> >
> > - 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]);
> > + 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;
>
> this seems confusing, how about we just add abolish bpf_fsession_link
It was more confusing in V1. I adopted Andrii's suggestion in
this version to make the logic here more clear. But it seems
still confusing :/
Maybe we need more document here to help the understanding.
> and add extra hlist_node hook to struct bpf_tramp_link .. we will waste
> 16 bytes for other cases, but the code seems less confusing to me
>
> untested, so I might overlooked something..
>
> jirka
>
>
>
> ---
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 4e7d72dfbcd4..7479664844ea 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -1309,6 +1309,7 @@ enum bpf_tramp_prog_type {
> BPF_TRAMP_MODIFY_RETURN,
> BPF_TRAMP_MAX,
> BPF_TRAMP_REPLACE, /* more than MAX */
> + BPF_TRAMP_FSESSION,
> };
>
> struct bpf_tramp_image {
> @@ -1861,6 +1862,7 @@ struct bpf_link_ops {
> struct bpf_tramp_link {
> struct bpf_link link;
> struct hlist_node tramp_hlist;
> + struct hlist_node extra_hlist;
> u64 cookie;
> };
In this way, it indeed can make the update of the hlist more clear. However,
I think that you missed the reading of the hlist as I mentioned above.
You can't add both the "tramp_hlist" and "extra_hlist" to the same hlist. If
so, how do we iterate the hlist? Do I miss something?
Thanks!
Menglong Dong
>
[......]
> void test_tracing_failure(void)
>
>
>
Powered by blists - more mailing lists