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

[Thomas Lamprecht]

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