All of lore.kernel.org
 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 36/39] assorted variants of irqfd setup: convert to CLASS(fd)
Date: Tue, 30 Jul 2024 01:16:22 -0400	[thread overview]
Message-ID: <20240730051625.14349-36-viro@kernel.org> (raw)
In-Reply-To: <20240730051625.14349-1-viro@kernel.org>

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

in all of those failure exits prior to fdget() are plain returns and
the only thing done after fdput() is (on failure exits) a kfree(),
which can be done before fdput() just fine.

NOTE: in acrn_irqfd_assign() 'fail:' failure exit is wrong for
eventfd_ctx_fileget() failure (we only want fdput() there) and once
we stop doing that, it doesn't need to check if eventfd is NULL or
ERR_PTR(...) there.

NOTE: in privcmd we move fdget() up before the allocation - more
to the point, before the copy_from_user() attempt.

[trivial conflict in privcmd]

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 drivers/vfio/virqfd.c     | 16 +++-------------
 drivers/virt/acrn/irqfd.c | 13 ++++---------
 drivers/xen/privcmd.c     | 17 ++++-------------
 virt/kvm/eventfd.c        | 15 +++------------
 4 files changed, 14 insertions(+), 47 deletions(-)

diff --git a/drivers/vfio/virqfd.c b/drivers/vfio/virqfd.c
index d22881245e89..aa2891f97508 100644
--- a/drivers/vfio/virqfd.c
+++ b/drivers/vfio/virqfd.c
@@ -113,7 +113,6 @@ int vfio_virqfd_enable(void *opaque,
 		       void (*thread)(void *, void *),
 		       void *data, struct virqfd **pvirqfd, int fd)
 {
-	struct fd irqfd;
 	struct eventfd_ctx *ctx;
 	struct virqfd *virqfd;
 	int ret = 0;
@@ -133,8 +132,8 @@ int vfio_virqfd_enable(void *opaque,
 	INIT_WORK(&virqfd->inject, virqfd_inject);
 	INIT_WORK(&virqfd->flush_inject, virqfd_flush_inject);
 
-	irqfd = fdget(fd);
-	if (!fd_file(irqfd)) {
+	CLASS(fd, irqfd)(fd);
+	if (fd_empty(irqfd)) {
 		ret = -EBADF;
 		goto err_fd;
 	}
@@ -142,7 +141,7 @@ int vfio_virqfd_enable(void *opaque,
 	ctx = eventfd_ctx_fileget(fd_file(irqfd));
 	if (IS_ERR(ctx)) {
 		ret = PTR_ERR(ctx);
-		goto err_ctx;
+		goto err_fd;
 	}
 
 	virqfd->eventfd = ctx;
@@ -181,18 +180,9 @@ int vfio_virqfd_enable(void *opaque,
 		if ((!handler || handler(opaque, data)) && thread)
 			schedule_work(&virqfd->inject);
 	}
-
-	/*
-	 * Do not drop the file until the irqfd is fully initialized,
-	 * otherwise we might race against the EPOLLHUP.
-	 */
-	fdput(irqfd);
-
 	return 0;
 err_busy:
 	eventfd_ctx_put(ctx);
-err_ctx:
-	fdput(irqfd);
 err_fd:
 	kfree(virqfd);
 
diff --git a/drivers/virt/acrn/irqfd.c b/drivers/virt/acrn/irqfd.c
index 9994d818bb7e..b7da24ca1475 100644
--- a/drivers/virt/acrn/irqfd.c
+++ b/drivers/virt/acrn/irqfd.c
@@ -112,7 +112,6 @@ static int acrn_irqfd_assign(struct acrn_vm *vm, struct acrn_irqfd *args)
 	struct eventfd_ctx *eventfd = NULL;
 	struct hsm_irqfd *irqfd, *tmp;
 	__poll_t events;
-	struct fd f;
 	int ret = 0;
 
 	irqfd = kzalloc(sizeof(*irqfd), GFP_KERNEL);
@@ -124,8 +123,8 @@ static int acrn_irqfd_assign(struct acrn_vm *vm, struct acrn_irqfd *args)
 	INIT_LIST_HEAD(&irqfd->list);
 	INIT_WORK(&irqfd->shutdown, hsm_irqfd_shutdown_work);
 
-	f = fdget(args->fd);
-	if (!fd_file(f)) {
+	CLASS(fd, f)(args->fd);
+	if (fd_empty(f)) {
 		ret = -EBADF;
 		goto out;
 	}
@@ -133,7 +132,7 @@ static int acrn_irqfd_assign(struct acrn_vm *vm, struct acrn_irqfd *args)
 	eventfd = eventfd_ctx_fileget(fd_file(f));
 	if (IS_ERR(eventfd)) {
 		ret = PTR_ERR(eventfd);
-		goto fail;
+		goto out;
 	}
 
 	irqfd->eventfd = eventfd;
@@ -162,13 +161,9 @@ static int acrn_irqfd_assign(struct acrn_vm *vm, struct acrn_irqfd *args)
 	if (events & EPOLLIN)
 		acrn_irqfd_inject(irqfd);
 
-	fdput(f);
 	return 0;
 fail:
-	if (eventfd && !IS_ERR(eventfd))
-		eventfd_ctx_put(eventfd);
-
-	fdput(f);
+	eventfd_ctx_put(eventfd);
 out:
 	kfree(irqfd);
 	return ret;
diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c
index ba02b732fa49..8a5bdf1f3050 100644
--- a/drivers/xen/privcmd.c
+++ b/drivers/xen/privcmd.c
@@ -939,10 +939,11 @@ static int privcmd_irqfd_assign(struct privcmd_irqfd *irqfd)
 	struct privcmd_kernel_irqfd *kirqfd, *tmp;
 	unsigned long flags;
 	__poll_t events;
-	struct fd f;
 	void *dm_op;
 	int ret, idx;
 
+	CLASS(fd, f)(irqfd->fd);
+
 	kirqfd = kzalloc(sizeof(*kirqfd) + irqfd->size, GFP_KERNEL);
 	if (!kirqfd)
 		return -ENOMEM;
@@ -958,8 +959,7 @@ static int privcmd_irqfd_assign(struct privcmd_irqfd *irqfd)
 	kirqfd->dom = irqfd->dom;
 	INIT_WORK(&kirqfd->shutdown, irqfd_shutdown);
 
-	f = fdget(irqfd->fd);
-	if (!fd_file(f)) {
+	if (fd_empty(f)) {
 		ret = -EBADF;
 		goto error_kfree;
 	}
@@ -967,7 +967,7 @@ static int privcmd_irqfd_assign(struct privcmd_irqfd *irqfd)
 	kirqfd->eventfd = eventfd_ctx_fileget(fd_file(f));
 	if (IS_ERR(kirqfd->eventfd)) {
 		ret = PTR_ERR(kirqfd->eventfd);
-		goto error_fd_put;
+		goto error_kfree;
 	}
 
 	/*
@@ -1000,20 +1000,11 @@ static int privcmd_irqfd_assign(struct privcmd_irqfd *irqfd)
 		irqfd_inject(kirqfd);
 
 	srcu_read_unlock(&irqfds_srcu, idx);
-
-	/*
-	 * Do not drop the file until the kirqfd is fully initialized, otherwise
-	 * we might race against the EPOLLHUP.
-	 */
-	fdput(f);
 	return 0;
 
 error_eventfd:
 	eventfd_ctx_put(kirqfd->eventfd);
 
-error_fd_put:
-	fdput(f);
-
 error_kfree:
 	kfree(kirqfd);
 	return ret;
diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c
index 65efb3735e79..70bc0d1f5f6a 100644
--- a/virt/kvm/eventfd.c
+++ b/virt/kvm/eventfd.c
@@ -303,7 +303,6 @@ static int
 kvm_irqfd_assign(struct kvm *kvm, struct kvm_irqfd *args)
 {
 	struct kvm_kernel_irqfd *irqfd, *tmp;
-	struct fd f;
 	struct eventfd_ctx *eventfd = NULL, *resamplefd = NULL;
 	int ret;
 	__poll_t events;
@@ -326,8 +325,8 @@ kvm_irqfd_assign(struct kvm *kvm, struct kvm_irqfd *args)
 	INIT_WORK(&irqfd->shutdown, irqfd_shutdown);
 	seqcount_spinlock_init(&irqfd->irq_entry_sc, &kvm->irqfds.lock);
 
-	f = fdget(args->fd);
-	if (!fd_file(f)) {
+	CLASS(fd, f)(args->fd);
+	if (fd_empty(f)) {
 		ret = -EBADF;
 		goto out;
 	}
@@ -335,7 +334,7 @@ kvm_irqfd_assign(struct kvm *kvm, struct kvm_irqfd *args)
 	eventfd = eventfd_ctx_fileget(fd_file(f));
 	if (IS_ERR(eventfd)) {
 		ret = PTR_ERR(eventfd);
-		goto fail;
+		goto out;
 	}
 
 	irqfd->eventfd = eventfd;
@@ -439,12 +438,6 @@ kvm_irqfd_assign(struct kvm *kvm, struct kvm_irqfd *args)
 #endif
 
 	srcu_read_unlock(&kvm->irq_srcu, idx);
-
-	/*
-	 * do not drop the file until the irqfd is fully initialized, otherwise
-	 * we might race against the EPOLLHUP
-	 */
-	fdput(f);
 	return 0;
 
 fail:
@@ -457,8 +450,6 @@ kvm_irqfd_assign(struct kvm *kvm, struct kvm_irqfd *args)
 	if (eventfd && !IS_ERR(eventfd))
 		eventfd_ctx_put(eventfd);
 
-	fdput(f);
-
 out:
 	kfree(irqfd);
 	return ret;
-- 
2.39.2


  parent reply	other threads:[~2024-07-30  5:15 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   ` [PATCH 10/39] get rid of perf_fget_light(), convert kernel/events/core.c to CLASS(fd) viro
2024-08-07 10:25     ` 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   ` viro [this message]
2024-08-07 10:46     ` [PATCH 36/39] assorted variants of irqfd setup: convert to CLASS(fd) 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-36-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 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.