Unverified Commit 2c8f4742 authored by David Howells's avatar David Howells Committed by Christian Brauner
Browse files

netfs: Fix potential for tearing in ->remote_i_size and ->zero_point

Fix potential tearing in using ->remote_i_size and ->zero_point by copying
i_size_read() and i_size_write() and using the same seqcount as for i_size.

We need to make sure that netfslib and the filesystems that use it always
hold i_lock whilst updating any of the sizes to prevent i_size_seqcount
from getting corrupted.

Fixes: 4058f742 ("netfs: Keep track of the actual remote file size")
Fixes: 100ccd18 ("netfs: Optimise away reads above the point at which there can be no data")
Closes: https://sashiko.dev/#/patchset/20260414082004.3756080-1-dhowells%40redhat.com


Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
Link: https://patch.msgid.link/20260512123404.719402-6-dhowells@redhat.com


cc: Paulo Alcantara <pc@manguebit.org>
cc: Matthew Wilcox <willy@infradead.org>
cc: netfs@lists.linux.dev
cc: linux-fsdevel@vger.kernel.org
Signed-off-by: default avatarChristian Brauner <brauner@kernel.org>
parent 8a8c0cfd
Loading
Loading
Loading
Loading
+0 −13
Original line number Diff line number Diff line
@@ -75,17 +75,4 @@ static inline void v9fs_invalidate_inode_attr(struct inode *inode)

int v9fs_open_to_dotl_flags(int flags);

static inline void v9fs_i_size_write(struct inode *inode, loff_t i_size)
{
	/*
	 * 32-bit need the lock, concurrent updates could break the
	 * sequences and make i_size_read() loop forever.
	 * 64-bit updates are atomic and can skip the locking.
	 */
	if (sizeof(i_size) > sizeof(long))
		spin_lock(&inode->i_lock);
	i_size_write(inode, i_size);
	if (sizeof(i_size) > sizeof(long))
		spin_unlock(&inode->i_lock);
}
#endif
+4 −2
Original line number Diff line number Diff line
@@ -1141,11 +1141,13 @@ v9fs_stat2inode(struct p9_wstat *stat, struct inode *inode,
	mode |= inode->i_mode & ~S_IALLUGO;
	inode->i_mode = mode;

	v9inode->netfs.remote_i_size = stat->length;
	spin_lock(&inode->i_lock);
	netfs_write_remote_i_size(inode, stat->length);
	if (!(flags & V9FS_STAT2INODE_KEEP_ISIZE))
		v9fs_i_size_write(inode, stat->length);
		i_size_write(inode, stat->length);
	/* not real number of blocks, but 512 byte ones ... */
	inode->i_blocks = (stat->length + 512 - 1) >> 9;
	spin_unlock(&inode->i_lock);
	v9inode->cache_validity &= ~V9FS_INO_INVALID_ATTR;
}

