From: Zhan Xusheng ntfs_file_write_iter() moves the data size and, for a non-resident uncompressed file, the initialized size to pos + count before the copy. __ntfs_extend_initialized_size() only zeroes [old initialized size, pos), so the range [pos, pos + count) is initialized only by the copy itself. When iomap_file_buffered_write() copies less than count, it returns a positive short count and the error rollback at out: does not run. The file keeps the size pos + count, and [pos + ret, pos + count) reads back whatever the freed clusters held before, also after a remount. A local user can read the old contents of freed clusters this way. Run the existing rollback for a short write as well, and roll the data size back to the end of what was written. For a buffered write, keep the initialized size and zero what the copy did not reach, up to the end of the block holding pos + count, because a page fault through a shared mapping may already rely on that initialized size. A direct write rolls the initialized size back as before. Fixes: b041ca562526 ("ntfs: update iomap and address space operations") Cc: stable@vger.kernel.org Signed-off-by: Zhan Xusheng --- v4: for a buffered write, zero the unwritten range instead of truncating the page cache and taking the initialized size back, which dropped data stored through a shared mapping during the copy (Baolin). v3: clear the cache up to EOF, not up to the extended initialized size, which left the rest of the block holding pos + count (Baolin). v2: also clear the cached tail of the last block, which v1 left in place when the write stayed inside the old data size (Baolin). v3: https://lore.kernel.org/all/20261112093233.1118469-1-zhanxusheng1024@gmail.com/ v2: https://lore.kernel.org/all/20261112030815.982026-1-zhanxusheng1024@gmail.com/ v1: https://lore.kernel.org/all/20261111093459.660484-1-zhanxusheng1024@gmail.com/ Tested in a VM on ntfs-next b025f526c5d0, with all free space filled with a known pattern first. Short buffered writes of 4M, old data readable after drop_caches and after remount, 3 runs each: unfixed v3 v4 fallocated, ends block-aligned 3137536 0 0 fallocated, ends inside a block 3141332 0 0 fallocated, unaligned start 3137536 0 0 same, pos + count in that page - 0 0 append 3137536 0 0 With the sequence from the v3 review (marker stored through a shared mapping at 64K while pwritev() at 0 is stalled, short count 8192), the marker is kept on unfixed and v4 and lost on v3, 3 runs each. A store into [pos + ret, pos + count) itself during the copy is zeroed with v4, as that range belongs to the write. fsx (buffered, no-mmap, O_DIRECT and mixed), compressed files, fallocate, insert/collapse range and punch hole give the same results with v4 as with v3. --- fs/ntfs/file.c | 57 +++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 50 insertions(+), 7 deletions(-) diff --git a/fs/ntfs/file.c b/fs/ntfs/file.c index 3213a0c9f96b..5ac11d60be37 100644 --- a/fs/ntfs/file.c +++ b/fs/ntfs/file.c @@ -928,10 +928,21 @@ ssize_t ntfs_file_write_iter(struct kiocb *iocb, struct iov_iter *from) ret = iomap_file_buffered_write(iocb, from, &ntfs_write_iomap_ops, &ntfs_iomap_folio_ops, NULL); out: - if (ret < 0 && ret != -EIOCBQUEUED) { + /* + * Both sizes were moved to end before the copy, so a short write, + * like a failed one, must take back what it did not write. + */ + if (ret < count && ret != -EIOCBQUEUED) { + loff_t new_init_size = old_init_size; + loff_t new_data_size = old_data_size; int init_err = 0; int data_err = 0; + if (ret > 0) { + new_init_size = max(old_init_size, pos + ret); + new_data_size = max(old_data_size, pos + ret); + } + /* * Like ntfs_setattr_size(): __ntfs_attr_truncate_vfs() * expects the invalidate lock held across the size @@ -939,14 +950,46 @@ ssize_t ntfs_file_write_iter(struct kiocb *iocb, struct iov_iter *from) */ filemap_invalidate_lock(vi->i_mapping); + /* + * A page fault through a shared mapping may have used the + * extended initialized size while a buffered write was copying, + * so taking it back could drop that data. Zero what the write + * did not reach instead, including the rest of the block at end + * that the copy may have read from disk, unless such a fault + * already moved the initialized size past end and zeroed it. + */ + if (!(iocb->ki_flags & IOCB_DIRECT) && NInoNonResident(ni) && + !NInoCompressed(ni) && ni->initialized_size > new_init_size) { + loff_t zend = ni->initialized_size > end ? end : + round_up(end, i_blocksize(vi)); + + zend = min(zend, new_data_size); + if (zend > new_init_size) + init_err = iomap_zero_range(vi, new_init_size, + zend - new_init_size, NULL, + &ntfs_seek_iomap_ops, + &ntfs_iomap_folio_ops, NULL); + new_init_size = min(ni->initialized_size, new_data_size); + } + /* ntfs_attr_set_initialized_size() rejects a resident one. */ - if (ni->initialized_size != old_init_size && + if (!init_err && ni->initialized_size != new_init_size && NInoNonResident(ni)) { struct ntfs_inode *mrec_ni = ntfs_base_inode(ni); + /* + * A direct write did not use the page cache; drop what + * a concurrent reader may have cached past the new + * initialized size, as __ntfs_extend_initialized_size() + * does. + */ + if (iocb->ki_flags & IOCB_DIRECT) + truncate_pagecache_range(vi, new_init_size, + max_t(loff_t, i_size_read(vi), + ni->initialized_size) - 1); mutex_lock(&mrec_ni->mrec_lock); init_err = ntfs_attr_set_initialized_size(ni, - old_init_size); + new_init_size); mutex_unlock(&mrec_ni->mrec_lock); } /* @@ -955,13 +998,13 @@ ssize_t ntfs_file_write_iter(struct kiocb *iocb, struct iov_iter *from) * the uninitialized tail, so skip it unless the rollback * above restored that size. */ - if (ni->data_size != old_data_size && !init_err) { + if (ni->data_size != new_data_size && !init_err) { struct ntfs_inode *mrec_ni = ntfs_base_inode(ni); - truncate_setsize(vi, old_data_size); + truncate_setsize(vi, new_data_size); mutex_lock(&mrec_ni->mrec_lock); - data_err = __ntfs_attr_truncate_vfs(ni, old_data_size, - old_data_size); + data_err = __ntfs_attr_truncate_vfs(ni, new_data_size, + new_data_size); mutex_unlock(&mrec_ni->mrec_lock); /* Like ntfs_setattr_size(): stay on the durable size. */ if (data_err) base-commit: b025f526c5d04929212c9d5fb7ffc96608f3eacc -- 2.43.0