[PATCH bpf-next v2 0/2] bpf: Add LINK_DETACH for perf links

[PATCH bpf-next v2 0/2] bpf: Add LINK_DETACH for perf links

[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

[PATCH bpf-next v2 1/2] bpf: Add LINK_DETACH support for perf link

[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

[PATCH bpf-next v2 2/2] selftests/bpf: Test LINK_DETACH for perf link

[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

Re: [PATCH bpf-next v2 1/2] bpf: Add LINK_DETACH support for perf link

[bot+bpf-ci]

[-- 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

Re: [PATCH bpf-next v2 1/2] bpf: Add LINK_DETACH support for perf link

[Kumar Kartikeya Dwivedi]

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

Re: [PATCH bpf-next v2 1/2] bpf: Add LINK_DETACH support for perf link

[Alexei Starovoitov]

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.

Re: [PATCH bpf-next v2 1/2] bpf: Add LINK_DETACH support for perf link

[Yonghong Song]

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