From: Paulo Alcantara <pc@manguebit.org> mainline inclusion from mainline-v7.3-rc6 commit c9b1194e8ec99d95db25db45edd2fbf690f043ab category: bugfix bugzilla: https://atomgit.com/src-openeuler/kernel/issues/20580 Reference: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=... -------------------------------- cifs_file_copychunk_range() and the clone (FICLONE) path of cifs_remap_file_range() do not serialise the destination page cache against the server-side copy the way the other server-side range operations (smb3_zero_range(), smb3_punch_hole(), smb3_collapse_range() and smb3_insert_range()) do. cifs_file_copychunk_range() invalidates the destination range with filemap_invalidate_inode(), which takes and drops the mapping's invalidate_lock internally, so the lock is no longer held when the copychunk ioctl is issued. It also never drains in-flight netfs I/O on the target. The clone path never takes the invalidate_lock at all (only i_rwsem), uses a bare truncate_inode_pages_range() and likewise does not drain outstanding I/O. As a result an asynchronous destination writeback can complete after the server-side copy/clone has run and reinstate stale data over the region just written by the server, corrupting the file. This is the same class of corruption as commit d7d2adcd022b ("smb/client: flush dirty data before punching a hole") and has been seen randomly in generic/363 against Windows Server. Fix both paths to follow the established ordering: hold the target mapping's invalidate_lock across the flush and invalidation of the destination and the server ioctl, and call netfs_wait_for_outstanding_io() on the target to drain in-flight writes before the ioctl is issued. Only the target inode's invalidate_lock is required, as the source is merely flushed and not invalidated; i_rwsem (already held via lock_two_nondirectories()) is acquired before the invalidate_lock, matching the VFS lock ordering. Since filemap_invalidate_inode() takes that same lock internally, replace it with its own unmap/flush/invalidate steps instead of calling it, and return early on a zero-length copy to avoid a range underflow. Fixes: 8101d6e112e2 ("cifs: Fix copy offload to flush destination region") Fixes: c54fc3a4f375 ("cifs: Fix flushing, invalidation and file size with FICLONE") Reviewed-by: David Howells <dhowells@redhat.com> Reviewed-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Paulo Alcantara <pc@manguebit.org> Cc: Christian Brauner <brauner@kernel.org> Cc: Matthew Wilcox <willy@infradead.org> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com> Cc: Shyam Prasad N <sprasad@microsoft.com> Cc: Tom Talpey <tom@talpey.com> Cc: Bharath SM <bharathsm@microsoft.com> Cc: stable@vger.kernel.org Conflicts: fs/smb/client/cifsfs.c [This tree has no netfs_wait_for_outstanding_io() and 8101d6e112e2 is not merged yet. So no need to change cifs_file_copychunk_range] Signed-off-by: Yifan Qiao <qiaoyifan4@huawei.com> --- fs/smb/client/cifsfs.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/fs/smb/client/cifsfs.c b/fs/smb/client/cifsfs.c index fb2692789cd6..334efdd8d8d9 100644 --- a/fs/smb/client/cifsfs.c +++ b/fs/smb/client/cifsfs.c @@ -1338,6 +1338,7 @@ static loff_t cifs_remap_file_range(struct file *src_file, loff_t off, * server could even support copy of range where source = target */ lock_two_nondirectories(target_inode, src_inode); + filemap_invalidate_lock(target_inode->i_mapping); if (len == 0) { loff_t src_size = i_size_read(src_inode); @@ -1387,8 +1388,19 @@ static loff_t cifs_remap_file_range(struct file *src_file, loff_t off, if (rc) goto unlock; - /* Discard all the folios that overlap the destination region. */ + /* Discard all the folios that overlap the destination region. Start at + * the old EOF when extending so the folio straddling it, which may hold + * data written past EOF through an mmap, is dropped too. + */ cifs_dbg(FYI, "about to discard pages %llx-%llx\n", fstart, fend); + fstart = min_t(unsigned long long, fstart, + i_size_read(target_inode)); + /* Drain any in-flight writeback on the destination before the server + * side clone rewrites the range. + */ + rc = filemap_write_and_wait_range(target_inode->i_mapping, fstart, fend); + if (rc) + goto unlock; truncate_inode_pages_range(&target_inode->i_data, fstart, fend); fscache_invalidate(cifs_inode_cookie(target_inode), NULL, @@ -1424,6 +1436,7 @@ static loff_t cifs_remap_file_range(struct file *src_file, loff_t off, that target is updated on the server */ CIFS_I(target_inode)->time = 0; unlock: + filemap_invalidate_unlock(target_inode->i_mapping); /* although unlocking in the reverse order from locking is not strictly necessary here it is a little cleaner to be consistent */ unlock_two_nondirectories(src_inode, target_inode); -- 2.52.0