* [PATCH bpf-next v2 0/2] bpf: Add LINK_DETACH for perf links @ 2026-03-04 21:02 Florian Lehner 2026-03-04 21:02 ` [PATCH bpf-next v2 1/2] bpf: Add LINK_DETACH support for perf link Florian Lehner 2026-03-04 21:02 ` [PATCH bpf-next v2 2/2] selftests/bpf: Test LINK_DETACH " Florian Lehner 0 siblings, 2 replies; 7+ messages in thread From: Florian Lehner @ 2026-03-04 21:02 UTC (permalink / raw) To: bpf Cc: ast, daniel, john.fastabend, andrii, martin.lau, eddyz87, song, yonghong.song, kpsingh, sdf, haoluo, jolsa, shuah, davem, kuba, hawk, Florian Lehner 73b11c2a introduced LINK_DETACH and implemented it for some link types, like xdp, netns and others. This patch implements LINK_DETACH for perf links, re-using existing link release handling code. Changes since initial approach [1]: - drop LINK_DETACH support for iter - add test for LINK_DETACH for perf event links [1] https://lore.kernel.org/bpf/aJOhPoTLdYnZmHYA@der-flo.net/ Florian Lehner (2): bpf: Add LINK_DETACH support for perf link selftests/bpf: Test LINK_DETACH for perf link kernel/bpf/syscall.c | 30 ++++++- .../selftests/bpf/prog_tests/perf_link.c | 81 ++++++++++++++++--- 2 files changed, 94 insertions(+), 17 deletions(-) -- 2.53.0 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH bpf-next v2 1/2] bpf: Add LINK_DETACH support for perf link 2026-03-04 21:02 [PATCH bpf-next v2 0/2] bpf: Add LINK_DETACH for perf links Florian Lehner @ 2026-03-04 21:02 ` Florian Lehner 2026-03-04 22:01 ` bot+bpf-ci 2026-03-04 21:02 ` [PATCH bpf-next v2 2/2] selftests/bpf: Test LINK_DETACH " Florian Lehner 1 sibling, 1 reply; 7+ messages in thread From: Florian Lehner @ 2026-03-04 21:02 UTC (permalink / raw) To: bpf Cc: ast, daniel, john.fastabend, andrii, martin.lau, eddyz87, song, yonghong.song, kpsingh, sdf, haoluo, jolsa, shuah, davem, kuba, hawk, Florian Lehner Add bpf_perf_link_detach() to allow detaching a BPF program from its perf event via BPF_LINK_DETACH while keeping the link file descriptor alive. This mirrors the existing behavior for xdp and cgroup links and enables temporarily disabling uprobes or other perf event-based programs that are attached via bpf_perf_links without closing the link. bpf_perf_link_release() is made idempotent using xchg() so that closing the link fd after an explicit LINK_DETACH does not call perf_event_free_bpf_prog() a second time. bpf_perf_link_fill_link_info() and bpf_perf_link_show_fdinfo() gain NULL guards so that querying an already-detached link returns an empty result rather than dereferencing a stale pointer. Signed-off-by: Florian Lehner <dev@der-flo.net> --- kernel/bpf/syscall.c | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index 274039e36465..7d567cd20a3f 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -3863,10 +3863,23 @@ struct bpf_perf_link { static void bpf_perf_link_release(struct bpf_link *link) { struct bpf_perf_link *perf_link = container_of(link, struct bpf_perf_link, link); - struct perf_event *event = perf_link->perf_file->private_data; + struct perf_event *event; + struct file *perf_file; + + /* bpf_perf_link_detach() may have already released perf_file */ + perf_file = xchg(&perf_link->perf_file, NULL); + if (!perf_file) + return; + event = perf_file->private_data; perf_event_free_bpf_prog(event); - fput(perf_link->perf_file); + fput(perf_file); +} + +static int bpf_perf_link_detach(struct bpf_link *link) +{ + bpf_perf_link_release(link); + return 0; } static void bpf_perf_link_dealloc(struct bpf_link *link) @@ -4075,9 +4088,13 @@ static int bpf_perf_link_fill_link_info(const struct bpf_link *link, { struct bpf_perf_link *perf_link; const struct perf_event *event; + struct file *perf_file; perf_link = container_of(link, struct bpf_perf_link, link); - event = perf_get_event(perf_link->perf_file); + perf_file = READ_ONCE(perf_link->perf_file); + if (!perf_file) + return 0; + event = perf_get_event(perf_file); if (IS_ERR(event)) return PTR_ERR(event); @@ -4143,9 +4160,13 @@ static void bpf_perf_link_show_fdinfo(const struct bpf_link *link, { struct bpf_perf_link *perf_link; const struct perf_event *event; + struct file *perf_file; perf_link = container_of(link, struct bpf_perf_link, link); - event = perf_get_event(perf_link->perf_file); + perf_file = READ_ONCE(perf_link->perf_file); + if (!perf_file) + return; + event = perf_get_event(perf_file); if (IS_ERR(event)) return; @@ -4163,6 +4184,7 @@ static void bpf_perf_link_show_fdinfo(const struct bpf_link *link, static const struct bpf_link_ops bpf_perf_link_lops = { .release = bpf_perf_link_release, + .detach = bpf_perf_link_detach, .dealloc = bpf_perf_link_dealloc, .fill_link_info = bpf_perf_link_fill_link_info, .show_fdinfo = bpf_perf_link_show_fdinfo, -- 2.53.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next v2 1/2] bpf: Add LINK_DETACH support for perf link 2026-03-04 21:02 ` [PATCH bpf-next v2 1/2] bpf: Add LINK_DETACH support for perf link Florian Lehner @ 2026-03-04 22:01 ` bot+bpf-ci 2026-03-06 3:38 ` Kumar Kartikeya Dwivedi 0 siblings, 1 reply; 7+ messages in thread From: bot+bpf-ci @ 2026-03-04 22:01 UTC (permalink / raw) To: dev, bpf Cc: ast, daniel, john.fastabend, andrii, martin.lau, eddyz87, song, yonghong.song, kpsingh, sdf, haoluo, jolsa, shuah, davem, kuba, hawk, dev, ast, andrii, daniel, martin.lau, eddyz87, yonghong.song, clm, ihor.solodrai [-- Attachment #1: Type: text/plain, Size: 3544 bytes --] > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c > --- a/kernel/bpf/syscall.c > +++ b/kernel/bpf/syscall.c > @@ -3863,10 +3863,23 @@ struct bpf_perf_link { > static void bpf_perf_link_release(struct bpf_link *link) > { > struct bpf_perf_link *perf_link = container_of(link, struct bpf_perf_link, link); > - struct perf_event *event = perf_link->perf_file->private_data; > + struct perf_event *event; > + struct file *perf_file; > + > + /* bpf_perf_link_detach() may have already released perf_file */ > + perf_file = xchg(&perf_link->perf_file, NULL); > + if (!perf_file) > + return; > + > + event = perf_file->private_data; > perf_event_free_bpf_prog(event); > - fput(perf_link->perf_file); > + fput(perf_file); > +} [ ... ] > @@ -4075,9 +4088,13 @@ static int bpf_perf_link_fill_link_info(const struct bpf_link *link, > { > struct bpf_perf_link *perf_link; > const struct perf_event *event; > + struct file *perf_file; > + > perf_link = container_of(link, struct bpf_perf_link, link); > - event = perf_get_event(perf_link->perf_file); > + perf_file = READ_ONCE(perf_link->perf_file); > + if (!perf_file) > + return 0; > + event = perf_get_event(perf_file); Could bpf_perf_link_fill_link_info() race with bpf_perf_link_release() here? After READ_ONCE reads a non-NULL perf_file, no reference is taken on the file. A concurrent BPF_LINK_DETACH on another CPU can call bpf_perf_link_release(), which does xchg() followed by fput(). If the perf event fd has already been closed by userspace, fput() drops the last reference. Once the detaching thread returns to userspace, task_work runs __fput and frees the file struct. At that point perf_get_event() dereferences file->f_op on freed memory: Thread A (BPF_OBJ_GET_INFO_BY_FD) Thread B (BPF_LINK_DETACH) -------------------------------- -------------------------- perf_file = READ_ONCE(...) [non-NULL] xchg(&..., NULL) fput(perf_file) return to userspace task_work -> __fput -> free perf_get_event(perf_file) -> UAF The xdp and cgroup link types handle this by serializing fill_link_info and release under a shared lock (rtnl_lock and cgroup_lock respectively): kernel/bpf/cgroup.c:bpf_cgroup_link_fill_link_info() { cgroup_lock(); if (cg_link->cgroup) cg_id = cgroup_id(cg_link->cgroup); cgroup_unlock(); } net/core/dev.c:bpf_xdp_link_fill_link_info() { rtnl_lock(); if (xdp_link->dev) ifindex = xdp_link->dev->ifindex; rtnl_unlock(); } The same issue applies to bpf_perf_link_show_fdinfo() below. > @@ -4143,9 +4160,13 @@ static void bpf_perf_link_show_fdinfo(const struct bpf_link *link, > { > struct bpf_perf_link *perf_link; > const struct perf_event *event; > + struct file *perf_file; > + > perf_link = container_of(link, struct bpf_perf_link, link); > - event = perf_get_event(perf_link->perf_file); > + perf_file = READ_ONCE(perf_link->perf_file); > + if (!perf_file) > + return; > + event = perf_get_event(perf_file); Same race as bpf_perf_link_fill_link_info() above -- a concurrent BPF_LINK_DETACH can free perf_file between the READ_ONCE and the perf_get_event() call. --- AI reviewed your patch. Please fix the bug or email reply why it's not a bug. See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md CI run summary: https://github.com/kernel-patches/bpf/actions/runs/22690413273 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next v2 1/2] bpf: Add LINK_DETACH support for perf link 2026-03-04 22:01 ` bot+bpf-ci @ 2026-03-06 3:38 ` Kumar Kartikeya Dwivedi 2026-03-06 3:59 ` Alexei Starovoitov 0 siblings, 1 reply; 7+ messages in thread From: Kumar Kartikeya Dwivedi @ 2026-03-06 3:38 UTC (permalink / raw) To: bot+bpf-ci, ast Cc: dev, bpf, daniel, john.fastabend, andrii, martin.lau, eddyz87, song, yonghong.song, kpsingh, sdf, haoluo, jolsa, shuah, davem, kuba, hawk, martin.lau, clm, ihor.solodrai On Wed, 4 Mar 2026 at 23:01, <bot+bpf-ci@kernel.org> wrote: > > > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c > > --- a/kernel/bpf/syscall.c > > +++ b/kernel/bpf/syscall.c > > @@ -3863,10 +3863,23 @@ struct bpf_perf_link { > > static void bpf_perf_link_release(struct bpf_link *link) > > { > > struct bpf_perf_link *perf_link = container_of(link, struct bpf_perf_link, link); > > - struct perf_event *event = perf_link->perf_file->private_data; > > + struct perf_event *event; > > + struct file *perf_file; > > + > > + /* bpf_perf_link_detach() may have already released perf_file */ > > + perf_file = xchg(&perf_link->perf_file, NULL); > > + if (!perf_file) > > + return; > > + > > + event = perf_file->private_data; > > perf_event_free_bpf_prog(event); > > - fput(perf_link->perf_file); > > + fput(perf_file); > > +} > > [ ... ] > > > @@ -4075,9 +4088,13 @@ static int bpf_perf_link_fill_link_info(const struct bpf_link *link, > > { > > struct bpf_perf_link *perf_link; > > const struct perf_event *event; > > + struct file *perf_file; > > + > > perf_link = container_of(link, struct bpf_perf_link, link); > > - event = perf_get_event(perf_link->perf_file); > > + perf_file = READ_ONCE(perf_link->perf_file); > > + if (!perf_file) > > + return 0; > > + event = perf_get_event(perf_file); > > Could bpf_perf_link_fill_link_info() race with bpf_perf_link_release() > here? > > After READ_ONCE reads a non-NULL perf_file, no reference is taken on > the file. A concurrent BPF_LINK_DETACH on another CPU can call > bpf_perf_link_release(), which does xchg() followed by fput(). If > the perf event fd has already been closed by userspace, fput() drops > the last reference. Once the detaching thread returns to userspace, > task_work runs __fput and frees the file struct. > > At that point perf_get_event() dereferences file->f_op on freed > memory: > > Thread A (BPF_OBJ_GET_INFO_BY_FD) Thread B (BPF_LINK_DETACH) > -------------------------------- -------------------------- > perf_file = READ_ONCE(...) [non-NULL] > xchg(&..., NULL) > fput(perf_file) > return to userspace > task_work -> __fput -> free > perf_get_event(perf_file) -> UAF > > The xdp and cgroup link types handle this by serializing > fill_link_info and release under a shared lock (rtnl_lock and > cgroup_lock respectively): > > kernel/bpf/cgroup.c:bpf_cgroup_link_fill_link_info() { > cgroup_lock(); > if (cg_link->cgroup) > cg_id = cgroup_id(cg_link->cgroup); > cgroup_unlock(); > } > > net/core/dev.c:bpf_xdp_link_fill_link_info() { > rtnl_lock(); > if (xdp_link->dev) > ifindex = xdp_link->dev->ifindex; > rtnl_unlock(); > } > > The same issue applies to bpf_perf_link_show_fdinfo() below. > > > @@ -4143,9 +4160,13 @@ static void bpf_perf_link_show_fdinfo(const struct bpf_link *link, > > { > > struct bpf_perf_link *perf_link; > > const struct perf_event *event; > > + struct file *perf_file; > > + > > perf_link = container_of(link, struct bpf_perf_link, link); > > - event = perf_get_event(perf_link->perf_file); > > + perf_file = READ_ONCE(perf_link->perf_file); > > + if (!perf_file) > > + return; > > + event = perf_get_event(perf_file); > > Same race as bpf_perf_link_fill_link_info() above -- a concurrent > BPF_LINK_DETACH can free perf_file between the READ_ONCE and the > perf_get_event() call. AI is correct, but is not proposing a fix, so I will try to be helpful. I think we can add per-link spin lock, so that operations on the same link are serialized. You can do the perf_file replacement in release under the lock, and call the rest of the stuff (perf_event_free.., fput) outside the lock. On the show_fdinfo and fill_link_info side, you will under the protection of the spin lock, read the perf_file and take a reference on it using get_file. Then you can release the lock and use it for the callback and fput on the way out. Other cases are unaffected since they are already under lock protection. I think a global lock is also an option, but I would lean toward having a 4-byte spin lock in the bpf_link itself. > > > --- > AI reviewed your patch. Please fix the bug or email reply why it's not a bug. > See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md > > CI run summary: https://github.com/kernel-patches/bpf/actions/runs/22690413273 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next v2 1/2] bpf: Add LINK_DETACH support for perf link 2026-03-06 3:38 ` Kumar Kartikeya Dwivedi @ 2026-03-06 3:59 ` Alexei Starovoitov 2026-03-11 16:48 ` Yonghong Song 0 siblings, 1 reply; 7+ messages in thread From: Alexei Starovoitov @ 2026-03-06 3:59 UTC (permalink / raw) To: Kumar Kartikeya Dwivedi Cc: bot+bpf-ci, Alexei Starovoitov, Florian Lehner, bpf, Daniel Borkmann, John Fastabend, Andrii Nakryiko, Martin KaFai Lau, Eduard, Song Liu, Yonghong Song, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Shuah Khan, David S. Miller, Jakub Kicinski, Jesper Dangaard Brouer, Martin KaFai Lau, Chris Mason, Ihor Solodrai On Thu, Mar 5, 2026 at 7:39 PM Kumar Kartikeya Dwivedi <memxor@gmail.com> wrote: > > On Wed, 4 Mar 2026 at 23:01, <bot+bpf-ci@kernel.org> wrote: > > > > > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c > > > --- a/kernel/bpf/syscall.c > > > +++ b/kernel/bpf/syscall.c > > > @@ -3863,10 +3863,23 @@ struct bpf_perf_link { > > > static void bpf_perf_link_release(struct bpf_link *link) > > > { > > > struct bpf_perf_link *perf_link = container_of(link, struct bpf_perf_link, link); > > > - struct perf_event *event = perf_link->perf_file->private_data; > > > + struct perf_event *event; > > > + struct file *perf_file; > > > + > > > + /* bpf_perf_link_detach() may have already released perf_file */ > > > + perf_file = xchg(&perf_link->perf_file, NULL); > > > + if (!perf_file) > > > + return; > > > + > > > + event = perf_file->private_data; > > > perf_event_free_bpf_prog(event); > > > - fput(perf_link->perf_file); > > > + fput(perf_file); > > > +} > > > > [ ... ] > > > > > @@ -4075,9 +4088,13 @@ static int bpf_perf_link_fill_link_info(const struct bpf_link *link, > > > { > > > struct bpf_perf_link *perf_link; > > > const struct perf_event *event; > > > + struct file *perf_file; > > > + > > > perf_link = container_of(link, struct bpf_perf_link, link); > > > - event = perf_get_event(perf_link->perf_file); > > > + perf_file = READ_ONCE(perf_link->perf_file); > > > + if (!perf_file) > > > + return 0; > > > + event = perf_get_event(perf_file); > > > > Could bpf_perf_link_fill_link_info() race with bpf_perf_link_release() > > here? > > > > After READ_ONCE reads a non-NULL perf_file, no reference is taken on > > the file. A concurrent BPF_LINK_DETACH on another CPU can call > > bpf_perf_link_release(), which does xchg() followed by fput(). If > > the perf event fd has already been closed by userspace, fput() drops > > the last reference. Once the detaching thread returns to userspace, > > task_work runs __fput and frees the file struct. > > > > At that point perf_get_event() dereferences file->f_op on freed > > memory: > > > > Thread A (BPF_OBJ_GET_INFO_BY_FD) Thread B (BPF_LINK_DETACH) > > -------------------------------- -------------------------- > > perf_file = READ_ONCE(...) [non-NULL] > > xchg(&..., NULL) > > fput(perf_file) > > return to userspace > > task_work -> __fput -> free > > perf_get_event(perf_file) -> UAF > > > > The xdp and cgroup link types handle this by serializing > > fill_link_info and release under a shared lock (rtnl_lock and > > cgroup_lock respectively): > > > > kernel/bpf/cgroup.c:bpf_cgroup_link_fill_link_info() { > > cgroup_lock(); > > if (cg_link->cgroup) > > cg_id = cgroup_id(cg_link->cgroup); > > cgroup_unlock(); > > } > > > > net/core/dev.c:bpf_xdp_link_fill_link_info() { > > rtnl_lock(); > > if (xdp_link->dev) > > ifindex = xdp_link->dev->ifindex; > > rtnl_unlock(); > > } > > > > The same issue applies to bpf_perf_link_show_fdinfo() below. > > > > > @@ -4143,9 +4160,13 @@ static void bpf_perf_link_show_fdinfo(const struct bpf_link *link, > > > { > > > struct bpf_perf_link *perf_link; > > > const struct perf_event *event; > > > + struct file *perf_file; > > > + > > > perf_link = container_of(link, struct bpf_perf_link, link); > > > - event = perf_get_event(perf_link->perf_file); > > > + perf_file = READ_ONCE(perf_link->perf_file); > > > + if (!perf_file) > > > + return; > > > + event = perf_get_event(perf_file); > > > > Same race as bpf_perf_link_fill_link_info() above -- a concurrent > > BPF_LINK_DETACH can free perf_file between the READ_ONCE and the > > perf_get_event() call. > > AI is correct, but is not proposing a fix, so I will try to be helpful. > I think we can add per-link spin lock, so that operations on the same > link are serialized. > You can do the perf_file replacement in release under the lock, and > call the rest of the stuff (perf_event_free.., fput) outside the lock. > On the show_fdinfo and fill_link_info side, you will under the > protection of the spin lock, read the perf_file and take a reference > on it using get_file. > Then you can release the lock and use it for the callback and fput on > the way out. > > Other cases are unaffected since they are already under lock > protection. I think a global lock is also an option, but I would lean > toward having a 4-byte spin lock in the bpf_link itself. I would just do a global mutex and avoid the complexity. fdinfo could be doing a bunch of work. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH bpf-next v2 1/2] bpf: Add LINK_DETACH support for perf link 2026-03-06 3:59 ` Alexei Starovoitov @ 2026-03-11 16:48 ` Yonghong Song 0 siblings, 0 replies; 7+ messages in thread From: Yonghong Song @ 2026-03-11 16:48 UTC (permalink / raw) To: Alexei Starovoitov, Kumar Kartikeya Dwivedi Cc: bot+bpf-ci, Alexei Starovoitov, Florian Lehner, bpf, Daniel Borkmann, John Fastabend, Andrii Nakryiko, Martin KaFai Lau, Eduard, Song Liu, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Shuah Khan, David S. Miller, Jakub Kicinski, Jesper Dangaard Brouer, Martin KaFai Lau, Chris Mason, Ihor Solodrai On 3/5/26 7:59 PM, Alexei Starovoitov wrote: > On Thu, Mar 5, 2026 at 7:39 PM Kumar Kartikeya Dwivedi <memxor@gmail.com> wrote: >> On Wed, 4 Mar 2026 at 23:01, <bot+bpf-ci@kernel.org> wrote: >>>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c >>>> --- a/kernel/bpf/syscall.c >>>> +++ b/kernel/bpf/syscall.c >>>> @@ -3863,10 +3863,23 @@ struct bpf_perf_link { >>>> static void bpf_perf_link_release(struct bpf_link *link) >>>> { >>>> struct bpf_perf_link *perf_link = container_of(link, struct bpf_perf_link, link); >>>> - struct perf_event *event = perf_link->perf_file->private_data; >>>> + struct perf_event *event; >>>> + struct file *perf_file; >>>> + >>>> + /* bpf_perf_link_detach() may have already released perf_file */ >>>> + perf_file = xchg(&perf_link->perf_file, NULL); >>>> + if (!perf_file) >>>> + return; >>>> + >>>> + event = perf_file->private_data; >>>> perf_event_free_bpf_prog(event); >>>> - fput(perf_link->perf_file); >>>> + fput(perf_file); >>>> +} >>> [ ... ] >>> >>>> @@ -4075,9 +4088,13 @@ static int bpf_perf_link_fill_link_info(const struct bpf_link *link, >>>> { >>>> struct bpf_perf_link *perf_link; >>>> const struct perf_event *event; >>>> + struct file *perf_file; >>>> + >>>> perf_link = container_of(link, struct bpf_perf_link, link); >>>> - event = perf_get_event(perf_link->perf_file); >>>> + perf_file = READ_ONCE(perf_link->perf_file); >>>> + if (!perf_file) >>>> + return 0; >>>> + event = perf_get_event(perf_file); >>> Could bpf_perf_link_fill_link_info() race with bpf_perf_link_release() >>> here? >>> >>> After READ_ONCE reads a non-NULL perf_file, no reference is taken on >>> the file. A concurrent BPF_LINK_DETACH on another CPU can call >>> bpf_perf_link_release(), which does xchg() followed by fput(). If >>> the perf event fd has already been closed by userspace, fput() drops >>> the last reference. Once the detaching thread returns to userspace, >>> task_work runs __fput and frees the file struct. >>> >>> At that point perf_get_event() dereferences file->f_op on freed >>> memory: >>> >>> Thread A (BPF_OBJ_GET_INFO_BY_FD) Thread B (BPF_LINK_DETACH) >>> -------------------------------- -------------------------- >>> perf_file = READ_ONCE(...) [non-NULL] >>> xchg(&..., NULL) >>> fput(perf_file) >>> return to userspace >>> task_work -> __fput -> free >>> perf_get_event(perf_file) -> UAF >>> >>> The xdp and cgroup link types handle this by serializing >>> fill_link_info and release under a shared lock (rtnl_lock and >>> cgroup_lock respectively): >>> >>> kernel/bpf/cgroup.c:bpf_cgroup_link_fill_link_info() { >>> cgroup_lock(); >>> if (cg_link->cgroup) >>> cg_id = cgroup_id(cg_link->cgroup); >>> cgroup_unlock(); >>> } >>> >>> net/core/dev.c:bpf_xdp_link_fill_link_info() { >>> rtnl_lock(); >>> if (xdp_link->dev) >>> ifindex = xdp_link->dev->ifindex; >>> rtnl_unlock(); >>> } >>> >>> The same issue applies to bpf_perf_link_show_fdinfo() below. >>> >>>> @@ -4143,9 +4160,13 @@ static void bpf_perf_link_show_fdinfo(const struct bpf_link *link, >>>> { >>>> struct bpf_perf_link *perf_link; >>>> const struct perf_event *event; >>>> + struct file *perf_file; >>>> + >>>> perf_link = container_of(link, struct bpf_perf_link, link); >>>> - event = perf_get_event(perf_link->perf_file); >>>> + perf_file = READ_ONCE(perf_link->perf_file); >>>> + if (!perf_file) >>>> + return; >>>> + event = perf_get_event(perf_file); >>> Same race as bpf_perf_link_fill_link_info() above -- a concurrent >>> BPF_LINK_DETACH can free perf_file between the READ_ONCE and the >>> perf_get_event() call. >> AI is correct, but is not proposing a fix, so I will try to be helpful. >> I think we can add per-link spin lock, so that operations on the same >> link are serialized. >> You can do the perf_file replacement in release under the lock, and >> call the rest of the stuff (perf_event_free.., fput) outside the lock. >> On the show_fdinfo and fill_link_info side, you will under the >> protection of the spin lock, read the perf_file and take a reference >> on it using get_file. >> Then you can release the lock and use it for the callback and fput on >> the way out. >> >> Other cases are unaffected since they are already under lock >> protection. I think a global lock is also an option, but I would lean >> toward having a 4-byte spin lock in the bpf_link itself. > I would just do a global mutex and avoid the complexity. > fdinfo could be doing a bunch of work. The following is an example using mutex lock: https://lore.kernel.org/r/20240410043527.3737160-1-yonghong.song@linux.dev ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH bpf-next v2 2/2] selftests/bpf: Test LINK_DETACH for perf link 2026-03-04 21:02 [PATCH bpf-next v2 0/2] bpf: Add LINK_DETACH for perf links Florian Lehner 2026-03-04 21:02 ` [PATCH bpf-next v2 1/2] bpf: Add LINK_DETACH support for perf link Florian Lehner @ 2026-03-04 21:02 ` Florian Lehner 1 sibling, 0 replies; 7+ messages in thread From: Florian Lehner @ 2026-03-04 21:02 UTC (permalink / raw) To: bpf Cc: ast, daniel, john.fastabend, andrii, martin.lau, eddyz87, song, yonghong.song, kpsingh, sdf, haoluo, jolsa, shuah, davem, kuba, hawk, Florian Lehner Add serial_test_perf_link_detach() to verify that the new LINK_DETACH support for BPF perf links works correctly. The test creates a link to a BPF program for a software perf event, confirms the program is executed, calls bpf_link__detach() to exercise the BPF_LINK_DETACH syscall path, and then verifies the program is no longer invoked after detach. Signed-off-by: Florian Lehner <dev@der-flo.net> --- .../selftests/bpf/prog_tests/perf_link.c | 81 ++++++++++++++++--- 1 file changed, 68 insertions(+), 13 deletions(-) diff --git a/tools/testing/selftests/bpf/prog_tests/perf_link.c b/tools/testing/selftests/bpf/prog_tests/perf_link.c index d940ff87fa08..40e2ad0f843e 100644 --- a/tools/testing/selftests/bpf/prog_tests/perf_link.c +++ b/tools/testing/selftests/bpf/prog_tests/perf_link.c @@ -27,30 +27,85 @@ static void burn_cpu(void) ++j; } -/* TODO: often fails in concurrent mode */ -void serial_test_perf_link(void) +static int perf_link_setup(struct test_perf_link **skel, int *pfd) { - struct test_perf_link *skel = NULL; struct perf_event_attr attr; - int pfd = -1, link_fd = -1, err; - int run_cnt_before, run_cnt_after; - struct bpf_link_info info; - __u32 info_len = sizeof(info); - __u64 timeout_time_ns; - /* create perf event */ memset(&attr, 0, sizeof(attr)); attr.size = sizeof(attr); attr.type = PERF_TYPE_SOFTWARE; attr.config = PERF_COUNT_SW_CPU_CLOCK; attr.freq = 1; attr.sample_freq = 1000; - pfd = syscall(__NR_perf_event_open, &attr, -1, 0, -1, PERF_FLAG_FD_CLOEXEC); - if (!ASSERT_GE(pfd, 0, "perf_fd")) + *pfd = syscall(__NR_perf_event_open, &attr, -1, 0, -1, PERF_FLAG_FD_CLOEXEC); + if (!ASSERT_GE(*pfd, 0, "perf_fd")) + return -1; + + *skel = test_perf_link__open_and_load(); + if (!ASSERT_OK_PTR(*skel, "skel_load")) + return -1; + + return 0; +} + +void serial_test_perf_link_detach(void) +{ + struct test_perf_link *skel = NULL; + int pfd = -1, link_fd = -1, err; + int run_cnt_before, run_cnt_after; + __u64 timeout_time_ns; + + if (perf_link_setup(&skel, &pfd)) + goto cleanup; + + link_fd = bpf_link_create(bpf_program__fd(skel->progs.handler), pfd, + BPF_PERF_EVENT, NULL); + if (!ASSERT_GE(link_fd, 0, "link_fd")) goto cleanup; - skel = test_perf_link__open_and_load(); - if (!ASSERT_OK_PTR(skel, "skel_load")) + /* ensure we get at least one perf_event prog execution */ + timeout_time_ns = get_time_ns() + BURN_TIMEOUT_NS; + while (true) { + burn_cpu(); + if (skel->bss->run_cnt > 0) + break; + if (!ASSERT_LT(get_time_ns(), timeout_time_ns, "run_cnt_timeout")) + goto cleanup; + } + + /* detach via BPF_LINK_DETACH - BPF program should no longer be executed */ + err = bpf_link_detach(link_fd); + if (!ASSERT_OK(err, "link_detach")) + goto cleanup; + + /* make sure there are no stragglers */ + kern_sync_rcu(); + + run_cnt_before = skel->bss->run_cnt; + burn_cpu(); + run_cnt_after = skel->bss->run_cnt; + + ASSERT_EQ(run_cnt_before, run_cnt_after, "run_cnt_detached"); + +cleanup: + if (link_fd >= 0) + close(link_fd); + if (pfd >= 0) + close(pfd); + test_perf_link__destroy(skel); +} + +/* TODO: often fails in concurrent mode */ +void serial_test_perf_link(void) +{ + struct test_perf_link *skel = NULL; + int pfd = -1, link_fd = -1, err; + int run_cnt_before, run_cnt_after; + struct bpf_link_info info; + __u32 info_len = sizeof(info); + __u64 timeout_time_ns; + + if (perf_link_setup(&skel, &pfd)) goto cleanup; link_fd = bpf_link_create(bpf_program__fd(skel->progs.handler), pfd, -- 2.53.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-03-11 16:48 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-03-04 21:02 [PATCH bpf-next v2 0/2] bpf: Add LINK_DETACH for perf links Florian Lehner 2026-03-04 21:02 ` [PATCH bpf-next v2 1/2] bpf: Add LINK_DETACH support for perf link Florian Lehner 2026-03-04 22:01 ` bot+bpf-ci 2026-03-06 3:38 ` Kumar Kartikeya Dwivedi 2026-03-06 3:59 ` Alexei Starovoitov 2026-03-11 16:48 ` Yonghong Song 2026-03-04 21:02 ` [PATCH bpf-next v2 2/2] selftests/bpf: Test LINK_DETACH " Florian Lehner
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.