public inbox for [email protected]
 help / color / mirror / Atom feed
* [PATCH 0/2] Fix locking for MSG_RING and IOPOLL targets
@ 2023-01-19 18:02 Jens Axboe
  2023-01-19 18:02 ` [PATCH 1/2] io_uring/msg_ring: move double lock/unlock helpers higher up Jens Axboe
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Jens Axboe @ 2023-01-19 18:02 UTC (permalink / raw)
  To: io-uring; +Cc: asml.silence

Hi,

First patch is just a prep patch, patch two adds conditional locking
for posting CQEs to an IOPOLL based target.

-- 
Jens Axboe



^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/2] io_uring/msg_ring: move double lock/unlock helpers higher up
  2023-01-19 18:02 [PATCH 0/2] Fix locking for MSG_RING and IOPOLL targets Jens Axboe
@ 2023-01-19 18:02 ` Jens Axboe
  2023-01-19 18:02 ` [PATCH 2/2] io_uring/msg_ring: fix missing lock on overflow for IOPOLL Jens Axboe
  2023-08-22 20:54 ` [PATCH 0/2] Fix locking for MSG_RING and IOPOLL targets Oleksandr Tymoshenko
  2 siblings, 0 replies; 7+ messages in thread
From: Jens Axboe @ 2023-01-19 18:02 UTC (permalink / raw)
  To: io-uring; +Cc: asml.silence, Jens Axboe

In preparation for needing them somewhere else, move them and get rid of
the unused 'issue_flags' for the unlock side.

No functional changes in this patch.

Signed-off-by: Jens Axboe <[email protected]>
---
 io_uring/msg_ring.c | 47 ++++++++++++++++++++++-----------------------
 1 file changed, 23 insertions(+), 24 deletions(-)

diff --git a/io_uring/msg_ring.c b/io_uring/msg_ring.c
index 2d3cd945a531..321f5eafef99 100644
--- a/io_uring/msg_ring.c
+++ b/io_uring/msg_ring.c
@@ -25,6 +25,28 @@ struct io_msg {
 	u32 flags;
 };
 
+static void io_double_unlock_ctx(struct io_ring_ctx *octx)
+{
+	mutex_unlock(&octx->uring_lock);
+}
+
+static int io_double_lock_ctx(struct io_ring_ctx *octx,
+			      unsigned int issue_flags)
+{
+	/*
+	 * To ensure proper ordering between the two ctxs, we can only
+	 * attempt a trylock on the target. If that fails and we already have
+	 * the source ctx lock, punt to io-wq.
+	 */
+	if (!(issue_flags & IO_URING_F_UNLOCKED)) {
+		if (!mutex_trylock(&octx->uring_lock))
+			return -EAGAIN;
+		return 0;
+	}
+	mutex_lock(&octx->uring_lock);
+	return 0;
+}
+
 void io_msg_ring_cleanup(struct io_kiocb *req)
 {
 	struct io_msg *msg = io_kiocb_to_cmd(req, struct io_msg);
@@ -77,29 +99,6 @@ static int io_msg_ring_data(struct io_kiocb *req)
 	return -EOVERFLOW;
 }
 
