Re: [Qemu-devel] [PATCH 3/5] virtiofsd: make lo_release() atomic

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Stefan Hajnoczi (stefanha@redhat.com) wrote:
> Hold the lock across both lo_map_get() and lo_map_remove() to prevent
> races between two FUSE_RELEASE requests.  In this case I don't see a
> serious bug but it's safer to do things atomically.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

OK, although I suspect there are lots of places the inode pointers 
are passed without the lock, so it might not help much.


Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>


and applied

> ---
>  contrib/virtiofsd/passthrough_ll.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/contrib/virtiofsd/passthrough_ll.c b/contrib/virtiofsd/passthrough_ll.c
> index 277a17fc03..c1500e092d 100644
> --- a/contrib/virtiofsd/passthrough_ll.c
> +++ b/contrib/virtiofsd/passthrough_ll.c
> @@ -1759,14 +1759,18 @@ static void lo_open(fuse_req_t req, fuse_ino_t ino, struct fuse_file_info *fi)
>  static void lo_release(fuse_req_t req, fuse_ino_t ino, struct fuse_file_info *fi)
>  {
>  	struct lo_data *lo = lo_data(req);
> -	int fd;
> +	struct lo_map_elem *elem;
> +	int fd = -1;
>  
>  	(void) ino;
>  
> -	fd = lo_fi_fd(req, fi);
> -
>  	pthread_mutex_lock(&lo->mutex);
> -	lo_map_remove(&lo->fd_map, fi->fh);
> +	elem = lo_map_get(&lo->fd_map, fi->fh);
> +	if (elem) {
> +		fd = elem->fd;
> +		elem = NULL;
> +		lo_map_remove(&lo->fd_map, fi->fh);
> +	}
>  	pthread_mutex_unlock(&lo->mutex);
>  
>  	close(fd);
> -- 
> 2.21.0
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK