[PATCH 1/2] ntfs: write back the truncated range before dropping it

[PATCH 1/2] ntfs: write back the truncated range before dropping it

[Hongling Zeng]

On a truncate, ntfs_setattr_size() drops the page cache beyond the new
size with truncate_setsize() and only then updates the attribute on
disk.  If that update fails, the size is reverted and the data beyond
the new size becomes visible again -- but the pages were already
dropped, and dirty data in them is lost, even though the truncate, the
operation the user asked for, never happened.

Write the range back before dropping it.  If the writeback fails, fail
the truncate with that error and keep the page cache intact; if the
on-disk truncation fails afterwards, the reverted size can be re-read
from disk.

Fixes: 9c87959601e8 ("ntfs: update file operations")
Cc: stable@vger.kernel.org
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
---
 fs/ntfs/file.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/fs/ntfs/file.c b/fs/ntfs/file.c
index 007d1614b9ac..1417a76788ed 100644
--- a/fs/ntfs/file.c
+++ b/fs/ntfs/file.c
@@ -283,12 +283,27 @@ static int ntfs_setattr_size(struct inode *vi, struct iattr *attr)
 		i_size_write(vi, attr->ia_size);
 		pagecache_isize_extended(vi, old_size, attr->ia_size);
 	} else {
+		/*
+		 * The on-disk truncation below can fail, in which case
+		 * the size is reverted and the data beyond the new size
+		 * becomes visible again.  Write that range back first:
+		 * truncate_setsize() drops those pages and dirty data
+		 * cannot be recovered afterwards.
+		 */
+		if (attr->ia_size < old_size) {
+			err = filemap_write_and_wait_range(vi->i_mapping,
+					attr->ia_size, old_size - 1);
+			if (err)
+				goto out_unlock;
+		}
+
 		truncate_setsize(vi, attr->ia_size);
 	}
 
 	err = ntfs_truncate_vfs(vi, attr->ia_size, old_size);
 	if (err)
 		i_size_write(vi, old_size);
+out_unlock:
 	filemap_invalidate_unlock(vi->i_mapping);
 
 	return err;
-- 
2.25.1

[PATCH 2/2] ntfs: roll back the in-memory attributes if the metadata update fails

[Hongling Zeng]

ntfs_setattr() copies the new attributes into the VFS inode with
setattr_copy() and only then writes them to disk: the POSIX ACL update
and the ntfs_ea_set_wsl_inode() call that stores the uid, gid and mode
in the metadata EAs.  When one of those fails, the error is returned
but the in-memory inode keeps the new, uncommitted attributes, so
memory and disk diverge until the inode is written back or evicted.

Remember the old mode, uid, gid and the FILE_ATTR_READONLY bit and
restore them before returning the error.  This is best effort: the
ctime update is left in place, ntfs_ea_set_wsl_inode() may have
completed some of the EAs before failing, and a failed ACL update may
leave the cached ACL divergent.  The error is still reported.

Fixes: 9c87959601e8 ("ntfs: update file operations")
Cc: stable@vger.kernel.org
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
---
 fs/ntfs/file.c | 28 ++++++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/fs/ntfs/file.c b/fs/ntfs/file.c
index 1417a76788ed..524b33c0e97f 100644
--- a/fs/ntfs/file.c
+++ b/fs/ntfs/file.c
@@ -325,6 +325,10 @@ int ntfs_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
 	unsigned int ia_valid = attr->ia_valid;
 	struct ntfs_inode *ni = NTFS_I(vi);
 	struct ntfs_volume *vol = ni->vol;
+	umode_t old_mode;
+	kuid_t old_uid;
+	kgid_t old_gid;
+	typeof(ni->flags) old_flags;
 
 	if (NVolShutdown(vol))
 		return -EIO;
@@ -351,12 +355,17 @@ int ntfs_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
 		ia_valid |= ATTR_MTIME | ATTR_CTIME;
 	}
 
+	old_mode = vi->i_mode;
+	old_uid = vi->i_uid;
+	old_gid = vi->i_gid;
+	old_flags = ni->flags & FILE_ATTR_READONLY;
+
 	setattr_copy(idmap, vi, attr);
 
 	if (vol->sb->s_flags & SB_POSIXACL && !S_ISLNK(vi->i_mode)) {
 		err = posix_acl_chmod(idmap, dentry, vi->i_mode);
 		if (err)
-			goto out;
+			goto out_restore;
 	}
 
 	if (0222 & vi->i_mode)