-static void io_double_unlock_ctx(struct io_ring_ctx *octx,
-				 unsigned int issue_flags)
-{
-	mutex_unlock(&octx->uring_lock);
-}
-
-static int io_double_lock_ctx(struct io_ring_ctx *octx,
-			      unsigned int issue_flags)
-{
-	/*
-	 * To ensure proper ordering between the two ctxs, we can only
-	 * attempt a trylock on the target. If that fails and we already have
-	 * the source ctx lock, punt to io-wq.
-	 */
-	if (!(issue_flags & IO_URING_F_UNLOCKED)) {
-		if (!mutex_trylock(&octx->uring_lock))
-			return -EAGAIN;
-		return 0;
-	}
-	mutex_lock(&octx->uring_lock);
-	return 0;
-}
-
 static struct file *io_msg_grab_file(struct io_kiocb *req, unsigned int issue_flags)
 {
 	struct io_msg *msg = io_kiocb_to_cmd(req, struct io_msg);
@@ -148,7 +147,7 @@ static int io_msg_install_complete(struct io_kiocb *req, unsigned int issue_flag
 	if (!io_post_aux_cqe(target_ctx, msg->user_data, msg->len, 0))
 		ret = -EOVERFLOW;
 out_unlock:
-	io_double_unlock_ctx(target_ctx, issue_flags);
+	io_double_unlock_ctx(target_ctx);
 	return ret;
 }
 
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/2] io_uring/msg_ring: fix missing lock on overflow for IOPOLL
  2023-01-19 18:02 [PATCH 0/2] Fix locking for MSG_RING and IOPOLL targets Jens Axboe
  2023-01-19 18:02 ` [PATCH 1/2] io_uring/msg_ring: move double lock/unlock helpers higher up Jens Axboe
@ 2023-01-19 18:02 ` Jens Axboe
  2023-08-22 20:54 ` [PATCH 0/2] Fix locking for MSG_RING and IOPOLL targets Oleksandr Tymoshenko
  2 siblings, 0 replies; 7+ messages in thread
From: Jens Axboe @ 2023-01-19 18:02 UTC (permalink / raw)
  To: io-uring; +Cc: asml.silence, Jens Axboe, Xingyuan Mo

If the target ring is configured with IOPOLL, then we always need to hold
the target ring uring_lock before posting CQEs. We could just grab it
unconditionally, but since we don't expect many target rings to be of this
type, make grabbing the uring_lock conditional on the ring type.

Link: https://lore.kernel.org/io-uring/Y8krlYa52%[email protected]/
Reported-by: Xingyuan Mo <[email protected]>
Signed-off-by: Jens Axboe <[email protected]>
---
 io_uring/msg_ring.c | 39 ++++++++++++++++++++++++++++++---------
 1 file changed, 30 insertions(+), 9 deletions(-)

diff --git a/io_uring/msg_ring.c b/io_uring/msg_ring.c
index 321f5eafef99..a333781565d3 100644
--- a/io_uring/msg_ring.c
+++ b/io_uring/msg_ring.c
@@ -65,20 +65,33 @@ static void io_msg_tw_complete(struct callback_head *head)
 	struct io_ring_ctx *target_ctx = req->file->private_data;
 	int ret = 0;
 
-	if (current->flags & PF_EXITING)
+	if (current->flags & PF_EXITING) {
 		ret = -EOWNERDEAD;
-	else if (!io_post_aux_cqe(target_ctx, msg->user_data, msg->len, 0))
-		ret = -EOVERFLOW;
+	} else {
+		/*
+		 * If the target ring is using IOPOLL mode, then we need to be
+		 * holding the uring_lock for posting completions. Other ring
+		 * types rely on the regular completion locking, which is
+		 * handled while posting.
+		 */
+		if (target_ctx->flags & IORING_SETUP_IOPOLL)
+			mutex_lock(&target_ctx->uring_lock);
+		if (!io_post_aux_cqe(target_ctx, msg->user_data, msg->len, 0))
+			ret = -EOVERFLOW;
+		if (target_ctx->flags & IORING_SETUP_IOPOLL)
+			mutex_unlock(&target_ctx->uring_lock);
+	}
 
 	if (ret < 0)
 		req_set_fail(req);
 	io_req_queue_tw_complete(req, ret);
 }
 
-static int io_msg_ring_data(struct io_kiocb *req)
+static int io_msg_ring_data(struct io_kiocb *req, unsigned int issue_flags)
 {
 	struct io_ring_ctx *target_ctx = req->file->private_data;
 	struct io_msg *msg = io_kiocb_to_cmd(req, struct io_msg);
+	int ret;
 
 	if (msg->src_fd || msg->dst_fd || msg->flags)
 		return -EINVAL;
@@ -93,10 +106,18 @@ static int io_msg_ring_data(struct io_kiocb *req)
 		return IOU_ISSUE_SKIP_COMPLETE;
 	}
 
-	if (io_post_aux_cqe(target_ctx, msg->user_data, msg->len, 0))
-		return 0;
-
-	return -EOVERFLOW;
+	ret = -EOVERFLOW;
+	if (target_ctx->flags & IORING_SETUP_IOPOLL) {
+		if (unlikely(io_double_lock_ctx(target_ctx, issue_flags)))
+			return -EAGAIN;
+		if (io_post_aux_cqe(target_ctx, msg->user_data, msg->len, 0))
+			ret = 0;
+		io_double_unlock_ctx(target_ctx);
+	} else {
+		if (io_post_aux_cqe(target_ctx, msg->user_data, msg->len, 0))
+			ret = 0;
+	}
+	return ret;
 }
 
 static struct file *io_msg_grab_file(struct io_kiocb *req, unsigned int issue_flags)
@@ -223,7 +244,7 @@ int io_msg_ring(struct io_kiocb *req, unsigned int issue_flags)
 
 	switch (msg->cmd) {
 	case IORING_MSG_DATA:
-		ret = io_msg_ring_data(req);
+		ret = io_msg_ring_data(req, issue_flags);
 		break;
 	case IORING_MSG_SEND_FD:
 		ret = io_msg_send_fd(req, issue_flags);
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 0/2] Fix locking for MSG_RING and IOPOLL targets
  2023-01-19 18:02 [PATCH 0/2] Fix locking for MSG_RING and IOPOLL targets Jens Axboe
  2023-01-19 18:02 ` [PATCH 1/2] io_uring/msg_ring: move double lock/unlock helpers higher up Jens Axboe
  2023-01-19 18:02 ` [PATCH 2/2] io_uring/msg_ring: fix missing lock on overflow for IOPOLL Jens Axboe
@ 2023-08-22 20:54 ` Oleksandr Tymoshenko
  2023-08-22 22:19   ` Gabriel Krisman Bertazi
  2023-08-22 23:30   ` Jens Axboe
  2 siblings, 2 replies; 7+ messages in thread
