[pve-devel] applied: [PATCH cluster] cfs-func-plug: use RW lock for save cached data access

[Wolfgang Bumiller]

applied with commit message fixups
(save -> safe and 1504 instead of 1505)

On Thu, Sep 21, 2017 at 02:08:00PM +0200, Thomas Lamprecht wrote:
> fuse may spawn multiple threads if there are concurrent accesses.
> 
> Our virtual files, e.g. ".members", ".rrd", are registered over our
> "func" cfs plug which is a bit special.
> 
> For each unique virtual file there exists a single cfs_plug_func_t
> instance, shared between all threads.
> As we directly operated unlocked on members of this structure
> parallel accesses raced between each other.
> This could result in quite visible problems like a crash after a
> double free (Bug 1504) or in less noticeable effects where one thread
> may read from an inconsistent, or already freed memory region.
> 
> Add a Reader/Writer lock to efficiently address this problem.
> Other plugs implement more functions and use a mutex to ensure
> consistency and thus do not have this problem.
> 
> Fixes: #1505
> Signed-off-by: Thomas Lamprecht <t.lamprecht at proxmox.com>
> ---
> 
> As this can lead to crashes I suggest master and stable-4 as apply
> targets
> 
>  data/src/cfs-plug-func.c | 12 ++++++++++--
>  data/src/cfs-plug.h      |  1 +
>  2 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/data/src/cfs-plug-func.c b/data/src/cfs-plug-func.c
> index 1982390..c369688 100644
> --- a/data/src/cfs-plug-func.c
> +++ b/data/src/cfs-plug-func.c
> @@ -79,18 +79,22 @@ cfs_plug_func_getattr(
>  
>  	cfs_plug_func_t *fplug = (cfs_plug_func_t *)plug;
>  
>  	memset(stbuf, 0, sizeof(struct stat));
>  
> +	g_rw_lock_writer_lock(&fplug->data_rw_lock);
>  	if (fplug->data)
>  		g_free(fplug->data);
>  
>  	fplug->data = fplug->update_callback(plug);
>  
> +	stbuf->st_size = fplug->data ? strlen(fplug->data) : 0;
> +
> +	g_rw_lock_writer_unlock(&fplug->data_rw_lock);
> +
>  	stbuf->st_mode = fplug->mode;
>  	stbuf->st_nlink = 1;
> -	stbuf->st_size = fplug->data ? strlen(fplug->data) : 0;
>  
>  	return 0;
>  }
>  
>  static int 
> @@ -109,26 +113,30 @@ cfs_plug_func_read(
>  	g_return_val_if_fail(path != NULL, PARAM_CHECK_ERRNO);
>  	g_return_val_if_fail(buf != NULL, PARAM_CHECK_ERRNO);
>  
>  	cfs_plug_func_t *fplug = (cfs_plug_func_t *)plug;
>  
> +	g_rw_lock_reader_lock(&fplug->data_rw_lock);
>  	char *data = fplug->data;
>  
>  	cfs_debug("enter cfs_plug_func_read %s", data);
>  
> -	if (!data)
> +	if (!data) {
> +		g_rw_lock_reader_unlock(&fplug->data_rw_lock);
>  		return 0;
> +	}
>  
>  	int len = strlen(data);
>  
>  	if (offset < len) {
>  		if (offset + size > len)
>  			size = len - offset;
>  		memcpy(buf, data + offset, size);
>  	} else {
>  		size = 0;
>  	}
> +	g_rw_lock_reader_unlock(&fplug->data_rw_lock);
>  
>  	return size;
>  }
>  
>  static int 
> diff --git a/data/src/cfs-plug.h b/data/src/cfs-plug.h
> index 1aef6ce..ac12ad6 100644
> --- a/data/src/cfs-plug.h
> +++ b/data/src/cfs-plug.h
> @@ -83,10 +83,11 @@ typedef int (*cfs_plug_func_write_data_fn_t)(
>  	size_t size);
>  
>  typedef struct {
>  	cfs_plug_t plug;
>  	char *data;
> +	GRWLock data_rw_lock;
>  	mode_t mode;
>  	cfs_plug_func_udpate_data_fn_t update_callback;
>  	cfs_plug_func_write_data_fn_t write_callback;
>  } cfs_plug_func_t;
>  
> -- 
> 2.11.0