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

[Hyunchul Lee <hyc.lee@gmail.com>]

On Wed, Oct 28, 2026 at 04:06:56PM +0800, Hongling Zeng wrote:
> 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>

Looks good to me.

Reviewed-by: Hyunchul Lee <hyc.lee@gmail.com>
> ---
>  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
> 

-- 
Thanks,
Hyunchul