[PATCH] vfs_default: Fix passing of errno from async calls

[Jeremy Allison]

On Wed, Aug 23, 2017 at 03:12:26PM -0700, Christof Schmitt via samba-technical wrote:
> I found this while testing with a system where the actual write() system
> call returned an error. This led to Samba hanging and never continuing
> with the result of the write. The attached patch fixes the problem. From
> what i understand, errno was never passed correctly back to the callers.

Wow - good catch !!!!

Can we change the assignment:

 -     state->err = errno;
 +     state->vfs_aio_state.error = errno;

to:

 -     state->err = errno;
 +	if (state->ret == -1) {
 +	     state->vfs_aio_state.error = errno;
 +	}

on each of the hunks, as we need to make
sure that the thread only assigns non-zero
to state->vfs_aio_state.error in the fail
case. It's (theoretically) possible that
the first go around returns -1, errno = EINTR and
the second go around returns 0, but
doesn't set errno (leaving it as the
previous EINTR value). We should only
set state->vfs_aio_state.error on a
*known* error return IMHO.

If you're happy with that RB+.

Jeremy.

> From 3b9133f42f7f11c655b54d5657722dbed953fa15 Mon Sep 17 00:00:00 2001
> From: Christof Schmitt <cs at samba.org>
> Date: Wed, 23 Aug 2017 14:37:28 -0700
> Subject: [PATCH] vfs_default: Fix passing of errno from async calls
> 
> Current code assigns errno from async pthreadpool calls to the
> vfs_default internal vfswrap_*_state.  The callers of the vfs_*_recv
> functions expect the value from errno in vfs_aio_state.error.
> 
> Correctly assign errno to vfs_aio_state.error and remove the unused
> internal err variable.
> 
> Signed-off-by: Christof Schmitt <cs at samba.org>
> ---
>  source3/modules/vfs_default.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/source3/modules/vfs_default.c b/source3/modules/vfs_default.c
> index fbfe654..458e878 100644
> --- a/source3/modules/vfs_default.c
> +++ b/source3/modules/vfs_default.c
> @@ -738,7 +738,6 @@ static int vfswrap_init_pool(struct smbd_server_connection *conn)
>  
>  struct vfswrap_pread_state {
>  	ssize_t ret;
> -	int err;
>  	int fd;
>  	void *buf;
>  	size_t count;
> @@ -812,7 +811,7 @@ static void vfs_pread_do(void *private_data)
>  				   state->offset);
>  	} while ((state->ret == -1) && (errno == EINTR));
>  
> -	state->err = errno;
> +	state->vfs_aio_state.error = errno;
>  
>  	PROFILE_TIMESTAMP(&end_time);
>  
> @@ -861,7 +860,6 @@ static ssize_t vfswrap_pread_recv(struct tevent_req *req,
>  
>  struct vfswrap_pwrite_state {
>  	ssize_t ret;
> -	int err;
>  	int fd;
>  	const void *buf;
>  	size_t count;
> @@ -935,7 +933,7 @@ static void vfs_pwrite_do(void *private_data)
>  				   state->offset);
>  	} while ((state->ret == -1) && (errno == EINTR));
>  
> -	state->err = errno;
> +	state->vfs_aio_state.error = errno;
>  
>  	PROFILE_TIMESTAMP(&end_time);
>  
> @@ -984,7 +982,6 @@ static ssize_t vfswrap_pwrite_recv(struct tevent_req *req,
>  
>  struct vfswrap_fsync_state {
>  	ssize_t ret;
> -	int err;
>  	int fd;
>  
>  	struct vfs_aio_state vfs_aio_state;
> @@ -1045,7 +1042,7 @@ static void vfs_fsync_do(void *private_data)
>  		state->ret = fsync(state->fd);
>  	} while ((state->ret == -1) && (errno == EINTR));
>  
> -	state->err = errno;
> +	state->vfs_aio_state.error = errno;
>  
>  	PROFILE_TIMESTAMP(&end_time);
>  
> -- 
> 1.8.3.1
>