[PATCH] io_uring/zcrx: reorder fd allocation and disclosure in zcrx_export()

[PATCH] io_uring/zcrx: reorder fd allocation and disclosure in zcrx_export()

[Bertie Tryner]

Currently, zcrx_export() allocates and discloses a file descriptor to
userspace before the backing file is successfully created. If file
creation fails, the fd is released back to the pool, but the number
has already been written to the user-provided control structure.

While this requires a misbehaving or racing userspace to trigger,
it is better practice to ensure the file descriptor is only
disclosed once the operation is guaranteed to succeed. This aligns
the ZCRX export logic with the standard patterns used in the VFS
layer and other fd-publishing paths.

Move the get_unused_fd_flags() and copy_to_user() calls to after
the file has been successfully created.

Signed-off-by: Bertie Tryner <Bertie.Tryner@warwick.ac.uk>
---
 io_uring/zcrx.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/io_uring/zcrx.c b/io_uring/zcrx.c
index 262ac73..700eff9 100644
--- a/io_uring/zcrx.c
+++ b/io_uring/zcrx.c
@@ -637,19 +637,10 @@ static int zcrx_export(struct io_ring_ctx *ctx, struct io_zcrx_ifq *ifq,
 {
 	struct zcrx_ctrl_export *ce = &ctrl->zc_export;
 	struct file *file;
-	int fd = -1;
+	int fd;
 
 	if (!mem_is_zero(ce, sizeof(*ce)))
 		return -EINVAL;
-	fd = get_unused_fd_flags(O_CLOEXEC);
-	if (fd < 0)
-		return fd;
-
-	ce->zcrx_fd = fd;
-	if (copy_to_user(arg, ctrl, sizeof(*ctrl))) {
-		put_unused_fd(fd);
-		return -EFAULT;
-	}
 
 	refcount_inc(&ifq->refs);
 	refcount_inc(&ifq->user_refs);
@@ -657,11 +648,23 @@ static int zcrx_export(struct io_ring_ctx *ctx, struct io_zcrx_ifq *ifq,
 	file = anon_inode_create_getfile("[zcrx]", &zcrx_box_fops,
 					 ifq, O_CLOEXEC, NULL);
 	if (IS_ERR(file)) {
-		put_unused_fd(fd);
 		zcrx_unregister(ifq);
 		return PTR_ERR(file);
 	}
 
+	fd = get_unused_fd_flags(O_CLOEXEC);
+	if (fd < 0) {
+		fput(file);
+		return fd;
+	}
+
+	ce->zcrx_fd = fd;
+	if (copy_to_user(arg, ctrl, sizeof(*ctrl))) {
+		fput(file);
+		put_unused_fd(fd);
+		return -EFAULT;
+	}
+
 	fd_install(fd, file);
 	return 0;
 }
-- 
2.50.1 (Apple Git-155)

Re: [PATCH] io_uring/zcrx: reorder fd allocation and disclosure in zcrx_export()

[Jens Axboe]

On 4/5/26 5:53 PM, Bertie Tryner wrote:
> Currently, zcrx_export() allocates and discloses a file descriptor to
> userspace before the backing file is successfully created. If file
> creation fails, the fd is released back to the pool, but the number
> has already been written to the user-provided control structure.
> 
> While this requires a misbehaving or racing userspace to trigger,
> it is better practice to ensure the file descriptor is only
> disclosed once the operation is guaranteed to succeed. This aligns
> the ZCRX export logic with the standard patterns used in the VFS
> layer and other fd-publishing paths.

Like I explained earlier, there's no "race" here at all. The file is
never visible until fd_install() has been done. Any attempt to use the
fd before that happens will get a NULL file in the kernel, and the IO
operation failed.

The operation clearly fails, and the error is returned to the
application. If the application is so buggy that it ignores that and
wants to use the 'fd' value, then it's just buggy. Simple as that, do
stupid things and win stupid prizes.

As a cleanup, this is fine. But the commit message is horribly
(deliberately?) misleading and that should get fixed. I'll let Pavel
decide what to do with this change.

-- 
Jens Axboe

Re: [PATCH] io_uring/zcrx: reorder fd allocation and disclosure in zcrx_export()

[TRYNER, BERTIE (UG)]

Hi Jens,
Apologies for the confusion. I'm a first-year student and still learning the specific terminology and norms for these reports; I certainly didn't intend to be misleading. I've just sent over a V2 via git send-email that frames this purely as a cleanup.
Thanks for the guidance!
Bertie

________________________________________
From: TRYNER, BERTIE (UG) <Bertie.Tryner@warwick.ac.uk>
Sent: 06 April 2026 18:05
To: Jens Axboe <axboe@kernel.dk>; Bertie Tryner <bertietryner@gmail.com>; io-uring@vger.kernel.org <io-uring@vger.kernel.org>
Cc: asml.silence@gmail.com <asml.silence@gmail.com>
Subject: Re: [PATCH] io_uring/zcrx: reorder fd allocation and disclosure in zcrx_export()
 
Hi Jens, 

Apologies for the confusion. Didn't mean to make it sound like there was a security issue in the commit message. Ive just sent over a V2, I hope the amended message is appropriate. Thanks alot!

Bertie Tryner
________________________________________
From: Jens Axboe <axboe@kernel.dk>
Sent: 06 April 2026 17:20
To: Bertie Tryner <bertietryner@gmail.com>; io-uring@vger.kernel.org <io-uring@vger.kernel.org>
Cc: asml.silence@gmail.com <asml.silence@gmail.com>; TRYNER, BERTIE (UG) <Bertie.Tryner@warwick.ac.uk>
Subject: Re: [PATCH] io_uring/zcrx: reorder fd allocation and disclosure in zcrx_export()
 
[You don't often get email from axboe@kernel.dk. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]

On 4/5/26 5:53 PM, Bertie Tryner wrote:
> Currently, zcrx_export() allocates and discloses a file descriptor to
> userspace before the backing file is successfully created. If file
> creation fails, the fd is released back to the pool, but the number
> has already been written to the user-provided control structure.
>
> While this requires a misbehaving or racing userspace to trigger,
> it is better practice to ensure the file descriptor is only
> disclosed once the operation is guaranteed to succeed. This aligns
> the ZCRX export logic with the standard patterns used in the VFS
> layer and other fd-publishing paths.

Like I explained earlier, there's no "race" here at all. The file is
never visible until fd_install() has been done. Any attempt to use the
fd before that happens will get a NULL file in the kernel, and the IO
operation failed.

The operation clearly fails, and the error is returned to the
application. If the application is so buggy that it ignores that and
wants to use the 'fd' value, then it's just buggy. Simple as that, do
stupid things and win stupid prizes.

As a cleanup, this is fine. But the commit message is horribly
(deliberately?) misleading and that should get fixed. I'll let Pavel
decide what to do with this change.

--
Jens Axboe