From: Oleksandr Tymoshenko @ 2023-08-22 20:54 UTC (permalink / raw)
  To: axboe; +Cc: asml.silence, io-uring

Hello,

Does this patchset need to be backported to 6.1?

Thank you

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/2] Fix locking for MSG_RING and IOPOLL targets
  2023-08-22 20:54 ` [PATCH 0/2] Fix locking for MSG_RING and IOPOLL targets Oleksandr Tymoshenko
@ 2023-08-22 22:19   ` Gabriel Krisman Bertazi
  2023-08-22 23:30   ` Jens Axboe
  1 sibling, 0 replies; 7+ messages in thread
From: Gabriel Krisman Bertazi @ 2023-08-22 22:19 UTC (permalink / raw)
  To: Oleksandr Tymoshenko; +Cc: axboe, asml.silence, io-uring

Oleksandr Tymoshenko <[email protected]> writes:

> Hello,
>
> Does this patchset need to be backported to 6.1?

Yes. Seems to be missing in linux-6.1.y and it is a security fix.  Do you want to send
a backport?

-- 
Gabriel Krisman Bertazi

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/2] Fix locking for MSG_RING and IOPOLL targets
  2023-08-22 20:54 ` [PATCH 0/2] Fix locking for MSG_RING and IOPOLL targets Oleksandr Tymoshenko
  2023-08-22 22:19   ` Gabriel Krisman Bertazi
@ 2023-08-22 23:30   ` Jens Axboe
  2023-08-23  0:06     ` Jens Axboe
  1 sibling, 1 reply; 7+ messages in thread
From: Jens Axboe @ 2023-08-22 23:30 UTC (permalink / raw)
  To: Oleksandr Tymoshenko; +Cc: asml.silence, io-uring

On 8/22/23 2:54 PM, Oleksandr Tymoshenko wrote:
> Hello,
> 
> Does this patchset need to be backported to 6.1?

Yeah it probably should, guessing it didn't get picked as there are
dependencies. I'll take a look.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/2] Fix locking for MSG_RING and IOPOLL targets
  2023-08-22 23:30   ` Jens Axboe
