[PATCH 1/2] ntfs: keep the file size on a failed initialized-size extension

[PATCH 1/2] ntfs: keep the file size on a failed initialized-size extension

[Hongling Zeng]

ntfs_extend_initialized_size() only extends the initialized size and
never changes the data size, but on a failure it calls
truncate_setsize() with the old initialized size, which shrinks the
VFS size down to it and drops the page cache beyond that point.

None of the callers rolls the shrunk size back.  A buffered write
into a hole has nothing to roll back - the data size never changed -
so the file keeps the shrunk size even though the data size on disk
is intact.  The mmap and named stream callers return without any
rollback either.

The sizes did not change, so nothing needs the shrink.  Remove it.

Fixes: af0db57d4293 ("ntfs: update inode operations")
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
---
 fs/ntfs/inode.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/fs/ntfs/inode.c b/fs/ntfs/inode.c
index 206db4e99ef1..ac13e8776909 100644
--- a/fs/ntfs/inode.c
+++ b/fs/ntfs/inode.c
@@ -2599,8 +2599,6 @@ int ntfs_extend_initialized_size(struct inode *vi, const loff_t offset,
 	mutex_lock(&mrec_ni->mrec_lock);
 	err = ntfs_attr_set_initialized_size(ni, new_size);
 	mutex_unlock(&mrec_ni->mrec_lock);
-	if (err)
-		truncate_setsize(vi, old_init_size);
 out:
 	filemap_invalidate_unlock(vi->i_mapping);
 	return err;
-- 
2.25.1

[PATCH 2/2] ntfs: fix a self-deadlock on writes to non-resident named streams

[Hongling Zeng]

The stream write ioctl holds the stream mapping's invalidate lock
across ntfs_inode_attr_pwrite(), which calls
ntfs_extend_initialized_size() for named streams, and that function
takes the same lock again, before the check that could skip the
extension.  A write to a non-resident named stream through
NTFS_IOC_STREAM_WRITE deadlocks on itself.

The ioctl is the only caller that reaches the named stream branch of
ntfs_inode_attr_pwrite(), and it always holds the lock there, so
split off __ntfs_extend_initialized_size(), which expects the caller
to hold the invalidate lock, and call it from that branch.  The write
and mmap paths keep taking the lock themselves.

Fixes: 3f11c48d4f94 ("ntfs: add pathname access for named streams")
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
---
 fs/ntfs/inode.c | 30 +++++++++++++++++++++++++-----
 1 file changed, 25 insertions(+), 5 deletions(-)

diff --git a/fs/ntfs/inode.c b/fs/ntfs/inode.c
index ac13e8776909..9275ebfebe3d 100644
--- a/fs/ntfs/inode.c
+++ b/fs/ntfs/inode.c
@@ -2554,8 +2554,12 @@ int ntfs_show_options(struct seq_file *sf, struct dentry *root)
 	return 0;
 }
 
