netfs, cifs: data corruption fixes
Subsystems (5)
Paulo Alcantara
maintainercover netfs, cifs: data corruption fixes
This series fixes a number of data corruption and spurious I/O error bugs found by running generic/363 (fsx) in a loop against Windows Server 2022 and Samba 4.24 servers.
* Post-EOF pagecache poisoning on extend (1-5)
Data dirtied past EOF through an mmapped region was never discarded, so it reappeared as file content once the file was extended by an ordinary write, a truncate, a zero range, a copy range or a clone range. pagecache_isize_extended() doesn't help here: it's a no-op on cifs because i_blkbits is 14. Only the one folio straddling the old EOF can hold such data, since pages wholly beyond EOF can't be faulted in, so each extending path now zeroes or discards that folio.
* Missing flush or drain before trusting server or pagecache state (6-9)
FSCTL_QUERY_ALLOCATED_RANGES can report just-written data as a hole unless it has been committed to disk first, and the O_TRUNC open, interior zero range and server-side copy/clone paths did not flush dirty data or drain in-flight I/O before an operation that assumes the pagecache and the server agree on the file's contents.
* fallocate refused without a read lease (10, 12)
smb3_zero_range() and smb3_simple_falloc() returned -EOPNOTSUPP for any size-extending request whenever the inode wasn't read caching, since the cached i_size couldn't be trusted. Query the server's EOF in that case instead of refusing the request outright.
* Short reads leaving stale data behind (11, 13, 14)
The read-gaps path and the DIO/unbuffered read collector left the untransferred tail of a short read untouched, and cifs could not tell a genuine EOF from a stale cached remote_i_size after a lease downgrade. A read racing an extending write could come back short and, in the read-gaps case, have that stale folio content written back to the server.
* Unstable pages during a signed write (15)
cifs signs the pagecache folios in place before handing them to the socket. A buffered write could modify a folio while a write subrequest built from it was still in flight, so the signature no longer matched the data that followed it; the server answered STATUS_ACCESS_DENIED, which surfaced later as -EIO.
Paulo Alcantara (15):
netfs: clear post-EOF pagecache when extending a file via write
smb: client: clear post-EOF pagecache when extending a file via
truncate
smb: client: discard post-EOF pagecache when extending a file via zero
range
smb: client: discard post-EOF pagecache when extending a file via copy
range
smb: client: discard post-EOF pagecache when extending a file via
clone range
smb: client: flush and commit data before querying allocated ranges
smb: client: drain outstanding I/O before truncating on O_TRUNC open
smb: client: flush dirty data before zeroing a range
smb: client: drain and invalidate before server-side copy/clone
smb: client: only require read lease for size-extending zero range
netfs: zero gaps in read-gaps folio to avoid writing back stale data
smb: client: only require read lease for size-extending preallocate
netfs: zero the tail of a short DIO/unbuffered read
smb: client: distinguish real EOF from a stale remote_i_size on read
smb: client: require stable pages for signed connections
Documentation/filesystems/netfs_library.rst | 25 ++++++ fs/netfs/buffered_read.c | 7 ++ fs/netfs/buffered_write.c | 9 ++ fs/netfs/direct_write.c | 9 ++ fs/netfs/misc.c | 71 +++++++++++++++ fs/netfs/read_collect.c | 24 +++++ fs/smb/client/cifsfs.c | 25 +++++- fs/smb/client/file.c | 2 + fs/smb/client/inode.c | 19 ++-- fs/smb/client/smb2ops.c | 97 ++++++++++++++++----- fs/smb/client/smb2pdu.c | 10 ++- include/linux/netfs.h | 2 + 12 files changed, 267 insertions(+), 33 deletions(-)
Signature · 1 line
2.55.0
Paulo Alcantara
maintainerPATCH 1/15 netfs: clear post-EOF pagecache when extending a file via write
Fix netfs to erase the contents of a hole created after the EOF by an ordinary write if dirty data has been previously left there by writes through an mmapped region. Neither the buffered nor the unbuffered/DIO write path clears that stale pagecache.
Zero the tail of the folio straddling the EOF before an extending write. That is the only folio that can hold data written past the EOF through an mmap, as pages wholly beyond the EOF can't be faulted in. The folio is zeroed rather than dropped so a concurrent extending write can't lose data.
Both write paths downgrade the i_rwsem to shared, so extending writes can run concurrently and the i_size read by the caller may be stale by the time the folio is locked. Re-read i_size under the folio lock and clamp the zeroed range up to it, so a racing write that already put data into the folio isn't clobbered.
Wait for any writeback on the folio to finish before zeroing it so that the pagecache isn't modified while it may still be read by the transport during transmission. Honour IOCB_NOWAIT by returning -EAGAIN rather than blocking on the folio lock, on writeback, or in folio_mkclean()'s rmap walk when the folio is mapped.
truncate_pagecache() can't be used here: it must be called with the i_rwsem held exclusively, but these write paths only hold it shared, and it would block unconditionally, breaking IOCB_NOWAIT.
Export the helper so filesystems can clear the same stale data from their own truncate paths.
This helper is required by the CIFS client to fix generic/363.
--- Documentation/filesystems/netfs_library.rst | 25 ++++++++ fs/netfs/buffered_write.c | 9 +++ fs/netfs/direct_write.c | 9 +++ fs/netfs/misc.c | 71 +++++++++++++++++++++ include/linux/netfs.h | 2 + 5 files changed, 116 insertions(+)
Documentation/filesystems/netfs_library.rst +25 -0
@@ -451,6 +451,31 @@ one. The inode should be marked ``NETFS_ICTX_SINGLE_NO_UPLOAD`` if this API is to be used. The writeback function requires the buffer to be of ITER_FOLIOQ type. +Clearing Stale Post-EOF Pagecache +--------------------------------- + +When a file is extended, data left in the pagecache past the old EOF by a write +through an mmap must not be exposed as file content. Netfslib clears this on +its own write paths, and exports a helper so a filesystem can do the same from, +for instance, its truncate or fallocate paths:: + + int netfs_clear_stale_post_eof(struct inode *inode, uoff_t from, + uoff_t to, bool nowait); + +This zeroes any such data within the ``[from, to)`` hole to be made, where @from +is the current EOF. Only the folio straddling @from can hold data written past +the EOF through an mmap, as pages wholly beyond the EOF can't be faulted in, so +the zeroing is limited to that folio and clamped to the top of the hole. The +folio is zeroed rather than dropped so that a concurrent extending write can't +lose data. If @nowait is set, it returns ``-EAGAIN`` rather than blocking. + +This overlaps with ``pagecache_isize_extended()`` but can't reuse it: that +helper is keyed on a sub-page block size and is a no-op when the block size is +``>= PAGE_SIZE`` (as on network filesystems), it runs after ``i_size`` has been +updated, it can't honour a non-blocking caller, and it doesn't wait for +writeback. As both address the same problem, a change to one should probably be +reflected in the other to keep them in sync. + High-Level VM API ==================
fs/netfs/buffered_write.c +9 -0
@@ -469,6 +469,7 @@ ssize_t netfs_buffered_write_iter_locked(struct kiocb *iocb, struct iov_iter *fr struct netfs_group *netfs_group) {struct file *file = iocb->ki_filp; +struct inode *inode =file_inode (file);ssize_t ret;trace_netfs_write_iter (iocb, from);
@@ -481,6 +482,14 @@ ssize_t netfs_buffered_write_iter_locked(struct kiocb *iocb, struct iov_iter *frif (ret)return ret; +if (iocb->ki_pos >i_size_read (inode)) { + ret =netfs_clear_stale_post_eof (inode,i_size_read (inode), + iocb->ki_pos, + iocb->ki_flags &IOCB_NOWAIT ); +if (ret) +return ret; + } +return netfs_perform_write (iocb, from, netfs_group); }EXPORT_SYMBOL (netfs_buffered_write_iter_locked);
fs/netfs/direct_write.c +9 -0
@@ -359,6 +359,15 @@ ssize_t netfs_unbuffered_write_iter(struct kiocb *iocb, struct iov_iter *from) ret =file_update_time (file);if (ret <0 ) goto out; + +if (iocb->ki_pos >i_size_read (inode)) { + ret =netfs_clear_stale_post_eof (inode,i_size_read (inode), + iocb->ki_pos, + iocb->ki_flags &IOCB_NOWAIT ); +if (ret <0 ) + goto out; + } +if (iocb->ki_flags &IOCB_NOWAIT ) {/* We could block if there are any pages in the range. */ ret = -EAGAIN ;
fs/netfs/misc.c +71 -0
80 lines not on this page · raw message
include/linux/netfs.h +2 -0
@@ -397,6 +397,8 @@ ssize_t netfs_unbuffered_write_iter(struct kiocb *iocb, struct iov_iter *from);ssize_t netfs_unbuffered_write_iter_locked (struct kiocb *iocb,struct iov_iter *iter,struct netfs_group *netfs_group);ssize_t netfs_file_write_iter (struct kiocb *iocb,struct iov_iter *from); +int netfs_clear_stale_post_eof (struct inode *inode,uoff_t from, +uoff_t to,bool nowait);/* Single, monolithic object read/write API. */ void netfs_single_mark_inode_dirty (struct inode *inode);
Signature · 1 line
2.55.0
Paulo Alcantara
maintainerPATCH 2/15 smb: client: clear post-EOF pagecache when extending a file via truncate
In reply to Paulo Alcantaracifs_setsize() relied on pagecache_isize_extended() to zero the tail of the folio straddling the old EOF, but that helper is a no-op on CIFS (i_blkbits is 14), so data dirtied past EOF through an mmap survived and became visible once the file was extended.
When extending, zero the tail of the folio straddling the old EOF with netfs_clear_stale_post_eof() instead, mirroring what pagecache_isize_extended() does for filesystems with a sub-page block size. Do this before updating i_size so that the helper, which clamps its zeroing to the current EOF, zeroes the whole [old_size, offset) hole. truncate_pagecache() is then called as in truncate_setsize(): it is essentially a no-op on extend and drops the pagecache beyond the new EOF on shrink.
--- fs/smb/client/inode.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)
fs/smb/client/inode.c +11 -5
@@ -3055,13 +3055,20 @@ int cifs_fiemap(struct inode *inode, struct fiemap_extent_info *fei, u64 start,void cifs_setsize (struct inode *inode,loff_t offset) { -loff_t old_size; +loff_t old_size =i_size_read (inode);u64 blocks =CIFS_INO_BLOCKS (offset); +/* +* When extending, zero the tail of the folio straddling the old EOF +* (before updating i_size) so data dirtied past EOF through an mmap +* isn't exposed. truncate_pagecache() then drops any pagecache beyond +* the new EOF, as in truncate_setsize(). +*/ +if (offset > old_size) +netfs_clear_stale_post_eof (inode, old_size, offset, false); +spin_lock (&inode->i_lock); - old_size =i_size_read (inode);i_size_write (inode, offset); -/* * Extending EOF does not allocate the intervening range. Only clamp * i_blocks on shrink; allocation growth comes from writes or from the
@@ -3071,8 +3078,7 @@ void cifs_setsize(struct inode *inode, loff_t offset) inode->i_blocks = blocks;spin_unlock (&inode->i_lock);inode_set_mtime_to_ts (inode,inode_set_ctime_current (inode)); -if (offset > old_size) -pagecache_isize_extended (inode, old_size, offset); +truncate_pagecache (inode, offset);netfs_wait_for_outstanding_io (inode); }
Signature · 1 line
2.55.0
Paulo Alcantara
maintainerPATCH 3/15 smb: client: discard post-EOF pagecache when extending a file via zero range
In reply to Paulo Alcantarasmb3_zero_range() removed the pagecache only for the range being zeroed, so when it extends the file the folio straddling the old EOF - which may hold data dirtied past EOF through an mmap - was never dropped and became visible as file content once the file was extended.
Lower the discard boundary to the smaller of the zero-range offset and the old EOF.
--- fs/smb/client/smb2ops.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
fs/smb/client/smb2ops.c +4 -1
@@ -3558,8 +3558,11 @@ static long smb3_zero_range(struct file *file, struct cifs_tcon *tcon,/* * We zero the range through ioctl, so we need remove the page caches * first, otherwise the data may be inconsistent with the server. +* +* 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. */ -truncate_pagecache_range (inode, offset, offset + len -1 ); +truncate_pagecache_range (inode,min (offset, i_size), offset + len -1 );netfs_wait_for_outstanding_io (inode);/* if file not oplocked can't be sure whether asking to extend size */
Signature · 1 line
2.55.0
Paulo Alcantara
maintainerPATCH 4/15 smb: client: discard post-EOF pagecache when extending a file via copy range
In reply to Paulo Alcantaracifs_file_copychunk_range() invalidated the pagecache only for the destination region, so when a copy extends the file the folio straddling the old EOF - which may hold data dirtied past EOF through an mmap - was never dropped and became visible as file content once the file was extended.
Start the invalidation at the smaller of the destination offset and the old EOF. filemap_invalidate_inode() writes back before invalidating while i_size is still old, so nothing past the old EOF is flushed to the server.
--- fs/smb/client/cifsfs.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
fs/smb/client/cifsfs.c +6 -1
@@ -1581,8 +1581,13 @@ ssize_t cifs_file_copychunk_range(unsigned int xid,/* Flush and invalidate all the folios in the destination region. If * the copy was successful, then some of the flush is extra overhead, * but we need to allow for the copy failing in some way (eg. ENOSPC). +* +* 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. */ - rc =filemap_invalidate_inode (target_inode, true, destoff, destoff + len -1 ); + rc =filemap_invalidate_inode (target_inode, true, +min (destoff,i_size_read (target_inode)), + destoff + len -1 );if (rc) goto unlock;
Signature · 1 line
2.55.0
Paulo Alcantara
maintainerPATCH 5/15 smb: client: discard post-EOF pagecache when extending a file via clone range
In reply to Paulo Alcantaracifs_remap_file_range() discarded the pagecache only for the folios overlapping the destination region, so when a clone extends the file the folio straddling the old EOF - which may hold data dirtied past EOF through an mmap - was never dropped and became visible as file content once the file was extended.
Start the discard at the smaller of fstart and the old EOF. i_size is still old here, so nothing past the old EOF is flushed to the server.
--- fs/smb/client/cifsfs.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
fs/smb/client/cifsfs.c +7 -2
@@ -1467,9 +1467,14 @@ static loff_t cifs_remap_file_range(struct file *src_file, loff_t off, i_size = target_inode->i_size;spin_unlock (&target_inode->i_lock); -/* 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); -truncate_inode_pages_range (&target_inode->i_data, fstart, fend); +truncate_inode_pages_range (&target_inode->i_data, +min (fstart, i_size), fend);fscache_invalidate (cifs_inode_cookie (target_inode),NULL , i_size,0 );
Signature · 1 line
2.55.0
Paulo Alcantara
maintainerPATCH 6/15 smb: client: flush and commit data before querying allocated ranges
In reply to Paulo AlcantaraThe mode 0 / FALLOC_FL_KEEP_SIZE fallocate emulation for small interior ranges in smb3_simple_fallocate_range() issues FSCTL_QUERY_ALLOCATED_RANGES and zero-fills any sub-range the server reports as an unallocated hole, to force block allocation without changing file contents.
This is unsafe against Windows servers. Per the Win32 documentation, FSCTL_QUERY_ALLOCATED_RANGES only reports ranges that *may* contain nonzero data and is explicitly not coherent with recently written data:
"a call to FSCTL_QUERY_ALLOCATED_RANGES [after writing to a network
file] would not necessarily return a correct list of allocated regions. To ensure coherency [...] flush the data to the file"
fsx (generic/363) reproduces the resulting corruption against Windows Server 2022: a range is written, an uncached read returns the data, yet the next FSCTL_QUERY_ALLOCATED_RANGES on that range reports it as a whole hole, so the emulation overwrites live data with zeroes. A network trace confirmed the WRITE, the READ returning data and the query returning an empty range list within microseconds. Samba does not exhibit this; its query is coherent with the written data.
Write back the dirty pages, drain any outstanding I/O and issue an SMB2 FLUSH to force the server to commit the data before querying the allocated ranges, so the query reflects the data actually present.
With this, generic/363 passes against both Windows Server 2022 and Samba over 100000 fsx operations.
--- fs/smb/client/smb2ops.c | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-)
fs/smb/client/smb2ops.c +26 -6
@@ -3748,6 +3748,24 @@ static int smb3_simple_fallocate_range(unsigned int xid, goto out; } +filemap_invalidate_lock (inode->i_mapping); + +/* +* Flush and commit the data to the server, otherwise +* FSCTL_QUERY_ALLOCATED_RANGES might report recently written data as +* unallocated holes on Windows Servers, and the loop below would +* then zero-fill them and corrupt the file. +*/ + rc =filemap_write_and_wait_range (inode->i_mapping, off, + off + len -1 ); +if (rc) + goto out_unlock; +netfs_wait_for_outstanding_io (inode); + rc =SMB2_flush (xid, tcon, cfile->fid.persistent_fid, + cfile->fid.volatile_fid); +if (rc) + goto out_unlock; + in_data.file_offset =cpu_to_le64 (off); in_data.length =cpu_to_le64 (len); rc = SMB2_ioctl(xid, tcon, cfile->fid.persistent_fid,
@@ -3757,7 +3775,7 @@ static int smb3_simple_fallocate_range(unsigned int xid,1024 *sizeof (struct file_allocated_range_buffer ), (char **)&out_data, &out_data_len);if (rc) - goto out; + goto out_unlock; tmp_data = out_data;while (len) {
@@ -3767,12 +3785,12 @@ static int smb3_simple_fallocate_range(unsigned int xid,if (out_data_len ==0 ) { rc =smb3_simple_fallocate_write_range (xid, tcon, cfile, off, len, buf); - goto out; + goto out_unlock; }if (out_data_len <sizeof (struct file_allocated_range_buffer )) { rc = -EINVAL ; - goto out; + goto out_unlock; } range_start =le64_to_cpu (tmp_data->file_offset);
@@ -3780,7 +3798,7 @@ static int smb3_simple_fallocate_range(unsigned int xid,if (check_add_overflow (range_start, range_len, &range_end) || range_end >S64_MAX ) { rc = -EINVAL ; - goto out; + goto out_unlock; }if (off < range_start) {
@@ -3795,11 +3813,11 @@ static int smb3_simple_fallocate_range(unsigned int xid, rc =smb3_simple_fallocate_write_range (xid, tcon, cfile, off, l, buf);if (rc) - goto out; + goto out_unlock; off = off + l; len = len - l;if (len ==0 ) - goto out; + goto out_unlock; }/* * We are at a section of allocated data, just skip forward
@@ -3818,6 +3836,8 @@ static int smb3_simple_fallocate_range(unsigned int xid, out_data_len -=sizeof (struct file_allocated_range_buffer ); } + out_unlock: +filemap_invalidate_unlock (inode->i_mapping); out:kfree (out_data);kvfree (buf);
Signature · 1 line
2.55.0
Paulo Alcantara
maintainerPATCH 7/15 smb: client: drain outstanding I/O before truncating on O_TRUNC open
In reply to Paulo Alcantaracifs_do_truncate() flushes the pagecache with filemap_write_and_wait() before resetting the file size to zero, but that does not wait for direct/async I/O that was already in flight, which can complete after the truncate and reinstate stale data past the new EOF.
Drain outstanding netfs I/O with netfs_wait_for_outstanding_io() before resizing the file.
--- fs/smb/client/file.c | 2 ++ 1 file changed, 2 insertions(+)
fs/smb/client/file.c +2 -0
@@ -1012,6 +1012,8 @@ static int cifs_do_truncate(const unsigned int xid, struct dentry *dentry) }mapping_set_error (inode->i_mapping, rc); +netfs_wait_for_outstanding_io (inode); + cfile =find_writable_file (cinode,FIND_FSUID_ONLY ); rc =cifs_file_flush (xid, inode, cfile);if (!rc) {
Signature · 1 line
2.55.0
Paulo Alcantara
maintainerPATCH 8/15 smb: client: flush dirty data before zeroing a range
In reply to Paulo Alcantarasmb3_zero_range() emulates FALLOC_FL_ZERO_RANGE by invalidating the page cache over the target range with truncate_pagecache_range() and then issuing FSCTL_SET_ZERO_DATA to the server.
Dirty data was only flushed conditionally, when the range reached or extended EOF. For a purely interior zero range that does not reach EOF, no flush happened, so a dirty folio overlapping the range could be written back to the server after the FSCTL and refill the range that was just zeroed with stale data.
Fix this by unconditionally flushing and waiting for dirty data in the range before invalidating the page cache and issuing FSCTL_SET_ZERO_DATA, exactly as was done for smb3_punch_hole() in commit d7d2adcd022b ("smb/client: flush dirty data before punching a hole"); the two paths are structurally identical here.
This is observed as generic/363 randomly reading stale data where a zeroed range is expected against Windows Server.
--- fs/smb/client/smb2ops.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)
fs/smb/client/smb2ops.c +4 -6
@@ -3547,13 +3547,11 @@ static long smb3_zero_range(struct file *file, struct cifs_tcon *tcon,filemap_invalidate_lock (inode->i_mapping);netfs_read_sizes (inode, &i_size, &remote_i_size, &zero_point); -if (offset + len >= remote_i_size && offset < i_size) { -unsigned long long top =umin (offset + len, i_size); - rc =filemap_write_and_wait_range (inode->i_mapping, offset, top -1 ); -if (rc <0 ) - goto zero_range_exit; - } + rc =filemap_write_and_wait_range (inode->i_mapping, offset, + offset + len -1 ); +if (rc <0 ) + goto zero_range_exit;/* * We zero the range through ioctl, so we need remove the page caches
Signature · 1 line
2.55.0
Paulo Alcantara
maintainerPATCH 9/15 smb: client: drain and invalidate before server-side copy/clone
In reply to Paulo Alcantaracifs_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.
--- fs/smb/client/cifsfs.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)
fs/smb/client/cifsfs.c +12 -3
@@ -1412,6 +1412,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);
@@ -1475,6 +1476,7 @@ static loff_t cifs_remap_file_range(struct file *src_file, loff_t off,cifs_dbg (FYI ,"about to discard pages %llx-%llx\n" , fstart, fend);truncate_inode_pages_range (&target_inode->i_data,min (fstart, i_size), fend); +netfs_wait_for_outstanding_io (target_inode);fscache_invalidate (cifs_inode_cookie (target_inode),NULL , i_size,0 );
@@ -1513,6 +1515,7 @@ static loff_t cifs_remap_file_range(struct file *src_file, loff_t off,if (rc)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);
@@ -1565,6 +1568,7 @@ ssize_t cifs_file_copychunk_range(unsigned int xid,* server could even support copy of range where source = target */ lock_two_nondirectories (target_inode, src_inode); +filemap_invalidate_lock (target_inode->i_mapping);cifs_dbg (FYI ,"about to flush pages\n" );
@@ -1590,11 +1594,15 @@ ssize_t cifs_file_copychunk_range(unsigned int xid,* 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. */ - rc =filemap_invalidate_inode (target_inode, true, -min (destoff,i_size_read (target_inode)), - destoff + len -1 ); + rc =filemap_write_and_wait_range (target_inode->i_mapping, +min (destoff,i_size_read (target_inode)), + destoff + len -1 );if (rc) goto unlock; +invalidate_inode_pages2_range (target_inode->i_mapping, +min (destoff,i_size_read (target_inode)) >>PAGE_SHIFT , + (destoff + len -1 ) >>PAGE_SHIFT ); +netfs_wait_for_outstanding_io (target_inode);fscache_invalidate (cifs_inode_cookie (target_inode),NULL ,i_size_read (target_inode),0 );
@@ -1626,6 +1634,7 @@ ssize_t cifs_file_copychunk_range(unsigned int xid,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 */
Signature · 1 line
2.55.0
Paulo Alcantara
maintainerPATCH 10/15 smb: client: only require read lease for size-extending zero range
In reply to Paulo Alcantarasmb3_zero_range() refuses any FALLOC_FL_ZERO_RANGE that clears FALLOC_FL_KEEP_SIZE with -EOPNOTSUPP whenever the inode is not read caching:
/* if file not oplocked can't be sure whether asking to extend size */ rc = -EOPNOTSUPP; if (keep_size == false && !CIFS_CACHE_READ(cifsi)) goto zero_range_exit;
The read lease is only needed to trust the cached i_size when deciding whether the range extends the file. When it is not held, the size can instead be fetched from the server, which is authoritative, rather than refusing the request outright: query the server's end of file and take the larger of it and the cached size for the interior-vs-extend decision. The larger of the two is used because the server's end of file reflects another client's growth while the cached size reflects this client's own writes that may not have reached the server yet; using the server size alone would wrongly shrink the file when the range flush above left an extending write unwritten.
Rejecting interior ranges is observed as generic/363 randomly failing against Windows Server with
do_zero_range: fallocate: Operation not supported
fsx issues an interior, non-KEEP_SIZE zero range while the inode is transiently not read caching: the server had just downgraded the file's lease from RWH to RH after breaking the write caching, and the ensuing handle reopen/revalidation left CIFS_CACHE_READ momentarily clear. The range sat well within the server's end of file, so no extend was needed, yet the range was refused and fsx aborted. This keeps the emulation correct even when a genuine lease break from another client leaves the inode without read caching -- the case the -EOPNOTSUPP guard turned into a hard failure. The extra round trip only happens on the no-lease path; the common cached case is unchanged.
--- fs/smb/client/smb2ops.c | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-)
fs/smb/client/smb2ops.c +21 -5
@@ -3520,6 +3520,20 @@ static long smb3_zero_data(struct file *file, struct cifs_tcon *tcon, 0, NULL, NULL); } +static long query_server_eof (const unsigned int xid,struct cifs_tcon *tcon, +struct cifsFileInfo *cfile, +unsigned long long *eof) +{ +struct smb2_file_all_info file_inf = {}; +long rc; + + rc =SMB2_query_info (xid, tcon, cfile->fid.persistent_fid, + cfile->fid.volatile_fid, &file_inf); +if (!rc) + *eof =le64_to_cpu (file_inf.EndOfFile); +return rc; +} +static long smb3_zero_range (struct file *file,struct cifs_tcon *tcon,unsigned long long offset,unsigned long long len,bool keep_size)
@@ -3563,10 +3577,12 @@ static long smb3_zero_range(struct file *file, struct cifs_tcon *tcon,truncate_pagecache_range (inode,min (offset, i_size), offset + len -1 );netfs_wait_for_outstanding_io (inode); -/* if file not oplocked can't be sure whether asking to extend size */ - rc = -EOPNOTSUPP ; -if (keep_size == false && !CIFS_CACHE_READ (cifsi)) - goto zero_range_exit; +if (!keep_size && !CIFS_CACHE_READ (cifsi)) { + rc =query_server_eof (xid, tcon, cfile, &remote_i_size); +if (rc) + goto zero_range_exit; + i_size =max (i_size, remote_i_size); + }fscache_invalidate (cifs_inode_cookie (inode),NULL ,i_size_read (inode),0 );
@@ -3578,7 +3594,7 @@ static long smb3_zero_range(struct file *file, struct cifs_tcon *tcon,/* * do we also need to change the size of the file? */ -if (keep_size == false && (unsigned long long )i_size_read (inode) < new_size) { +if (!keep_size && i_size < new_size) { rc =SMB2_set_eof (xid, tcon, cfile->fid.persistent_fid, cfile->fid.volatile_fid, cfile->pid, new_size);if (rc >=0 ) {
Signature · 1 line
2.55.0
Paulo Alcantara
maintainerPATCH 11/15 netfs: zero gaps in read-gaps folio to avoid writing back stale data
In reply to Paulo Alcantaranetfs_read_gaps() leaves gaps around a streaming write's dirty region unread on the server side, so a short/EOF response there left stale folio content that later got written back.
Zero it after the read completes, not before: netfs_wait_for_read() returns rreq->transferred once ret >= 0, so build an iterator over the same bvec array the read used, advance past ret, and zero the rest. The dirty region already maps to sink pages in that array rather than the real folio, so this can't touch it, and only the actual shortfall gets zeroed instead of the whole gap upfront.
--- fs/netfs/buffered_read.c | 7 +++++++ 1 file changed, 7 insertions(+)
fs/netfs/buffered_read.c +7 -0
@@ -541,6 +541,13 @@ static int netfs_read_gaps(struct file *file, struct folio *folio) ret =netfs_wait_for_read (rreq);if (ret >=0 ) { +if (ret < flen) { +struct iov_iter iter; + +iov_iter_bvec (&iter,ITER_DEST , bvec, i, flen); +iov_iter_advance (&iter, ret); +iov_iter_zero (flen - ret, &iter); + }if (group)folio_change_private (folio, group);else
Signature · 1 line
2.55.0
Paulo Alcantara
maintainerPATCH 12/15 smb: client: only require read lease for size-extending preallocate
In reply to Paulo Alcantarasmb3_simple_falloc() refuses any fallocate that clears FALLOC_FL_KEEP_SIZE with -EOPNOTSUPP whenever the inode is not read caching:
/* if file not oplocked can't be sure whether asking to extend size */
if (!CIFS_CACHE_READ(cifsi))
if (!keep_size) {
...
return rc;
}As with smb3_zero_range(), the read lease is only needed to trust the cached size when deciding whether the request extends the file. When it is not held, the size can instead be fetched from the server, which is authoritative, rather than refusing the request outright: after flushing, query the server's end of file and take the larger of it and the cached size for the interior-vs-extend decision. The larger of the two is used because the server's end of file reflects another client's growth while the cached size reflects this client's own writes that may not have reached the server yet; using the server size alone would wrongly treat an interior request as extending when a range flush left an extending write unwritten.
Rejecting interior requests is observed as generic/363 randomly failing against Windows Server with
do_preallocate: fallocate: Operation not supported
fsx issues an interior, non-KEEP_SIZE preallocate while the inode is transiently not read caching: the server had just downgraded the file's lease from RWH to RH after breaking the write caching, and the ensuing handle reopen/revalidation left CIFS_CACHE_READ momentarily clear. The range sat within the server's end of file, so no extend was needed, yet it was refused and fsx aborted. This keeps the emulation correct even when a genuine lease break from another client leaves the inode without read caching -- the case the -EOPNOTSUPP guard turned into a hard failure. The extra flush and round trip only happen when both keep_size is false and no read lease is held; every other case is unchanged.
--- fs/smb/client/smb2ops.c | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-)
fs/smb/client/smb2ops.c +19 -5
@@ -3880,14 +3880,28 @@ static long smb3_simple_falloc(struct file *file, struct cifs_tcon *tcon,trace_smb3_falloc_enter (xid, cfile->fid.persistent_fid, tcon->tid, tcon->ses->Suid, off, len); -/* if file not oplocked can't be sure whether asking to extend size */ -if (!CIFS_CACHE_READ (cifsi)) -if (!keep_size) { + +if (!keep_size && !CIFS_CACHE_READ (cifsi)) { +unsigned long long server_eof; + + rc =filemap_write_and_wait (inode->i_mapping); +if (rc) {trace_smb3_falloc_err (xid, cfile->fid.persistent_fid, tcon->tid, tcon->ses->Suid, off, len, rc);free_xid (xid);return rc; } +netfs_wait_for_outstanding_io (inode); + + rc =query_server_eof (xid, tcon, cfile, &server_eof); +if (rc) { +trace_smb3_falloc_err (xid, cfile->fid.persistent_fid, + tcon->tid, tcon->ses->Suid, off, len, rc); +free_xid (xid); +return rc; + } + old_eof =max_t (loff_t, old_eof, server_eof); + }/* * Extending the file
@@ -4013,7 +4027,7 @@ static long smb3_simple_falloc(struct file *file, struct cifs_tcon *tcon, } } -if ((keep_size == true) || (i_size_read (inode) >= off + len)) { +if (keep_size || old_eof >= off + len) {/* * At this point, we are trying to fallocate an internal * regions of a sparse file. Since smb2 does not have a
@@ -4042,7 +4056,7 @@ static long smb3_simple_falloc(struct file *file, struct cifs_tcon *tcon,* ie potentially making a few extra pages at the beginning * or end of the file non-sparse via set_sparse is harmless. */ -if ((off >8192 ) || (off + len +8192 <i_size_read (inode))) { +if (off >8192 || off + len +8192 < old_eof) { rc = -EOPNOTSUPP ; goto out; }
Signature · 1 line
2.55.0
Paulo Alcantara
maintainerPATCH 13/15 netfs: zero the tail of a short DIO/unbuffered read
In reply to Paulo AlcantaraThe buffered read collector zero-fills the tail of a short read that stops below the inode's i_size (netfs_clear_unread()), so a read that races an extending write still returns the expected number of bytes. The non-buffered collector path does no such thing: it just records how much was transferred.
Add the same zero-fill for the non-buffered case, gated onNETFS_SREQ_CLEAR_TAIL: a subreq's source sets that flag when a short result from it is known to be safe to treat as a hole, as opposed to NETFS_SREQ_HIT_EOF, which means the read genuinely ran off the end of the file and should be reported short as-is. Only CLEAR_TAIL should zero-fill here; a real EOF must stay a real short read.
No source currently sets CLEAR_TAIL on an unbuffered/DIO subrequest, so this is inert on its own -- a following change teaches cifs to set it in the one case that needs it.
--- fs/netfs/read_collect.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)
fs/netfs/read_collect.c +24 -0
@@ -33,6 +33,22 @@ static void netfs_clear_unread(struct netfs_io_subrequest *subreq)__set_bit (NETFS_SREQ_HIT_EOF , &subreq->flags); } +static void netfs_clear_unread_dio (struct netfs_io_subrequest *subreq) +{ +uoff_t pos = subreq->start + subreq->transferred; +struct netfs_io_request *rreq = subreq->rreq; +size_t fill; + +if (pos >= rreq->i_size) +return ; + + fill =min_t (uoff_t, rreq->i_size - pos, + subreq->len - subreq->transferred); + +netfs_reset_iter (subreq); + subreq->transferred +=iov_iter_zero (fill, &subreq->io_iter); +} +/* * Cancel the copy-to-cache mark on a folio. */
@@ -311,6 +327,14 @@ static void netfs_collect_read_results(struct netfs_io_request *rreq) test_bit(NETFS_SREQ_HIT_EOF, &front->flags))netfs_read_unlock_folios (rreq, ¬es); }else { +if (!(notes &HIT_PENDING ) && + front->error ==0 && + transferred < front->len && +test_bit (NETFS_SREQ_CLEAR_TAIL , &front->flags)) { +netfs_clear_unread_dio (front); + transferred = front->transferred; +trace_netfs_sreq (front, netfs_sreq_trace_clear); + } stream->collected_to = front->start + transferred; rreq->collected_to = stream->collected_to; }
Signature · 1 line
2.55.0
Paulo Alcantara
maintainerPATCH 14/15 smb: client: distinguish real EOF from a stale remote_i_size on read
In reply to Paulo Alcantarasmb2_readv_callback() sets NETFS_SREQ_HIT_EOF whenever a short read lines up with netfs_read_remote_i_size(inode), the server's EOF as the client currently believes it. That belief can be stale: after a lease downgrade and handle reopen, the tracked remote_i_size can sit below the client's own i_size while an extending write hasn't reached the server yet. A read in that gap comes back short for a reason that has nothing to do with the file's real size, but was still marked HIT_EOF, and netfs reports a short read for it as-is.
Only treat it as real EOF when the position is also at or past the client's own i_size; otherwise mark it NETFS_SREQ_CLEAR_TAIL instead, which tells netfs the shortfall is safe to zero-fill rather than report as a short read.
This is what fsx (generic/363) sees as "short read: 0x0 bytes instead of 0x<n>" against a Windows server.
--- fs/smb/client/smb2pdu.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
fs/smb/client/smb2pdu.c +8 -2
@@ -4796,14 +4796,20 @@ smb2_readv_callback(struct TCP_Server_Info *server, struct mid_q_entry *mid) rdata->got_bytes);if (rdata->result == -ENODATA ) { -__set_bit (NETFS_SREQ_HIT_EOF , &rdata->subreq.flags); rdata->result =0 ; +if (rdata->subreq.start + rdata->subreq.transferred >=i_size_read (inode)) +__set_bit (NETFS_SREQ_HIT_EOF , &rdata->subreq.flags); +else +__set_bit (NETFS_SREQ_CLEAR_TAIL , &rdata->subreq.flags); }else {size_t trans = rdata->subreq.transferred + rdata->got_bytes;if (trans < rdata->subreq.len && rdata->subreq.start + trans >=netfs_read_remote_i_size (inode)) { -__set_bit (NETFS_SREQ_HIT_EOF , &rdata->subreq.flags); rdata->result =0 ; +if (rdata->subreq.start + trans >=i_size_read (inode)) +__set_bit (NETFS_SREQ_HIT_EOF , &rdata->subreq.flags); +else +__set_bit (NETFS_SREQ_CLEAR_TAIL , &rdata->subreq.flags); }if (rdata->got_bytes)__set_bit (NETFS_SREQ_MADE_PROGRESS , &rdata->subreq.flags);
Signature · 1 line
2.55.0
Paulo Alcantara
maintainerPATCH 15/15 smb: client: require stable pages for signed connections
In reply to Paulo AlcantaraWhen signing, cifs computes the SMB signature over the pagecache folios in place and then hands those same folios to the socket. If a buffered write mutates a folio while a write subrequest is still in flight, the signature no longer matches the data that follows it, the server rejects the write with STATUS_ACCESS_DENIED (-EACCES), and the error is latched in the mapping, so the next fsync()/fallocate() returns -EIO.
Mark the mapping for stable writes so netfs_perform_write() waits for writeback to complete before modifying an in-flight folio. This is only needed for in-place signing: encryption copies the data into a transform buffer before sending, and plaintext transfers neither sign nor checksum the data, so gate it on the same condition cifs uses to decide whether to sign a request.
--- fs/smb/client/inode.c | 3 +++ 1 file changed, 3 insertions(+)
fs/smb/client/inode.c +3 -0
@@ -88,6 +88,9 @@ static void cifs_set_ops(struct inode *inode)else inode->i_data.a_ops = &cifs_addr_ops;mapping_set_large_folios (inode->i_mapping); +if (tcon->ses->server->sign && + !smb3_encryption_required (tcon)) +mapping_set_stable_writes (inode->i_mapping);break ;case S_IFDIR :if (IS_AUTOMOUNT (inode)) {
Signature · 1 line
2.55.0
Quoted 59 lines · Paulo Alcantara
This series fixes a number of data corruption and spurious I/O error bugs found by running generic/363 (fsx) in a loop against Windows Server 2022 and Samba 4.24 servers.
* Post-EOF pagecache poisoning on extend (1-5)
Data dirtied past EOF through an mmapped region was never discarded, so it reappeared as file content once the file was extended by an ordinary write, a truncate, a zero range, a copy range or a clone range.
pagecache_isize_extended()doesn't help here: it's a no-op on cifs becausei_blkbitsis 14. Only the one folio straddling the old EOF can hold such data, since pages wholly beyond EOF can't be faulted in, so each extending path now zeroes or discards that folio.* Missing flush or drain before trusting server or pagecache state (6-9)
FSCTL_QUERY_ALLOCATED_RANGEScan report just-written data as a hole unless it has been committed to disk first, and theO_TRUNCopen, interior zero range and server-side copy/clone paths did not flush dirty data or drain in-flight I/O before an operation that assumes the pagecache and the server agree on the file's contents.* fallocate refused without a read lease (10, 12)
smb3_zero_range()andsmb3_simple_falloc()returned-EOPNOTSUPPfor any size-extending request whenever the inode wasn't read caching, since the cachedi_sizecouldn't be trusted. Query the server's EOF in that case instead of refusing the request outright.* Short reads leaving stale data behind (11, 13, 14)
The read-gaps path and the DIO/unbuffered read collector left the untransferred tail of a short read untouched, and cifs could not tell a genuine EOF from a stale cached
remote_i_sizeafter a lease downgrade. A read racing an extending write could come back short and, in the read-gaps case, have that stale folio content written back to the server.* Unstable pages during a signed write (15)
cifs signs the pagecache folios in place before handing them to the socket. A buffered write could modify a folio while a write subrequest built from it was still in flight, so the signature no longer matched the data that followed it; the server answered
STATUS_ACCESS_DENIED, which surfaced later as-EIO.Paulo Alcantara (15):
netfs: clear post-EOF pagecache when extending a file via write
smb: client: clear post-EOF pagecache when extending a file via
truncate
smb: client: discard post-EOF pagecache when extending a file via zero
range
smb: client: discard post-EOF pagecache when extending a file via copy
range
smb: client: discard post-EOF pagecache when extending a file via
clone range
smb: client: flush and commit data before querying allocated ranges
smb: client: drain outstanding I/O before truncating onO_TRUNCopen
Quoted 8 lines
smb: client: flush dirty data before zeroing a range
smb: client: drain and invalidate before server-side copy/clone
smb: client: only require read lease for size-extending zero range
netfs: zero gaps in read-gaps folio to avoid writing back stale data
smb: client: only require read lease for size-extending preallocate
netfs: zero the tail of a short DIO/unbuffered read
smb: client: distinguish real EOF from a staleremote_i_sizeon read
smb: client: require stable pages for signed connections
Thanks!