linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: viro@kernel.org
To: linux-fsdevel@vger.kernel.org
Cc: amir73il@gmail.com, bpf@vger.kernel.org, brauner@kernel.org,
	cgroups@vger.kernel.org, kvm@vger.kernel.org,
	netdev@vger.kernel.org, torvalds@linux-foundation.org
Subject: [PATCH 10/39] get rid of perf_fget_light(), convert kernel/events/core.c to CLASS(fd)
Date: Tue, 30 Jul 2024 01:15:56 -0400	[thread overview]
Message-ID: <20240730051625.14349-10-viro@kernel.org> (raw)
In-Reply-To: <20240730051625.14349-1-viro@kernel.org>

From: Al Viro <viro@zeniv.linux.org.uk>

Lift fdget() and fdput() out of perf_fget_light(), turning it into
is_perf_file(struct fd f).  The life gets easier in both callers
if we do fdget() unconditionally, including the case when we are
given -1 instead of a descriptor - that avoids a reassignment in
perf_event_open(2) and it avoids a nasty temptation in _perf_ioctl()
where we must *not* lift output_event out of scope for output.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 kernel/events/core.c | 49 +++++++++++++++-----------------------------
 1 file changed, 16 insertions(+), 33 deletions(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index fd2ac9c7fd77..dae815c30514 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -5895,18 +5895,9 @@ EXPORT_SYMBOL_GPL(perf_event_period);
 
 static const struct file_operations perf_fops;
 
-static inline int perf_fget_light(int fd, struct fd *p)
+static inline bool is_perf_file(struct fd f)
 {
-	struct fd f = fdget(fd);
-	if (!fd_file(f))
-		return -EBADF;
-
-	if (fd_file(f)->f_op != &perf_fops) {
-		fdput(f);
-		return -EBADF;
-	}
-	*p = f;
-	return 0;
+	return !fd_empty(f) && fd_file(f)->f_op == &perf_fops;
 }
 
 static int perf_event_set_output(struct perf_event *event,
@@ -5954,20 +5945,14 @@ static long _perf_ioctl(struct perf_event *event, unsigned int cmd, unsigned lon
 
 	case PERF_EVENT_IOC_SET_OUTPUT:
 	{
-		int ret;
+		CLASS(fd, output)(arg);	     // arg == -1 => empty
+		struct perf_event *output_event = NULL;
 		if (arg != -1) {
-			struct perf_event *output_event;
-			struct fd output;
-			ret = perf_fget_light(arg, &output);
-			if (ret)
-				return ret;
+			if (!is_perf_file(output))
+				return -EBADF;
 			output_event = fd_file(output)->private_data;
-			ret = perf_event_set_output(event, output_event);
-			fdput(output);
-		} else {
-			ret = perf_event_set_output(event, NULL);
 		}
-		return ret;
+		return perf_event_set_output(event, output_event);
 	}
 
 	case PERF_EVENT_IOC_SET_FILTER:
@@ -12474,7 +12459,6 @@ SYSCALL_DEFINE5(perf_event_open,
 	struct perf_event_attr attr;
 	struct perf_event_context *ctx;
 	struct file *event_file = NULL;
-	struct fd group = EMPTY_FD;
 	struct task_struct *task = NULL;
 	struct pmu *pmu;
 	int event_fd;
@@ -12545,10 +12529,12 @@ SYSCALL_DEFINE5(perf_event_open,
 	if (event_fd < 0)
 		return event_fd;
 
+	CLASS(fd, group)(group_fd);     // group_fd == -1 => empty
 	if (group_fd != -1) {
-		err = perf_fget_light(group_fd, &group);
-		if (err)
+		if (!is_perf_file(group)) {
+			err = -EBADF;
 			goto err_fd;
+		}
 		group_leader = fd_file(group)->private_data;
 		if (flags & PERF_FLAG_FD_OUTPUT)
 			output_event = group_leader;
@@ -12560,7 +12546,7 @@ SYSCALL_DEFINE5(perf_event_open,
 		task = find_lively_task_by_vpid(pid);
 		if (IS_ERR(task)) {
 			err = PTR_ERR(task);
-			goto err_group_fd;
+			goto err_fd;
 		}
 	}
 
@@ -12827,12 +12813,11 @@ SYSCALL_DEFINE5(perf_event_open,
 	mutex_unlock(&current->perf_event_mutex);
 
 	/*
-	 * Drop the reference on the group_event after placing the
-	 * new event on the sibling_list. This ensures destruction
-	 * of the group leader will find the pointer to itself in
-	 * perf_group_detach().
+	 * File reference in group guarantees that group_leader has been
+	 * kept alive until we place the new event on the sibling_list.
+	 * This ensures destruction of the group leader will find
+	 * the pointer to itself in perf_group_detach().
 	 */
-	fdput(group);
 	fd_install(event_fd, event_file);
 	return event_fd;
 
@@ -12851,8 +12836,6 @@ SYSCALL_DEFINE5(perf_event_open,
 err_task:
 	if (task)
 		put_task_struct(task);
-err_group_fd:
-	fdput(group);
 err_fd:
 	put_unused_fd(event_fd);
 	return err;
-- 
2.39.2


  parent reply	other threads:[~2024-07-30  5:14 UTC|newest]

Thread overview: 134+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-07-30  5:09 [PATCHSET][RFC] struct fd and memory safety Al Viro
2024-07-30  5:15 ` [PATCH 01/39] memcg_write_event_control(): fix a user-triggerable oops viro
2024-07-30  5:15   ` [PATCH 02/39] introduce fd_file(), convert all accessors to it viro
2024-08-07  9:55     ` Christian Brauner
2024-07-30  5:15   ` [PATCH 03/39] struct fd: representation change viro
2024-07-30 18:10     ` Josef Bacik
2024-08-07 10:07       ` Christian Brauner
2024-08-07 10:03     ` Christian Brauner
2024-07-30  5:15   ` [PATCH 04/39] add struct fd constructors, get rid of __to_fd() viro
2024-08-07 10:09     ` Christian Brauner
2024-07-30  5:15   ` [PATCH 05/39] regularize emptiness checks in fini_module(2) and vfs_dedupe_file_range() viro
2024-08-07 10:10     ` Christian Brauner
2024-07-30  5:15   ` [PATCH 06/39] net/socket.c: switch to CLASS(fd) viro
2024-08-07 10:13     ` Christian Brauner
2024-07-30  5:15   ` [PATCH 07/39] introduce struct fderr, convert overlayfs uses to that viro
2024-07-30  5:15   ` [PATCH 08/39] experimental: convert fs/overlayfs/file.c to CLASS(...) viro
2024-07-30 19:10     ` Josef Bacik
2024-07-30 21:12       ` Al Viro
2024-07-31 21:11         ` Josef Bacik
2024-08-07 10:23     ` Christian Brauner
2024-07-30  5:15   ` [PATCH 09/39] timerfd: switch to CLASS(fd, ...) viro
2024-08-07 10:24     ` Christian Brauner
2024-07-30  5:15   ` viro [this message]
2024-08-07 10:25     ` [PATCH 10/39] get rid of perf_fget_light(), convert kernel/events/core.c to CLASS(fd) Christian Brauner
2024-07-30  5:15   ` [PATCH 11/39] switch netlink_getsockbyfilp() to taking descriptor viro
2024-08-07 10:26     ` Christian Brauner
2024-07-30  5:15   ` [PATCH 12/39] do_mq_notify(): saner skb freeing on failures viro
2024-07-30  5:15   ` [PATCH 13/39] do_mq_notify(): switch to CLASS(fd, ...) viro
2024-08-07 10:27     ` Christian Brauner
2024-07-30  5:16   ` [PATCH 14/39] simplify xfs_find_handle() a bit viro
2024-07-30  5:16   ` [PATCH 15/39] convert vmsplice() to CLASS(fd, ...) viro
2024-08-07 10:27     ` Christian Brauner
2024-07-30  5:16   ` [PATCH 16/39] convert __bpf_prog_get() " viro
2024-08-06 21:08     ` Andrii Nakryiko
2024-08-07 10:28     ` Christian Brauner
2024-07-30  5:16   ` [PATCH 17/39] bpf: resolve_pseudo_ldimm64(): take handling of a single ldimm64 insn into helper viro
2024-08-06 22:32     ` Andrii Nakryiko
2024-08-07 10:29       ` Christian Brauner
2024-08-07 15:30         ` Andrii Nakryiko
2024-08-08 16:51           ` Alexei Starovoitov
2024-08-08 20:35             ` Andrii Nakryiko
2024-08-09  1:23               ` Alexei Starovoitov
2024-08-09 17:23                 ` Andrii Nakryiko
2024-08-10  3:29             ` Al Viro
2024-08-12 20:05               ` Andrii Nakryiko
2024-08-13  2:06                 ` Al Viro
2024-08-13  3:32                   ` Andrii Nakryiko
2024-07-30  5:16   ` [PATCH 18/39] bpf maps: switch to CLASS(fd, ...) viro
2024-08-07 10:34     ` Christian Brauner
2024-07-30  5:16   ` [PATCH 19/39] fdget_raw() users: switch to CLASS(fd_raw, ...) viro
2024-08-07 10:35     ` Christian Brauner
2024-07-30  5:16   ` [PATCH 20/39] introduce "fd_pos" class, convert fdget_pos() users to it viro
2024-08-07 10:36     ` Christian Brauner
2024-07-30  5:16   ` [PATCH 21/39] o2hb_region_dev_store(): avoid goto around fdget()/fdput() viro
2024-07-30  5:16   ` [PATCH 22/39] privcmd_ioeventfd_assign(): don't open-code eventfd_ctx_fdget() viro
2024-07-30  5:16   ` [PATCH 23/39] fdget(), trivial conversions viro
2024-08-07 10:37     ` Christian Brauner
2024-07-30  5:16   ` [PATCH 24/39] fdget(), more " viro
2024-08-07 10:39     ` Christian Brauner
2024-07-30  5:16   ` [PATCH 25/39] convert do_preadv()/do_pwritev() viro
2024-08-07 10:39     ` Christian Brauner
2024-07-30  5:16   ` [PATCH 26/39] convert cachestat(2) viro
2024-08-07 10:39     ` Christian Brauner
2024-07-30  5:16   ` [PATCH 27/39] switch spufs_calls_{get,put}() to CLASS() use viro
2024-07-30  5:16   ` [PATCH 28/39] convert spu_run(2) viro
2024-08-07 10:40     ` Christian Brauner
2024-07-30  5:16   ` [PATCH 29/39] convert media_request_get_by_fd() viro
2024-08-07 10:40     ` Christian Brauner
2024-07-30  5:16   ` [PATCH 30/39] convert coda_parse_fd() viro
2024-08-07 10:41     ` Christian Brauner
2024-07-30  5:16   ` [PATCH 31/39] convert cifs_ioctl_copychunk() viro
2024-08-07 10:41     ` Christian Brauner
2024-07-30  5:16   ` [PATCH 32/39] convert vfs_dedupe_file_range() viro
2024-08-07 10:42     ` Christian Brauner
2024-07-30  5:16   ` [PATCH 33/39] convert do_select() viro
2024-08-07 10:42     ` Christian Brauner
2024-07-30  5:16   ` [PATCH 34/39] do_pollfd(): convert to CLASS(fd) viro
2024-08-07 10:43     ` Christian Brauner
2024-07-30  5:16   ` [PATCH 35/39] convert bpf_token_create() viro
2024-08-06 22:42     ` Andrii Nakryiko
2024-08-10  3:46       ` Al Viro
2024-08-12 20:06         ` Andrii Nakryiko
2024-08-07 10:44     ` Christian Brauner
2024-07-30  5:16   ` [PATCH 36/39] assorted variants of irqfd setup: convert to CLASS(fd) viro
2024-08-07 10:46     ` Christian Brauner
2024-08-10  3:53       ` Al Viro
2024-07-30  5:16   ` [PATCH 37/39] memcg_write_event_control(): switch " viro
2024-08-07 10:47     ` Christian Brauner
2024-07-30  5:16   ` [PATCH 38/39] css_set_fork(): switch to CLASS(fd_raw, ...) viro
2024-08-07 10:47     ` Christian Brauner
2024-07-30  5:16   ` [PATCH 39/39] deal with the last remaing boolean uses of fd_file() viro
2024-08-07 10:48     ` Christian Brauner
2024-07-30  7:13   ` [PATCH 01/39] memcg_write_event_control(): fix a user-triggerable oops Michal Hocko
2024-07-30  7:18     ` Al Viro
2024-07-30  7:37       ` Michal Hocko
2024-07-30  5:17 ` [PATCHSET][RFC] struct fd and memory safety Al Viro
2024-07-30 20:02 ` Josef Bacik
2024-07-31  0:43 ` Al Viro
2024-08-06 17:58 ` Jason Gunthorpe
2024-08-06 18:56   ` Al Viro
2024-08-07 10:51 ` Christian Brauner
2024-11-02  5:02 ` [PATCHSET v3] " Al Viro
2024-11-02  5:07   ` [PATCH v3 01/28] net/socket.c: switch to CLASS(fd) Al Viro
2024-11-02  5:08     ` [PATCH v3 02/28] regularize emptiness checks in fini_module(2) and vfs_dedupe_file_range() Al Viro
2024-11-02  5:08     ` [PATCH v3 03/28] timerfd: switch to CLASS(fd) Al Viro
2024-11-02  5:08     ` [PATCH v3 04/28] get rid of perf_fget_light(), convert kernel/events/core.c " Al Viro
2024-11-02  5:08     ` [PATCH v3 05/28] switch netlink_getsockbyfilp() to taking descriptor Al Viro
2024-11-02  5:08     ` [PATCH v3 06/28] do_mq_notify(): saner skb freeing on failures Al Viro
2024-11-02  5:08     ` [PATCH v3 07/28] do_mq_notify(): switch to CLASS(fd) Al Viro
2024-11-02  5:08     ` [PATCH v3 08/28] simplify xfs_find_handle() a bit Al Viro
2024-11-02  5:08     ` [PATCH v3 09/28] convert vmsplice() to CLASS(fd) Al Viro
2024-11-02  5:08     ` [PATCH v3 10/28] fdget_raw() users: switch to CLASS(fd_raw) Al Viro
2024-11-02  5:08     ` [PATCH v3 11/28] introduce "fd_pos" class, convert fdget_pos() users to it Al Viro
2024-11-02  5:08     ` [PATCH v3 12/28] o2hb_region_dev_store(): avoid goto around fdget()/fdput() Al Viro
2024-11-02  5:08     ` [PATCH v3 13/28] privcmd_ioeventfd_assign(): don't open-code eventfd_ctx_fdget() Al Viro
2024-11-02  5:08     ` [PATCH v3 14/28] fdget(), trivial conversions Al Viro
2024-11-11 17:22       ` Francesco Lavra
2024-11-02  5:08     ` [PATCH v3 15/28] fdget(), more " Al Viro
2024-11-02  5:08     ` [PATCH v3 16/28] convert do_preadv()/do_pwritev() Al Viro
2024-11-02  5:08     ` [PATCH v3 17/28] convert cachestat(2) Al Viro
2024-11-02  5:08     ` [PATCH v3 18/28] switch spufs_calls_{get,put}() to CLASS() use Al Viro
2024-11-02  5:08     ` [PATCH v3 19/28] convert spu_run(2) Al Viro
2024-11-02  5:08     ` [PATCH v3 20/28] convert media_request_get_by_fd() Al Viro
2024-11-02  5:08     ` [PATCH v3 21/28] convert cifs_ioctl_copychunk() Al Viro
2024-11-02  5:08     ` [PATCH v3 22/28] convert vfs_dedupe_file_range() Al Viro
2024-11-02  5:08     ` [PATCH v3 23/28] convert do_select() Al Viro
2024-11-02  5:08     ` [PATCH v3 24/28] do_pollfd(): convert to CLASS(fd) Al Viro
2024-11-02  5:08     ` [PATCH v3 25/28] assorted variants of irqfd setup: " Al Viro
2024-11-02  5:08     ` [PATCH v3 26/28] memcg_write_event_control(): switch " Al Viro
2024-11-02  5:08     ` [PATCH v3 27/28] css_set_fork(): switch to CLASS(fd_raw, ...) Al Viro
2024-11-02  5:08     ` [PATCH v3 28/28] deal with the last remaing boolean uses of fd_file() Al Viro
2024-11-02 12:21     ` [PATCH v3 01/28] net/socket.c: switch to CLASS(fd) Simon Horman
2024-11-03  6:31       ` Al Viro
2024-11-06 10:03         ` Simon Horman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240730051625.14349-10-viro@kernel.org \
    --to=viro@kernel.org \
    --cc=amir73il@gmail.com \
    --cc=bpf@vger.kernel.org \
    --cc=brauner@kernel.org \
    --cc=cgroups@vger.kernel.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).