@ 2023-08-23  0:06     ` Jens Axboe
  0 siblings, 0 replies; 7+ messages in thread
From: Jens Axboe @ 2023-08-23  0:06 UTC (permalink / raw)
  To: Oleksandr Tymoshenko; +Cc: asml.silence, io-uring

[-- Attachment #1: Type: text/plain, Size: 324 bytes --]

On 8/22/23 5:30 PM, Jens Axboe wrote:
> On 8/22/23 2:54 PM, Oleksandr Tymoshenko wrote:
>> Hello,
>>
>> Does this patchset need to be backported to 6.1?
> 
> Yeah it probably should, guessing it didn't get picked as there are
> dependencies. I'll take a look.

Here they are - I'll send them in for stable.

-- 
Jens Axboe


[-- Attachment #2: 0004-io_uring-msg_ring-fix-missing-lock-on-overflow-for-I.patch --]
[-- Type: text/x-patch, Size: 2379 bytes --]

From c47db9ca937e232606060077bb09cff49e75dbb7 Mon Sep 17 00:00:00 2001
From: Jens Axboe <[email protected]>
Date: Tue, 22 Aug 2023 18:00:02 -0600
Subject: [PATCH 4/4] io_uring/msg_ring: fix missing lock on overflow for
 IOPOLL

Commit e12d7a46f65ae4b7d58a5e0c1cbfa825cf8d830d upstream.

If the target ring is configured with IOPOLL, then we always need to hold
the target ring uring_lock before posting CQEs. We could just grab it
unconditionally, but since we don't expect many target rings to be of this
type, make grabbing the uring_lock conditional on the ring type.

Link: https://lore.kernel.org/io-uring/Y8krlYa52%[email protected]/
Reported-by: Xingyuan Mo <[email protected]>
Signed-off-by: Jens Axboe <[email protected]>
---
 io_uring/msg_ring.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/io_uring/msg_ring.c b/io_uring/msg_ring.c
index 825d8c579fd3..cd922d2bef5f 100644
--- a/io_uring/msg_ring.c
+++ b/io_uring/msg_ring.c
@@ -57,20 +57,30 @@ void io_msg_ring_cleanup(struct io_kiocb *req)
 	msg->src_file = NULL;
 }
 
-static int io_msg_ring_data(struct io_kiocb *req)
+static int io_msg_ring_data(struct io_kiocb *req, unsigned int issue_flags)
 {
 	struct io_ring_ctx *target_ctx = req->file->private_data;
 	struct io_msg *msg = io_kiocb_to_cmd(req, struct io_msg);
+	int ret;
 
 	if (msg->src_fd || msg->dst_fd || msg->flags)
 		return -EINVAL;
 	if (target_ctx->flags & IORING_SETUP_R_DISABLED)
 		return -EBADFD;
 
-	if (io_post_aux_cqe(target_ctx, msg->user_data, msg->len, 0, true))
-		return 0;
+	ret = -EOVERFLOW;
+	if (target_ctx->flags & IORING_SETUP_IOPOLL) {
+		if (unlikely(io_double_lock_ctx(target_ctx, issue_flags)))
+			return -EAGAIN;
+		if (io_post_aux_cqe(target_ctx, msg->user_data, msg->len, 0, true))
+			ret = 0;
+		io_double_unlock_ctx(target_ctx);
+	} else {
+		if (io_post_aux_cqe(target_ctx, msg->user_data, msg->len, 0, true))
+			ret = 0;
+	}
 
-	return -EOVERFLOW;
+	return ret;
 }
 
 static struct file *io_msg_grab_file(struct io_kiocb *req, unsigned int issue_flags)
@@ -175,7 +185,7 @@ int io_msg_ring(struct io_kiocb *req, unsigned int issue_flags)
 
 	switch (msg->cmd) {
 	case IORING_MSG_DATA:
-		ret = io_msg_ring_data(req);
+		ret = io_msg_ring_data(req, issue_flags);
 		break;
 	case IORING_MSG_SEND_FD:
 		ret = io_msg_send_fd(req, issue_flags);
-- 
2.40.1


[-- Attachment #3: 0003-io_uring-msg_ring-move-double-lock-unlock-helpers-hi.patch --]
[-- Type: text/x-patch, Size: 2648 bytes --]

From 5c3ffa8ec64e0726dcbfe37b5d126701301011ed Mon Sep 17 00:00:00 2001
From: Jens Axboe <[email protected]>
Date: Thu, 19 Jan 2023 09:01:27 -0700
Subject: [PATCH 3/4] io_uring/msg_ring: move double lock/unlock helpers higher
 up

Commit 423d5081d0451faa59a707e57373801da5b40141 upstream.

In preparation for needing them somewhere else, move them and get rid of
the unused 'issue_flags' for the unlock side.

No functional changes in this patch.

Signed-off-by: Jens Axboe <[email protected]>
---
 io_uring/msg_ring.c | 47 ++++++++++++++++++++++-----------------------
 1 file changed, 23 insertions(+), 24 deletions(-)

diff --git a/io_uring/msg_ring.c b/io_uring/msg_ring.c
index fd96a0cd85f0..825d8c579fd3 100644
--- a/io_uring/msg_ring.c
+++ b/io_uring/msg_ring.c
@@ -24,6 +24,28 @@ struct io_msg {
 	u32 flags;
 };
 
+static void io_double_unlock_ctx(struct io_ring_ctx *octx)
+{
+	mutex_unlock(&octx->uring_lock);
+}
+
+static int io_double_lock_ctx(struct io_ring_ctx *octx,
+			      unsigned int issue_flags)
+{
+	/*
+	 * To ensure proper ordering between the two ctxs, we can only
+	 * attempt a trylock on the target. If that fails and we already have
+	 * the source ctx lock, punt to io-wq.
+	 */
+	if (!(issue_flags & IO_URING_F_UNLOCKED)) {
+		if (!mutex_trylock(&octx->uring_lock))
+			return -EAGAIN;
+		return 0;
+	}
+	mutex_lock(&octx->uring_lock);
+	return 0;
+}
+
 void io_msg_ring_cleanup(struct io_kiocb *req)
 {
 	struct io_msg *msg = io_kiocb_to_cmd(req, struct io_msg);
@@ -51,29 +73,6 @@ static int io_msg_ring_data(struct io_kiocb *req)
 	return -EOVERFLOW;
 }
 
-static void io_double_unlock_ctx(struct io_ring_ctx *octx,
-				 unsigned int issue_flags)
-{
-	mutex_unlock(&octx->uring_lock);
-}
-
-static int io_double_lock_ctx(struct io_ring_ctx *octx,
-			      unsigned int issue_flags)
-{
-	/*
-	 * To ensure proper ordering between the two ctxs, we can only
-	 * attempt a trylock on the target. If that fails and we already have
-	 * the source ctx lock, punt to io-wq.
-	 */
-	if (!(issue_flags & IO_URING_F_UNLOCKED)) {
-		if (!mutex_trylock(&octx->uring_lock))
-			return -EAGAIN;
-		return 0;
-	}
-	mutex_lock(&octx->uring_lock);
-	return 0;
-}
-
 static struct file *io_msg_grab_file(struct io_kiocb *req, unsigned int issue_flags)
 {
 	struct io_msg *msg = io_kiocb_to_cmd(req, struct io_msg);
@@ -122,7 +121,7 @@ static int io_msg_install_complete(struct io_kiocb *req, unsigned int issue_flag
 	if (!io_post_aux_cqe(target_ctx, msg->user_data, ret, 0, true))
 		ret = -EOVERFLOW;
 out_unlock:
-	io_double_unlock_ctx(target_ctx, issue_flags);
+	io_double_unlock_ctx(target_ctx);
 	return ret;
 }
 
-- 
2.40.1


[-- Attachment #4: 0002-io_uring-extract-a-io_msg_install_complete-helper.patch --]
[-- Type: text/x-patch, Size: 2957 bytes --]

From ef7b782465bd6943fbcacb4af4fbe790137a5820 Mon Sep 17 00:00:00 2001
From: Pavel Begunkov <[email protected]>
Date: Wed, 7 Dec 2022 03:53:35 +0000
Subject: [PATCH 2/4] io_uring: extract a io_msg_install_complete helper

Commit 172113101641cf1f9628c528ec790cb809f2b704 upstream.

Extract a helper called io_msg_install_complete() from io_msg_send_fd(),
will be used later.

Signed-off-by: Pavel Begunkov <[email protected]>
Link: https://lore.kernel.org/r/1500ca1054cc4286a3ee1c60aacead57fcdfa02a.1670384893.git.asml.silence@gmail.com
Signed-off-by: Jens Axboe <[email protected]>
---
 io_uring/msg_ring.c | 38 +++++++++++++++++++++-----------------
 1 file changed, 21 insertions(+), 17 deletions(-)

diff --git a/io_uring/msg_ring.c b/io_uring/msg_ring.c
index ee8e7ac8c582..fd96a0cd85f0 100644
--- a/io_uring/msg_ring.c
+++ b/io_uring/msg_ring.c
@@ -94,40 +94,25 @@ static struct file *io_msg_grab_file(struct io_kiocb *req, unsigned int issue_fl
 	return file;
 }
 
-static int io_msg_send_fd(struct io_kiocb *req, unsigned int issue_flags)
+static int io_msg_install_complete(struct io_kiocb *req, unsigned int issue_flags)
 {
 	struct io_ring_ctx *target_ctx = req->file->private_data;
 	struct io_msg *msg = io_kiocb_to_cmd(req, struct io_msg);
-	struct io_ring_ctx *ctx = req->ctx;
 	struct file *src_file = msg->src_file;
 	int ret;
 
-	if (msg->len)
-		return -EINVAL;
-	if (target_ctx == ctx)
-		return -EINVAL;
-	if (target_ctx->flags & IORING_SETUP_R_DISABLED)
-		return -EBADFD;
-	if (!src_file) {
-		src_file = io_msg_grab_file(req, issue_flags);
-		if (!src_file)
-			return -EBADF;
-		msg->src_file = src_file;
-		req->flags |= REQ_F_NEED_CLEANUP;
-	}
-
 	if (unlikely(io_double_lock_ctx(target_ctx, issue_flags)))
 		return -EAGAIN;
 
 	ret = __io_fixed_fd_install(target_ctx, src_file, msg->dst_fd);
 	if (ret < 0)
 		goto out_unlock;
+
 	msg->src_file = NULL;
 	req->flags &= ~REQ_F_NEED_CLEANUP;
 
 	if (msg->flags & IORING_MSG_RING_CQE_SKIP)
 		goto out_unlock;
-
 	/*
 	 * If this fails, the target still received the file descriptor but
 	 * wasn't notified of the fact. This means that if this request
@@ -141,6 +126,25 @@ static int io_msg_send_fd(struct io_kiocb *req, unsigned int issue_flags)
 	return ret;
 }
 
+static int io_msg_send_fd(struct io_kiocb *req, unsigned int issue_flags)
+{
+	struct io_ring_ctx *target_ctx = req->file->private_data;
+	struct io_msg *msg = io_kiocb_to_cmd(req, struct io_msg);
+	struct io_ring_ctx *ctx = req->ctx;
+	struct file *src_file = msg->src_file;
+
+	if (target_ctx == ctx)
+		return -EINVAL;
+	if (!src_file) {
+		src_file = io_msg_grab_file(req, issue_flags);
+		if (!src_file)
+			return -EBADF;
+		msg->src_file = src_file;
+		req->flags |= REQ_F_NEED_CLEANUP;
+	}
+	return io_msg_install_complete(req, issue_flags);
+}
+
 int io_msg_ring_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 {
 	struct io_msg *msg = io_kiocb_to_cmd(req, struct io_msg);
-- 
2.40.1


[-- Attachment #5: 0001-io_uring-get-rid-of-double-locking.patch --]
[-- Type: text/x-patch, Size: 6081 bytes --]

From f10ba483becae0b8c7de0e9fe3e0f6d07a405b7a Mon Sep 17 00:00:00 2001
From: Pavel Begunkov <[email protected]>
Date: Wed, 7 Dec 2022 03:53:34 +0000
Subject: [PATCH 1/4] io_uring: get rid of double locking

Commit 11373026f2960390d5e330df4e92735c4265c440 upstream.

We don't need to take both uring_locks at once, msg_ring can be split in
two parts, first getting a file from the filetable of the first ring and
then installing it into the second one.

Signed-off-by: Pavel Begunkov <[email protected]>
Link: https://lore.kernel.org/r/a80ecc2bc99c3b3f2cf20015d618b7c51419a797.1670384893.git.asml.silence@gmail.com
Signed-off-by: Jens Axboe <[email protected]>
---
 io_uring/msg_ring.c | 85 ++++++++++++++++++++++++++-------------------
 io_uring/msg_ring.h |  1 +
 io_uring/opdef.c    |  1 +
 3 files changed, 51 insertions(+), 36 deletions(-)

diff --git a/io_uring/msg_ring.c b/io_uring/msg_ring.c
index 3526389ac218..ee8e7ac8c582 100644
--- a/io_uring/msg_ring.c
+++ b/io_uring/msg_ring.c
@@ -15,6 +15,7 @@
 
 struct io_msg {
 	struct file			*file;
+	struct file			*src_file;
 	u64 user_data;
 	u32 len;
 	u32 cmd;
@@ -23,6 +24,17 @@ struct io_msg {
 	u32 flags;
 };
 
+void io_msg_ring_cleanup(struct io_kiocb *req)
+{
+	struct io_msg *msg = io_kiocb_to_cmd(req, struct io_msg);
+
+	if (WARN_ON_ONCE(!msg->src_file))
+		return;
+
+	fput(msg->src_file);
+	msg->src_file = NULL;
+}
+
 static int io_msg_ring_data(struct io_kiocb *req)
 {
 	struct io_ring_ctx *target_ctx = req->file->private_data;
@@ -39,17 +51,13 @@ static int io_msg_ring_data(struct io_kiocb *req)
 	return -EOVERFLOW;
 }
 
-static void io_double_unlock_ctx(struct io_ring_ctx *ctx,
-				 struct io_ring_ctx *octx,
+static void io_double_unlock_ctx(struct io_ring_ctx *octx,
 				 unsigned int issue_flags)
 {
-	if (issue_flags & IO_URING_F_UNLOCKED)
-		mutex_unlock(&ctx->uring_lock);
 	mutex_unlock(&octx->uring_lock);
 }
 
-static int io_double_lock_ctx(struct io_ring_ctx *ctx,
-			      struct io_ring_ctx *octx,
+static int io_double_lock_ctx(struct io_ring_ctx *octx,
 			      unsigned int issue_flags)
 {
 	/*
@@ -62,17 +70,28 @@ static int io_double_lock_ctx(struct io_ring_ctx *ctx,
 			return -EAGAIN;
 		return 0;
 	}
+	mutex_lock(&octx->uring_lock);
+	return 0;
+}
 
-	/* Always grab smallest value ctx first. We know ctx != octx. */
-	if (ctx < octx) {
-		mutex_lock(&ctx->uring_lock);
-		mutex_lock(&octx->uring_lock);
-	} else {
-		mutex_lock(&octx->uring_lock);
-		mutex_lock(&ctx->uring_lock);
+static struct file *io_msg_grab_file(struct io_kiocb *req, unsigned int issue_flags)
+{
+	struct io_msg *msg = io_kiocb_to_cmd(req, struct io_msg);
+	struct io_ring_ctx *ctx = req->ctx;
+	struct file *file = NULL;
+	unsigned long file_ptr;
+	int idx = msg->src_fd;
+
+	io_ring_submit_lock(ctx, issue_flags);
+	if (likely(idx < ctx->nr_user_files)) {
+		idx = array_index_nospec(idx, ctx->nr_user_files);
+		file_ptr = io_fixed_file_slot(&ctx->file_table, idx)->file_ptr;
+		file = (struct file *) (file_ptr & FFS_MASK);
+		if (file)
+			get_file(file);
 	}
-
-	return 0;
+	io_ring_submit_unlock(ctx, issue_flags);
+	return file;
 }
 
 static int io_msg_send_fd(struct io_kiocb *req, unsigned int issue_flags)
@@ -80,8 +99,7 @@ static int io_msg_send_fd(struct io_kiocb *req, unsigned int issue_flags)
 	struct io_ring_ctx *target_ctx = req->file->private_data;
 	struct io_msg *msg = io_kiocb_to_cmd(req, struct io_msg);
 	struct io_ring_ctx *ctx = req->ctx;
-	unsigned long file_ptr;
-	struct file *src_file;
+	struct file *src_file = msg->src_file;
 	int ret;
 
 	if (msg->len)
@@ -90,28 +108,22 @@ static int io_msg_send_fd(struct io_kiocb *req, unsigned int issue_flags)
 		return -EINVAL;
 	if (target_ctx->flags & IORING_SETUP_R_DISABLED)
 		return -EBADFD;
+	if (!src_file) {
+		src_file = io_msg_grab_file(req, issue_flags);
+		if (!src_file)
+			return -EBADF;
+		msg->src_file = src_file;
+		req->flags |= REQ_F_NEED_CLEANUP;
+	}
 
-	ret = io_double_lock_ctx(ctx, target_ctx, issue_flags);
-	if (unlikely(ret))
-		return ret;
-
-	ret = -EBADF;
-	if (unlikely(msg->src_fd >= ctx->nr_user_files))
-		goto out_unlock;
-
-	msg->src_fd = array_index_nospec(msg->src_fd, ctx->nr_user_files);
-	file_ptr = io_fixed_file_slot(&ctx->file_table, msg->src_fd)->file_ptr;
-	if (!file_ptr)
-		goto out_unlock;
-
-	src_file = (struct file *) (file_ptr & FFS_MASK);
-	get_file(src_file);
+	if (unlikely(io_double_lock_ctx(target_ctx, issue_flags)))
+		return -EAGAIN;
 
 	ret = __io_fixed_fd_install(target_ctx, src_file, msg->dst_fd);
-	if (ret < 0) {
-		fput(src_file);
+	if (ret < 0)
 		goto out_unlock;
-	}
+	msg->src_file = NULL;
+	req->flags &= ~REQ_F_NEED_CLEANUP;
 
 	if (msg->flags & IORING_MSG_RING_CQE_SKIP)
 		goto out_unlock;
@@ -125,7 +137,7 @@ static int io_msg_send_fd(struct io_kiocb *req, unsigned int issue_flags)
 	if (!io_post_aux_cqe(target_ctx, msg->user_data, ret, 0, true))
 		ret = -EOVERFLOW;
 out_unlock:
-	io_double_unlock_ctx(ctx, target_ctx, issue_flags);
+	io_double_unlock_ctx(target_ctx, issue_flags);
 	return ret;
 }
 
@@ -136,6 +148,7 @@ int io_msg_ring_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
 	if (unlikely(sqe->buf_index || sqe->personality))
 		return -EINVAL;
 
+	msg->src_file = NULL;
 	msg->user_data = READ_ONCE(sqe->off);
 	msg->len = READ_ONCE(sqe->len);
 	msg->cmd = READ_ONCE(sqe->addr);
diff --git a/io_uring/msg_ring.h b/io_uring/msg_ring.h
index fb9601f202d0..3987ee6c0e5f 100644
--- a/io_uring/msg_ring.h
+++ b/io_uring/msg_ring.h
@@ -2,3 +2,4 @@
 
 int io_msg_ring_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe);
 int io_msg_ring(struct io_kiocb *req, unsigned int issue_flags);
+void io_msg_ring_cleanup(struct io_kiocb *req);
diff --git a/io_uring/opdef.c b/io_uring/opdef.c
index 04dd2c983fce..3aa0d65c50e3 100644
--- a/io_uring/opdef.c
+++ b/io_uring/opdef.c
@@ -445,6 +445,7 @@ const struct io_op_def io_op_defs[] = {
 		.name			= "MSG_RING",
 		.prep			= io_msg_ring_prep,
 		.issue			= io_msg_ring,
+		.cleanup		= io_msg_ring_cleanup,
 	},
 	[IORING_OP_FSETXATTR] = {
 		.needs_file = 1,
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2023-08-23  0:06 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-01-19 18:02 [PATCH 0/2] Fix locking for MSG_RING and IOPOLL targets Jens Axboe
2023-01-19 18:02 ` [PATCH 1/2] io_uring/msg_ring: move double lock/unlock helpers higher up Jens Axboe
2023-01-19 18:02 ` [PATCH 2/2] io_uring/msg_ring: fix missing lock on overflow for IOPOLL Jens Axboe
2023-08-22 20:54 ` [PATCH 0/2] Fix locking for MSG_RING and IOPOLL targets Oleksandr Tymoshenko
2023-08-22 22:19   ` Gabriel Krisman Bertazi
2023-08-22 23:30   ` Jens Axboe
2023-08-23  0:06     ` Jens Axboe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox