linux-kernel - [PATCH] fuse: annotate potential data-race in num

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240509125716.1268016-1-leitao@debian.org>
Date: Thu,  9 May 2024 05:57:15 -0700
From: Breno Leitao <leitao@...ian.org>
To: Miklos Szeredi <miklos@...redi.hu>
Cc: paulmck@...nel.org,
	linux-fsdevel@...r.kernel.org (open list:FUSE: FILESYSTEM IN USERSPACE),
	linux-kernel@...r.kernel.org (open list)
Subject: [PATCH] fuse: annotate potential data-race in num_background

A data race occurs when two concurrent data paths potentially access
fuse_conn->num_background simultaneously.

Specifically, fuse_request_end() accesses and modifies ->num_background
while holding the bg_lock, whereas fuse_readahead() reads
->num_background without acquiring any lock beforehand. This potential
data race is flagged by KCSAN:

	BUG: KCSAN: data-race in fuse_readahead [fuse] / fuse_request_end [fuse]

	read-write to 0xffff8883a6666598 of 4 bytes by task 113809 on cpu 39:
	fuse_request_end (fs/fuse/dev.c:318) fuse
	fuse_dev_do_write (fs/fuse/dev.c:?) fuse
	fuse_dev_write (fs/fuse/dev.c:?) fuse
	...

	read to 0xffff8883a6666598 of 4 bytes by task 113787 on cpu 8:
	fuse_readahead (fs/fuse/file.c:1005) fuse
	read_pages (mm/readahead.c:166)
	page_cache_ra_unbounded (mm/readahead.c:?)
	...

	value changed: 0x00000001 -> 0x00000000

Annotated the reader with READ_ONCE() and the writer with WRITE_ONCE()
to avoid such complaint from KCSAN.

Suggested-by: Miklos Szeredi <miklos@...redi.hu>
Signed-off-by: Breno Leitao <leitao@...ian.org>
---
 fs/fuse/dev.c  | 6 ++++--
 fs/fuse/file.c | 2 +-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
index 3ec8bb5e68ff..8e63dba49eff 100644
--- a/fs/fuse/dev.c
+++ b/fs/fuse/dev.c
@@ -282,6 +282,7 @@ void fuse_request_end(struct fuse_req *req)
 	struct fuse_mount *fm = req->fm;
 	struct fuse_conn *fc = fm->fc;
 	struct fuse_iqueue *fiq = &fc->iq;
+	unsigned int num_background;
 
 	if (test_and_set_bit(FR_FINISHED, &req->flags))
 		goto put_request;
@@ -301,7 +302,8 @@ void fuse_request_end(struct fuse_req *req)
 	if (test_bit(FR_BACKGROUND, &req->flags)) {
 		spin_lock(&fc->bg_lock);
 		clear_bit(FR_BACKGROUND, &req->flags);
-		if (fc->num_background == fc->max_background) {
+		num_background = READ_ONCE(fc->num_background);
+		if (num_background == fc->max_background) {
 			fc->blocked = 0;
 			wake_up(&fc->blocked_waitq);
 		} else if (!fc->blocked) {
@@ -315,7 +317,7 @@ void fuse_request_end(struct fuse_req *req)
 				wake_up(&fc->blocked_waitq);
 		}
 
-		fc->num_background--;
+		WRITE_ONCE(fc->num_background, num_background - 1);
 		fc->active_background--;
 		flush_bg_queue(fc);
 		spin_unlock(&fc->bg_lock);
diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index b57ce4157640..07331889bbf3 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -1002,7 +1002,7 @@ static void fuse_readahead(struct readahead_control *rac)
 		struct fuse_io_args *ia;
 		struct fuse_args_pages *ap;
 
-		if (fc->num_background >= fc->congestion_threshold &&
+		if (READ_ONCE(fc->num_background) >= fc->congestion_threshold &&
 		    rac->ra->async_size >= readahead_count(rac))
 			/*
 			 * Congested and only async pages left, so skip the
-- 
2.43.0