linux-kernel - [RFC PATCH v5 8/8] nfsd: take inode_lock when querying for NFSv4 GETATTR

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-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu,  8 Sep 2022 13:24:48 -0400
From:   Jeff Layton <jlayton@...nel.org>
To:     tytso@....edu, adilger.kernel@...ger.ca, djwong@...nel.org,
        david@...morbit.com, trondmy@...merspace.com, neilb@...e.de,
        viro@...iv.linux.org.uk, zohar@...ux.ibm.com, xiubli@...hat.com,
        chuck.lever@...cle.com, lczerner@...hat.com, jack@...e.cz,
        bfields@...ldses.org, brauner@...nel.org, fweimer@...hat.com
Cc:     linux-btrfs@...r.kernel.org, linux-fsdevel@...r.kernel.org,
        linux-kernel@...r.kernel.org, ceph-devel@...r.kernel.org,
        linux-ext4@...r.kernel.org, linux-nfs@...r.kernel.org,
        linux-xfs@...r.kernel.org
Subject: [RFC PATCH v5 8/8] nfsd: take inode_lock when querying for NFSv4 GETATTR

The i_version counter for regular files is updated in update_time, and
that's usually done before copying the data to the pagecache. It's
possible that a reader and writer could race like this:

	reader		writer
	------		------
			i_version++
	read
	getattr
			update page cache

If that happens then the reader may associate the i_version value with
the wrong inode state.

All of the existing filesystems that implement i_version take the
i_rwsem in their write_iter operations before incrementing it. Take the
inode_lock when issuing a getattr for NFSv4 attributes to prevent the
above race.

Signed-off-by: Jeff Layton <jlayton@...nel.org>
---
 fs/nfsd/nfs4xdr.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 4eec2ce05e7e..f7951d8d55ca 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -2872,9 +2872,22 @@ nfsd4_encode_fattr(struct xdr_stream *xdr, struct svc_fh *fhp,
 			goto out;
 	}
 
+	/*
+	 * The inode lock is needed here to ensure that there is not a
+	 * write to the inode in progress that might change the size,
+	 * or an in-progress directory morphing operation for directory
+	 * inodes.
+	 *
+	 * READ and GETATTR are not guaranteed to be atomic, even when in
+	 * the same compound, but we do try to present attributes in the
+	 * GETATTR reply as representing a single point in time.
+	 */
+	inode_lock(d_inode(dentry));
 	err = vfs_getattr(&path, &stat,
 			  STATX_BASIC_STATS | STATX_BTIME | STATX_INO_VERSION,
 			  AT_STATX_SYNC_AS_STAT);
+	inode_unlock(d_inode(dentry));
+
 	if (err)
 		goto out_nfserr;
 	if (!(stat.result_mask & STATX_BTIME))
-- 
2.37.3