[PATCH] erofs-utils: lib/diskbuf: fix MT data race in erofs_diskbuf_reserve()

Utkal Singh <[email protected]>
Newsgroups org.ozlabs.lists.linux-erofs
Message-ID <[email protected]>
Two threads calling erofs_diskbuf_reserve() concurrently can both
observe strm->locked == false and both advance tailoffset, silently
producing an incorrect offset in the output image. Add a
pthread_mutex_t to serialize access.

The commit path also gains the missing strm->locked = false reset;
without it, locked was set once and never cleared, making the flag
a latch rather than a guard.

libpthread is already a build requirement (-lpthread in
lib/Makefile.am) and pthread_mutex_t is used identically in
lib/workqueue.c and lib/compress.c, so no build system changes
are required.

Fixes: 13f7268 ("erofs-utils: lib: introduce multi-threaded I/O framework")
Signed-off-by: Utkal Singh <[email protected]>
---
 lib/diskbuf.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/lib/diskbuf.c b/lib/diskbuf.c
index 0bf42da..ccfeaa4 100644
--- a/lib/diskbuf.c
+++ b/lib/diskbuf.c
@@ -2,6 +2,7 @@
 #include "erofs/diskbuf.h"
 #include "erofs/internal.h"
 #include "erofs/print.h"
+#include <pthread.h>
 #include <stdio.h>
 #include <errno.h>
 #include <sys/stat.h>
@@ -15,6 +16,7 @@ static struct erofs_diskbufstrm {
 	int fd;
 	unsigned int alignsize;
 	bool locked;
+	pthread_mutex_t lock;
 } *dbufstrm;
 
 int erofs_diskbuf_getfd(struct erofs_diskbuf *db, u64 *fpos)
@@ -34,15 +36,20 @@ int erofs_diskbuf_reserve(struct erofs_diskbuf *db, int sid, u64 *off)
 {
 	struct erofs_diskbufstrm *strm = dbufstrm + sid;
 
-	if (strm->tailoffset & (strm->alignsize - 1)) {
-		strm->tailoffset = round_up(strm->tailoffset, strm->alignsize);
+	pthread_mutex_lock(&strm->lock);
+	if (strm->locked) {
+		pthread_mutex_unlock(&strm->lock);
+		return -EBUSY;
 	}
+	if (strm->tailoffset & (strm->alignsize - 1))
+		strm->tailoffset = round_up(strm->tailoffset, strm->alignsize);
 	db->offset = strm->tailoffset;
 	if (off)
 		*off = db->offset + strm->devpos;
 	db->sp = strm;
 	(void)erofs_atomic_inc_return(&strm->count);
-	strm->locked = true;	/* TODO: need a real lock for MT */
+	strm->locked = true;
+	pthread_mutex_unlock(&strm->lock);
 	return strm->fd;
 }
 
@@ -51,9 +58,12 @@ void erofs_diskbuf_commit(struct erofs_diskbuf *db, u64 len)
 	struct erofs_diskbufstrm *strm = db->sp;
 
 	DBG_BUGON(!strm);
+	pthread_mutex_lock(&strm->lock);
 	DBG_BUGON(!strm->locked);
 	DBG_BUGON(strm->tailoffset != db->offset);
 	strm->tailoffset += len;
+	strm->locked = false;
+	pthread_mutex_unlock(&strm->lock);
 }
 
 void erofs_diskbuf_close(struct erofs_diskbuf *db)
@@ -115,6 +125,7 @@ int erofs_diskbuf_init(unsigned int nstrms)
 setupone:
 		strm->tailoffset = 0;
 		erofs_atomic_set(&strm->count, 1);
+		pthread_mutex_init(&strm->lock, NULL);
 		if (fstat(strm->fd, &st))
 			return -errno;
 		strm->alignsize = max_t(u32, st.st_blksize, getpagesize());
@@ -132,6 +143,7 @@ void erofs_diskbuf_exit(void)
 	for (strm = dbufstrm; strm->fd >= 0; ++strm) {
 		DBG_BUGON(erofs_atomic_read(&strm->count) != 1);
 
+		pthread_mutex_destroy(&strm->lock);
 		close(strm->fd);
 		strm->fd = -1;
 	}
-- 
2.43.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.