linux-kernel - [PATCH][RESEND] vfs: serialize updates to file->f_sb_err with f

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 for Android: free password hash cracker in your pocket

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

Message-Id: <20210104184347.90598-1-jlayton@kernel.org>
Date:   Mon,  4 Jan 2021 13:43:47 -0500
From:   Jeff Layton <jlayton@...nel.org>
To:     viro@...iv.linux.org.uk
Cc:     linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
        sargun@...gun.me, amir73il@...il.com, vgoyal@...hat.com
Subject: [PATCH][RESEND] vfs: serialize updates to file->f_sb_err with f_lock

When I added the ability for syncfs to report writeback errors, I
neglected to adequately protect file->f_sb_err. While changes to
sb->s_wb_err don't require locking, we do need to protect the errseq_t
cursor in file->f_sb_err.

We could have racing updates to that value if two tasks are issuing
syncfs() on the same fd at the same time, possibly causing an error to
be reported twice or not at all.

Fix this by protecting the f_sb_err field with the file->f_lock.

Fixes: 735e4ae5ba28 ("vfs: track per-sb writeback errors and report them to syncfs")
Signed-off-by: Jeff Layton <jlayton@...nel.org>
---
 fs/sync.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

Al, could you pick up this patch for v5.11 or v5.12? I sent the original
version about a month ago, but it didn't get picked up.

In the original posting I marked this for stable, but I'm not sure it
really qualifies since it's a pretty unlikely race with an oddball
use-case (overlapping syncfs() calls on the same fd).

diff --git a/fs/sync.c b/fs/sync.c
index 1373a610dc78..3be26ff72431 100644
--- a/fs/sync.c
+++ b/fs/sync.c
@@ -162,7 +162,7 @@ SYSCALL_DEFINE1(syncfs, int, fd)
 {
 	struct fd f = fdget(fd);
 	struct super_block *sb;
-	int ret, ret2;
+	int ret, ret2 = 0;

 	if (!f.file)
 		return -EBADF;
@@ -172,7 +172,12 @@ SYSCALL_DEFINE1(syncfs, int, fd)
 	ret = sync_filesystem(sb);
 	up_read(&sb->s_umount);

-	ret2 = errseq_check_and_advance(&sb->s_wb_err, &f.file->f_sb_err);
+	if (errseq_check(&sb->s_wb_err, f.file->f_sb_err)) {
+		/* Something changed, must use slow path */
+		spin_lock(&f.file->f_lock);
+		ret2 = errseq_check_and_advance(&sb->s_wb_err, &f.file->f_sb_err);
+		spin_unlock(&f.file->f_lock);
+	}

 	fdput(f);
 	return ret ? ret : ret2;
-- 
2.29.2