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

[Jeremy Allison]

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 ?


> From a1819f8b03ca43c860bdff7b2bdde79674fe7cc4 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.
> 
> 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 | 21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/source3/modules/vfs_io_uring.c b/source3/modules/vfs_io_uring.c
> index a7308ff9edd3..1340d3a5ca9e 100644
> --- a/source3/modules/vfs_io_uring.c
> +++ b/source3/modules/vfs_io_uring.c
> @@ -35,6 +35,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;
>  	bool nowait_pread;
> @@ -253,13 +255,22 @@ static void vfs_io_uring_queue_run(struct vfs_io_uring_config *config)
>  	struct timespec end_time;
>  	int ret;
>  
> -	PROFILE_TIMESTAMP(&start_time);
> -
>  	if (config->uring.ring_fd == -1) {
>  		vfs_io_uring_config_destroy(config, -ESTALE, __location__);
>  		return;
>  	}
>  
> +	if (config->busy) {
> +		config->need_retry = true;
> +		return;
> +	}
> +	config->busy = true;
> +
> +start_again:
> +	config->need_retry = false;
> +
> +	PROFILE_TIMESTAMP(&start_time);
> +
>  	for (cur = config->queue; cur != NULL; cur = next) {
>  		struct io_uring_sqe *sqe = NULL;
>  		void *state = _tevent_req_data(cur->req);
> @@ -299,6 +310,12 @@ static void vfs_io_uring_queue_run(struct vfs_io_uring_config *config)
>  	}
>  
>  	io_uring_cq_advance(&config->uring, nr);
> +
> +	if (config->need_retry) {
> +		goto start_again;
> +	}
> +
> +	config->busy = false;
>  }
>  
>  static void vfs_io_uring_request_submit(struct vfs_io_uring_request *cur)
> -- 
> 2.17.1
>