+8 −4
Original line number Diff line number Diff line
@@ -634,10 +634,12 @@ v9fs_stat2inode_dotl(struct p9_stat_dotl *stat, struct inode *inode,
		mode |= inode->i_mode & ~S_IALLUGO;
		inode->i_mode = mode;

		v9inode->netfs.remote_i_size = stat->st_size;
		spin_lock(&inode->i_lock);
		netfs_write_remote_i_size(inode, stat->st_size);
		if (!(flags & V9FS_STAT2INODE_KEEP_ISIZE))
			v9fs_i_size_write(inode, stat->st_size);
			i_size_write(inode, stat->st_size);
		inode->i_blocks = stat->st_blocks;
		spin_unlock(&inode->i_lock);
	} else {
		if (stat->st_result_mask & P9_STATS_ATIME) {
			inode_set_atime(inode, stat->st_atime_sec,
@@ -662,13 +664,15 @@ v9fs_stat2inode_dotl(struct p9_stat_dotl *stat, struct inode *inode,
			mode |= inode->i_mode & ~S_IALLUGO;
			inode->i_mode = mode;
		}
		spin_lock(&inode->i_lock);
		if (!(flags & V9FS_STAT2INODE_KEEP_ISIZE) &&
		    stat->st_result_mask & P9_STATS_SIZE) {
			v9inode->netfs.remote_i_size = stat->st_size;
			v9fs_i_size_write(inode, stat->st_size);
			netfs_write_remote_i_size(inode, stat->st_size);
			i_size_write(inode, stat->st_size);
		}
		if (stat->st_result_mask & P9_STATS_BLOCKS)
			inode->i_blocks = stat->st_blocks;
		spin_unlock(&inode->i_lock);
	}
	if (stat->st_result_mask & P9_STATS_GEN)
		inode->i_generation = stat->st_gen;
+19 −5
Original line number Diff line number Diff line
@@ -427,21 +427,35 @@ static void afs_free_request(struct netfs_io_request *rreq)
	afs_put_wb_key(rreq->netfs_priv2);
}

static void afs_update_i_size(struct inode *inode, loff_t new_i_size)
/*
 * Set the file size and block count, taking ->cb_lock and ->i_lock to maintain
 * coherency and prevent 64-bit tearing on 32-bit arches.
 *
 * Also, estimate the number of 512 bytes blocks used, rounded up to nearest 1K
 * for consistency with other AFS clients.
 */
void afs_set_i_size(struct afs_vnode *vnode, loff_t new_i_size)
{
	struct afs_vnode *vnode = AFS_FS_I(inode);
	struct inode *inode = &vnode->netfs.inode;
	loff_t i_size;

	write_seqlock(&vnode->cb_lock);
	i_size = i_size_read(&vnode->netfs.inode);
	spin_lock(&inode->i_lock);
	i_size = i_size_read(inode);
	if (new_i_size > i_size) {
		i_size_write(&vnode->netfs.inode, new_i_size);
		inode_set_bytes(&vnode->netfs.inode, new_i_size);
		i_size_write(inode, new_i_size);
		inode_set_bytes(inode, round_up(new_i_size, 1024));
	}
	spin_unlock(&inode->i_lock);
	write_sequnlock(&vnode->cb_lock);
	fscache_update_cookie(afs_vnode_cache(vnode), NULL, &new_i_size);
}

static void afs_update_i_size(struct inode *inode, loff_t new_i_size)
{
	afs_set_i_size(AFS_FS_I(inode), new_i_size);
}

static void afs_netfs_invalidate_cache(struct netfs_io_request *wreq)
{
	struct afs_vnode *vnode = AFS_FS_I(wreq->inode);
+22 −9
Original line number Diff line number Diff line
@@ -224,7 +224,8 @@ static int afs_inode_init_from_status(struct afs_operation *op,
		return afs_protocol_error(NULL, afs_eproto_file_type);
	}

	afs_set_i_size(vnode, status->size);
	i_size_write(inode, status->size);
	inode_set_bytes(inode, status->size);
	afs_set_netfs_context(vnode);

	vnode->invalid_before	= status->data_version;
@@ -253,7 +254,8 @@ static void afs_apply_status(struct afs_operation *op,
{
	struct afs_file_status *status = &vp->scb.status;
	struct afs_vnode *vnode = vp->vnode;
	struct inode *inode = &vnode->netfs.inode;
	struct netfs_inode *ictx = &vnode->netfs;
	struct inode *inode = &ictx->inode;
	struct timespec64 t;
	umode_t mode;
	bool unexpected_jump = false;
@@ -336,6 +338,8 @@ static void afs_apply_status(struct afs_operation *op,
	}

	if (data_changed) {
		unsigned long long zero_point, size = status->size;

		inode_set_iversion_raw(inode, status->data_version);

		/* Only update the size if the data version jumped.  If the
@@ -343,16 +347,25 @@ static void afs_apply_status(struct afs_operation *op,
		 * idea of what the size should be that's not the same as
		 * what's on the server.
		 */
		vnode->netfs.remote_i_size = status->size;
		if (change_size || status->size > i_size_read(inode)) {
			afs_set_i_size(vnode, status->size);
		spin_lock(&inode->i_lock);

		if (change_size || size > i_size_read(inode)) {
			/* We can read the sizes directly as we hold i_lock. */
			zero_point = ictx->_zero_point;

			if (unexpected_jump)
				vnode->netfs.zero_point = status->size;
				zero_point = size;
			netfs_write_sizes(inode, size, size, zero_point);
			inode_set_bytes(inode, size);
			inode_set_ctime_to_ts(inode, t);
			inode_set_atime_to_ts(inode, t);
		} else {
			netfs_write_remote_i_size(inode, size);
		}
		spin_unlock(&inode->i_lock);

		if (op->ops == &afs_fetch_data_operation)
			op->fetch.subreq->rreq->i_size = status->size;
			op->fetch.subreq->rreq->i_size = size;
	}
}

@@ -709,7 +722,7 @@ int afs_getattr(struct mnt_idmap *idmap, const struct path *path,
		 * it, but we need to give userspace the server's size.
		 */
		if (S_ISDIR(inode->i_mode))
			stat->size = vnode->netfs.remote_i_size;
			stat->size = netfs_read_remote_i_size(inode);
	} while (read_seqretry(&vnode->cb_lock, seq));

	return 0;
@@ -889,7 +902,7 @@ int afs_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
		 */
		if (!(attr->ia_valid & (supported & ~ATTR_SIZE & ~ATTR_MTIME)) &&
		    attr->ia_size < i_size &&
		    attr->ia_size > vnode->netfs.remote_i_size) {
		    attr->ia_size > netfs_read_remote_i_size(inode)) {
			truncate_setsize(inode, attr->ia_size);
			netfs_resize_file(&vnode->netfs, size, false);
			fscache_resize_cookie(afs_vnode_cache(vnode),
Loading