Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Jeff Layton <jlayton@kernel.org>
To:     viro@zeniv.linux.org.uk
Cc:     linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
        sargun@sargun.me, amir73il@gmail.com, vgoyal@redhat.com
Subject: [PATCH][RESEND] vfs: serialize updates to file->f_sb_err with f_lock
Date:   Mon,  4 Jan 2021 13:43:47 -0500
Message-Id: <20210104184347.90598-1-jlayton@kernel.org>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

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@kernel.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