-int ntfs_extend_initialized_size(struct inode *vi, const loff_t offset,
-				 const loff_t new_size)
+/*
+ * The caller must hold @vi's mapping invalidate lock.
+ */
+static int __ntfs_extend_initialized_size(struct inode *vi,
+					  const loff_t offset,
+					  const loff_t new_size)
 {
 	struct ntfs_inode *ni = NTFS_I(vi);
 	struct ntfs_inode *mrec_ni = ntfs_base_inode(ni);
@@ -2563,10 +2567,11 @@ int ntfs_extend_initialized_size(struct inode *vi, const loff_t offset,
 	unsigned long flags;
 	int err = 0;
 
+	lockdep_assert_held_write(&vi->i_mapping->invalidate_lock);
+
 	if (!NInoNonResident(ni))
 		return -EINVAL;
 
-	filemap_invalidate_lock(vi->i_mapping);
 	read_lock_irqsave(&ni->size_lock, flags);
 	old_init_size = ni->initialized_size;
 	read_unlock_irqrestore(&ni->size_lock, flags);
@@ -2600,6 +2605,16 @@ int ntfs_extend_initialized_size(struct inode *vi, const loff_t offset,
 	err = ntfs_attr_set_initialized_size(ni, new_size);
 	mutex_unlock(&mrec_ni->mrec_lock);
 out:
+	return err;
+}
+
+int ntfs_extend_initialized_size(struct inode *vi, const loff_t offset,
+				 const loff_t new_size)
+{
+	int err;
+
+	filemap_invalidate_lock(vi->i_mapping);
+	err = __ntfs_extend_initialized_size(vi, offset, new_size);
 	filemap_invalidate_unlock(vi->i_mapping);
 	return err;
 }
@@ -4085,8 +4100,13 @@ s64 ntfs_inode_attr_pwrite(struct inode *vi, s64 pos, s64 count, u8 *buf, bool s
 	mutex_unlock(&mrec_ni->mrec_lock);
 	/* Attribute writes bypass iomap's delayed-allocation preparation. */
 	if (ntfs_inode_is_named_stream(ni)) {
-		/* A failed write must not expose any newly initialized data. */
-		ret = ntfs_extend_initialized_size(vi, pos + count, pos + count);
+		/*
+		 * A failed write must not expose any newly initialized
+		 * data.  The stream ioctl caller holds the mapping
+		 * invalidate lock.
+		 */
+		ret = __ntfs_extend_initialized_size(vi, pos + count,
+				pos + count);
 		modified |= ni->initialized_size != old_init_size;
 		if (!ret)
 			ret = ntfs_attr_fallocate(ni, pos, count, true);
-- 
2.25.1

Re: [PATCH 1/2] ntfs: keep the file size on a failed initialized-size extension

[liubaolin]

在 2026/10/28 16:06, Hongling Zeng 写道:
> ntfs_extend_initialized_size() only extends the initialized size and
> never changes the data size, but on a failure it calls
> truncate_setsize() with the old initialized size, which shrinks the
> VFS size down to it and drops the page cache beyond that point.
> 
> None of the callers rolls the shrunk size back.  A buffered write
> into a hole has nothing to roll back - the data size never changed -
> so the file keeps the shrunk size even though the data size on disk
> is intact.  The mmap and named stream callers return without any
> rollback either.
> 
> The sizes did not change, so nothing needs the shrink.  Remove it.
> 
> Fixes: af0db57d4293 ("ntfs: update inode operations")
> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
> ---
>   fs/ntfs/inode.c | 2 --
>   1 file changed, 2 deletions(-)
> 
> diff --git a/fs/ntfs/inode.c b/fs/ntfs/inode.c
> index 206db4e99ef1..ac13e8776909 100644
> --- a/fs/ntfs/inode.c
> +++ b/fs/ntfs/inode.c
> @@ -2599,8 +2599,6 @@ int ntfs_extend_initialized_size(struct inode *vi, const loff_t offset,
>   	mutex_lock(&mrec_ni->mrec_lock);
>   	err = ntfs_attr_set_initialized_size(ni, new_size);
>   	mutex_unlock(&mrec_ni->mrec_lock);
> -	if (err)
> -		truncate_setsize(vi, old_init_size);
>   out:
>   	filemap_invalidate_unlock(vi->i_mapping);
>   	return err;

Looks good to me.

Reviewed-by: Baolin Liu <liubaolin@kylinos.cn>

Re: [PATCH 2/2] ntfs: fix a self-deadlock on writes to non-resident named streams

[liubaolin]

在 2026/10/28 16:06, Hongling Zeng 写道:
> The stream write ioctl holds the stream mapping's invalidate lock
> across ntfs_inode_attr_pwrite(), which calls
> ntfs_extend_initialized_size() for named streams, and that function
> takes the same lock again, before the check that could skip the
> extension.  A write to a non-resident named stream through
> NTFS_IOC_STREAM_WRITE deadlocks on itself.
> 
> The ioctl is the only caller that reaches the named stream branch of
> ntfs_inode_attr_pwrite(), and it always holds the lock there, so
> split off __ntfs_extend_initialized_size(), which expects the caller
> to hold the invalidate lock, and call it from that branch.  The write
> and mmap paths keep taking the lock themselves.
> 
> Fixes: 3f11c48d4f94 ("ntfs: add pathname access for named streams")
> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
> ---
>   fs/ntfs/inode.c | 30 +++++++++++++++++++++++++-----
>   1 file changed, 25 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/ntfs/inode.c b/fs/ntfs/inode.c
> index ac13e8776909..9275ebfebe3d 100644
> --- a/fs/ntfs/inode.c
> +++ b/fs/ntfs/inode.c
> @@ -2554,8 +2554,12 @@ int ntfs_show_options(struct seq_file *sf, struct dentry *root)
>   	return 0;
>   }
>   
> -int ntfs_extend_initialized_size(struct inode *vi, const loff_t offset,
> -				 const loff_t new_size)
> +/*
> + * The caller must hold @vi's mapping invalidate lock.
> + */
> +static int __ntfs_extend_initialized_size(struct inode *vi,
> +					  const loff_t offset,
> +					  const loff_t new_size)
>   {
>   	struct ntfs_inode *ni = NTFS_I(vi);
>   	struct ntfs_inode *mrec_ni = ntfs_base_inode(ni);
> @@ -2563,10 +2567,11 @@ int ntfs_extend_initialized_size(struct inode *vi, const loff_t offset,
>   	unsigned long flags;
>   	int err = 0;
>   
> +	lockdep_assert_held_write(&vi->i_mapping->invalidate_lock);
> +
>   	if (!NInoNonResident(ni))
>   		return -EINVAL;
>   
> -	filemap_invalidate_lock(vi->i_mapping);
>   	read_lock_irqsave(&ni->size_lock, flags);
>   	old_init_size = ni->initialized_size;
>   	read_unlock_irqrestore(&ni->size_lock, flags);
> @@ -2600,6 +2605,16 @@ int ntfs_extend_initialized_size(struct inode *vi, const loff_t offset,
>   	err = ntfs_attr_set_initialized_size(ni, new_size);
>   	mutex_unlock(&mrec_ni->mrec_lock);
>   out:
> +	return err;
> +}
> +
> +int ntfs_extend_initialized_size(struct inode *vi, const loff_t offset,
> +				 const loff_t new_size)
> +{
> +	int err;
> +
> +	filemap_invalidate_lock(vi->i_mapping);
> +	err = __ntfs_extend_initialized_size(vi, offset, new_size);
>   	filemap_invalidate_unlock(vi->i_mapping);
>   	return err;
>   }
> @@ -4085,8 +4100,13 @@ s64 ntfs_inode_attr_pwrite(struct inode *vi, s64 pos, s64 count, u8 *buf, bool s
>   	mutex_unlock(&mrec_ni->mrec_lock);
>   	/* Attribute writes bypass iomap's delayed-allocation preparation. */
>   	if (ntfs_inode_is_named_stream(ni)) {
> -		/* A failed write must not expose any newly initialized data. */
> -		ret = ntfs_extend_initialized_size(vi, pos + count, pos + count);
> +		/*
> +		 * A failed write must not expose any newly initialized
> +		 * data.  The stream ioctl caller holds the mapping
> +		 * invalidate lock.
> +		 */
> +		ret = __ntfs_extend_initialized_size(vi, pos + count,
> +				pos + count);
>   		modified |= ni->initialized_size != old_init_size;
>   		if (!ret)
>   			ret = ntfs_attr_fallocate(ni, pos, count, true);

Looks good to me.

Reviewed-by: Baolin Liu <liubaolin@kylinos.cn>