Re: [PATCH] ntfs: trim the sizes after a short write

[liubaolin]

在 2026/11/11 17:34, Zhan Xusheng 写道:
> From: Zhan Xusheng <zhanxusheng@xxxxxxxxxx>
>
> 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 two
> sizes back to the end of what was written instead of to the old sizes.
>
> Fixes: b041ca562526 ("ntfs: update iomap and address space operations")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Zhan Xusheng <zhanxusheng@xxxxxxxxxx>
> ---
> A reproducer is available on request.
>
> Tested in a VM on mainline b3dfbde62cf1, with the free space filled
> with a known pattern first.  Short append of 4M that stops after
> 1056768 bytes:
>
>                       write()   size grows by   old data readable
>    before             1056768   4194304         3137536 bytes
>    after              1056768   1056768         0
>
> A short write 1M past EOF gives the same results.  Full writes and
> compressed files are unchanged.
> ---
>   fs/ntfs/file.c | 25 ++++++++++++++++++-------
>   1 file changed, 18 insertions(+), 7 deletions(-)
>
> diff --git a/fs/ntfs/file.c b/fs/ntfs/file.c
> index 3213a0c9f96b..98bb3d5d420e 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
> @@ -940,13 +951,13 @@ ssize_t ntfs_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
>   		filemap_invalidate_lock(vi->i_mapping);
>   
>   		/* ntfs_attr_set_initialized_size() rejects a resident one. */
> -		if (ni->initialized_size != old_init_size &&
> +		if (ni->initialized_size != new_init_size &&
>   		    NInoNonResident(ni)) {
>   			struct ntfs_inode *mrec_ni = ntfs_base_inode(ni);
>   
>   			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 +966,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) {

Hi Xusheng,
   Do we also need to clear the cached bytes beyond new_init_size when 
rolling back initialized_size, independently of whether data_size changes?

   Consider a short buffered write into allocated but uninitialized 
space within the existing EOF:
old_init_size < pos + ret < end <= old_data_size
   Here new_data_size equals ni->data_size, so truncate_setsize() is 
skipped. During a short-copy retry ending inside a block, 
__iomap_write_begin() can read the whole mapped block and mark it 
uptodate, including bytes beyond pos + ret. At that point, 
ntfs_iomap_put_folio_non_resident() still sees the temporarily extended 
initialized_size, so those trailing bytes can remain uncleared.

   ntfs_attr_set_initialized_size() does not clear the page cache. A 
subsequent buffered read can therefore return those cached bytes without 
going through the read path that zeros uninitialized data.

   Could we also clear the newly uninitialized cached range under the 
invalidate lock, while preserving the successfully written prefix?

Thanks,
Baolin.

>   			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