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

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

[Utkal Singh]

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 <singhutkal015@gmail.com>
---
 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

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

[Gao Xiang]

On 2026/4/2 13:04, Utkal Singh wrote:
> Two threads calling erofs_diskbuf_reserve() concurrently can both
> observe strm->locked == false and both advance tailoffset, silently

How?

> 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 <singhutkal015@gmail.com>
> ---
>   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;

Do you ever notice erofs_mutex_t and include/erofs/lock.h?

By the way, I sincerely hope you pay less attention
of this project since I've told you the reasons in
emails before.

It won't help with your gsoc proposal.  Thanks for
your efforts and contributions to the EROFS project.

Thanks,
Gao Xiang