@@ -378,11 +387,26 @@ int ntfs_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
 		err = ntfs_ea_set_wsl_inode(vi, 0, NULL, flags);
 		mutex_unlock(&ni->mrec_lock);
 		if (err)
-			goto out;
+			goto out_restore;
 
 	}
 
 	mark_inode_dirty(vi);
+	goto out;
+
+out_restore:
+	/*
+	 * The on-disk metadata update failed: put the in-memory
+	 * attributes back so that the inode does not keep the new,
+	 * uncommitted values.  This is best effort: the ctime update is
+	 * left in place, ntfs_ea_set_wsl_inode() may have completed some
+	 * of the EAs before failing, and a failed ACL update may leave
+	 * the cached ACL divergent.
+	 */
+	vi->i_mode = old_mode;
+	vi->i_uid = old_uid;
+	vi->i_gid = old_gid;
+	ni->flags = (ni->flags & ~FILE_ATTR_READONLY) | old_flags;
 out:
 	return err;
 }
-- 
2.25.1

Re: [PATCH 2/2] ntfs: roll back the in-memory attributes if the metadata update fails

[liubaolin]

在 2026/9/15 14:27, Hongling Zeng 写道:
> ntfs_setattr() copies the new attributes into the VFS inode with
> setattr_copy() and only then writes them to disk: the POSIX ACL update
> and the ntfs_ea_set_wsl_inode() call that stores the uid, gid and mode
> in the metadata EAs.  When one of those fails, the error is returned
> but the in-memory inode keeps the new, uncommitted attributes, so
> memory and disk diverge until the inode is written back or evicted.
> 
> Remember the old mode, uid, gid and the FILE_ATTR_READONLY bit and
> restore them before returning the error.  This is best effort: the
> ctime update is left in place, ntfs_ea_set_wsl_inode() may have
> completed some of the EAs before failing, and a failed ACL update may
> leave the cached ACL divergent.  The error is still reported.
> 
> Fixes: 9c87959601e8 ("ntfs: update file operations")
> Cc: stable@vger.kernel.org
> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
> ---
>   fs/ntfs/file.c | 28 ++++++++++++++++++++++++++--
>   1 file changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/ntfs/file.c b/fs/ntfs/file.c
> index 1417a76788ed..524b33c0e97f 100644
> --- a/fs/ntfs/file.c
> +++ b/fs/ntfs/file.c
> @@ -325,6 +325,10 @@ int ntfs_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
>   	unsigned int ia_valid = attr->ia_valid;
>   	struct ntfs_inode *ni = NTFS_I(vi);
>   	struct ntfs_volume *vol = ni->vol;
> +	umode_t old_mode;
> +	kuid_t old_uid;
> +	kgid_t old_gid;
> +	typeof(ni->flags) old_flags;
>   
>   	if (NVolShutdown(vol))
>   		return -EIO;
> @@ -351,12 +355,17 @@ int ntfs_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
>   		ia_valid |= ATTR_MTIME | ATTR_CTIME;
>   	}
>   
> +	old_mode = vi->i_mode;
> +	old_uid = vi->i_uid;
> +	old_gid = vi->i_gid;
> +	old_flags = ni->flags & FILE_ATTR_READONLY;
> +
>   	setattr_copy(idmap, vi, attr);
>   
>   	if (vol->sb->s_flags & SB_POSIXACL && !S_ISLNK(vi->i_mode)) {
>   		err = posix_acl_chmod(idmap, dentry, vi->i_mode);
>   		if (err)
> -			goto out;
> +			goto out_restore;
>   	}
>   
>   	if (0222 & vi->i_mode)
> @@ -378,11 +387,26 @@ int ntfs_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
>   		err = ntfs_ea_set_wsl_inode(vi, 0, NULL, flags);
>   		mutex_unlock(&ni->mrec_lock);
>   		if (err)
> -			goto out;
> +			goto out_restore;
>   

Hi Hongling,
    Thanks for the patch.  Restoring the in-memory attributes addresses 
the failure case where no on-disk update has been committed.
   However, ntfs_ea_set_wsl_inode() updates $LXUID, $LXGID and $LXMOD 
independently, so it can fail after partially updating the on-disk 
metadata.  In that case, restoring all VFS attributes to their old 
values can itself create an inconsistency with the disk state.  The ACL 
path has a similar issue if the ACL update succeeds before the later EA 
update fails.

   This requires transactional updates of the related EAs with atomic 
