Data Corruption bug with Samba's vfs_iouring and Linux 5.6.7/5.7rc3

[Stefan Metzmacher]

Am 08.05.20 um 22:51 schrieb Jeremy Allison:
> On Fri, May 08, 2020 at 01:47:09PM -0700, Jeremy Allison via samba-technical wrote:
>> On Fri, May 08, 2020 at 09:35:31PM +0200, Stefan Metzmacher via samba-technical wrote:
>>>
>>> Thanks very much Jeremy! I didn't noticed that.
>>>
>>> I guess the attached patch should be able to fix the recursion.
>>
>> Oh Metze that's *really* ugly :-). I thought about
>> doing that in my code and decided it was in too bad
>> taste to live :-).
>>
>> Is there a cleaner way than putting "busy" and "retry"
>> variables in the config struct ?
> 
> And a "Goto again" as well :-(. Bleegh.

This version would actually work and looks a bit similar to
your version.

Can you life with that version?

metze

-------------- next part --------------
From ce467400f4098d0edbee9534a5667c0b92422e8f Mon Sep 17 00:00:00 2001
From: Stefan Metzmacher <metze at samba.org>
Date: Fri, 8 May 2020 21:29:53 +0200
Subject: [PATCH] vfs_io_uring: avoid stack recursion of
 vfs_io_uring_queue_run()

Instead we remember if recursion was triggered and jump to
the start of the function again from the end.

This should make it safe to be called from the completion_fn().

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14361

Signed-off-by: Stefan Metzmacher <metze at samba.org>
---
 source3/modules/vfs_io_uring.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/source3/modules/vfs_io_uring.c b/source3/modules/vfs_io_uring.c
index ee23449c63c6..df41c74a7953 100644
--- a/source3/modules/vfs_io_uring.c
+++ b/source3/modules/vfs_io_uring.c
@@ -34,6 +34,8 @@ struct vfs_io_uring_request;
 struct vfs_io_uring_config {
 	struct io_uring uring;
 	struct tevent_fd *fde;
+	bool busy;
+	bool need_retry;
 	struct vfs_io_uring_request *queue;
 	struct vfs_io_uring_request *pending;
 };
@@ -222,7 +224,7 @@ static int vfs_io_uring_connect(vfs_handle_struct *handle, const char *service,
 	return 0;
 }
 
-static void vfs_io_uring_queue_run(struct vfs_io_uring_config *config)
+static void _vfs_io_uring_queue_run(struct vfs_io_uring_config *config)
 {
 	struct vfs_io_uring_request *cur = NULL, *next = NULL;
 	struct io_uring_cqe *cqe = NULL;
@@ -280,6 +282,22 @@ static void vfs_io_uring_queue_run(struct vfs_io_uring_config *config)
 	io_uring_cq_advance(&config->uring, nr);
 }
 
+static void vfs_io_uring_queue_run(struct vfs_io_uring_config *config)
+{
+	if (config->busy) {
+		config->need_retry = true;
+		return;
+	}
+	config->busy = true;
+
+	do {
+		config->need_retry = false;
+		_vfs_io_uring_queue_run(config);
+	} while (config->need_retry);
+
+	config->busy = false;
+}
+
 static void vfs_io_uring_fd_handler(struct tevent_context *ev,
 				    struct tevent_fd *fde,
 				    uint16_t flags,
-- 
2.17.1

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20200508/213771ca/signature.sig>