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

[Dr. David Alan Gilbert]

* Stefan Hajnoczi (address@hidden) 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 <address@hidden>

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 <address@hidden>


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 / address@hidden / Manchester, UK