failure handling.  Maintainer Namjae is currently working on the NTFS 
journal support, which should provide the necessary transactional 
infrastructure.
   Could this patch be deferred until that journal work is available, so 
that the attribute updates and their rollback can be handled atomically?

Thanks,
Baolin.

>   	}
>   
>   	mark_inode_dirty(vi);
> +	goto out;
> +
> +out_restore:
> +	/*
> +	 * The on-disk metadata update failed: put the in-memory
> +	 * attributes back so that the inode does not keep the new,
> +	 * uncommitted values.  This is best effort: the ctime update is
> +	 * left in place, ntfs_ea_set_wsl_inode() may have completed some
> +	 * of the EAs before failing, and a failed ACL update may leave
> +	 * the cached ACL divergent.
> +	 */
> +	vi->i_mode = old_mode;
> +	vi->i_uid = old_uid;
> +	vi->i_gid = old_gid;
> +	ni->flags = (ni->flags & ~FILE_ATTR_READONLY) | old_flags;
>   out:
>   	return err;
>   }

Re: [PATCH 1/2] ntfs: write back the truncated range before dropping it

[liubaolin]

在 2026/9/15 14:27, Hongling Zeng 写道:
> On a truncate, ntfs_setattr_size() drops the page cache beyond the new
> size with truncate_setsize() and only then updates the attribute on
> disk.  If that update fails, the size is reverted and the data beyond
> the new size becomes visible again -- but the pages were already
> dropped, and dirty data in them is lost, even though the truncate, the
> operation the user asked for, never happened.
> 
> Write the range back before dropping it.  If the writeback fails, fail
> the truncate with that error and keep the page cache intact; if the
> on-disk truncation fails afterwards, the reverted size can be re-read
> from disk.
> 
> Fixes: 9c87959601e8 ("ntfs: update file operations")
> Cc: stable@vger.kernel.org
> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
> ---
>   fs/ntfs/file.c | 15 +++++++++++++++
>   1 file changed, 15 insertions(+)
> 
> diff --git a/fs/ntfs/file.c b/fs/ntfs/file.c
> index 007d1614b9ac..1417a76788ed 100644
> --- a/fs/ntfs/file.c
> +++ b/fs/ntfs/file.c
> @@ -283,12 +283,27 @@ static int ntfs_setattr_size(struct inode *vi, struct iattr *attr)
>   		i_size_write(vi, attr->ia_size);
>   		pagecache_isize_extended(vi, old_size, attr->ia_size);
>   	} else {
> +		/*
> +		 * The on-disk truncation below can fail, in which case
> +		 * the size is reverted and the data beyond the new size
> +		 * becomes visible again.  Write that range back first:
> +		 * truncate_setsize() drops those pages and dirty data
> +		 * cannot be recovered afterwards.
> +		 */
> +		if (attr->ia_size < old_size) {
> +			err = filemap_write_and_wait_range(vi->i_mapping,
> +					attr->ia_size, old_size - 1);
> +			if (err)
> +				goto out_unlock;
> +		}
> +
>   		truncate_setsize(vi, attr->ia_size);
>   	}
>   
>   	err = ntfs_truncate_vfs(vi, attr->ia_size, old_size);
>   	if (err)
>   		i_size_write(vi, old_size);
> +out_unlock:
>   	filemap_invalidate_unlock(vi->i_mapping);
>   
>   	return err;

Hi Hongling,
   Thanks for the patch.  This fixes the page-aligned truncate case.

   However, this does not fully preserve the original data for a 
non-page-aligned truncate.  For example, with 4 KiB pages, consider 
truncating an 8 KiB file to 3000 bytes.  The writeback added by this 
patch preserves bytes 3000..8191 on disk, but truncate_setsize() 
eventually calls truncate_inode_partial_folio(), which zeroes bytes 
3000..4095 in the folio containing the new EOF while leaving that folio 
in the page cache.
   If ntfs_truncate_vfs() then fails, restoring only i_size makes 
3000..4095 visible again, but the EOF folio is still uptodate. Reads can 
therefore return the cached zeros instead of refaulting the original 
data from disk.

   The error path also needs to restore the folio containing the new 
EOF, or invalidate it so that it is refaulted from disk, with mmap 
writers taken into account.

Thanks,
Baolin.