mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 1/2] ntfs: take invalidate_lock in ntfs_setattr_size()
@ 2026-08-28  6:16 Hongling Zeng
  2026-08-28  6:16 ` [PATCH 2/2] ntfs: take invalidate_lock in ntfs_filemap_page_mkwrite() Hongling Zeng
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Hongling Zeng @ 2026-08-28  6:16 UTC (permalink / raw)
  To: linkinjeon, hyc.lee
  Cc: ntfs, linux-kernel, zhongling0719, Hongling Zeng, stable

ntfs_setattr_size() updates i_size and resizes the on-disk attribute
without holding mapping->invalidate_lock.  Page faults take the lock
shared, so a fault racing the resize can resolve a VCN against the
transient runlist state of ntfs_non_resident_attr_expand() and fail
with a spurious SIGBUS, and can interleave with the size-change
epilogue (truncate_pagecache(), i_size_write(),
pagecache_isize_extended()).

Take invalidate_lock exclusively around the whole resize after
inode_dio_wait(), matching the fallocate path and other filesystems
such as xfs, which wraps truncate in its mmaplock (= invalidate_lock).

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, 11 insertions(+), 4 deletions(-)

diff --git a/fs/ntfs/file.c b/fs/ntfs/file.c
index 1969e4f444f7..585ab2145797 100644
--- a/fs/ntfs/file.c
+++ b/fs/ntfs/file.c
@@ -270,18 +270,25 @@ static int ntfs_setattr_size(struct inode *vi, struct iattr *attr)
 		return err;
 
 	inode_dio_wait(vi);
+
+	/*
+	 * Serialize with page faults and pagecache instantiation so that
+	 * readers cannot observe the size change until the attribute
+	 * updates below have completed.
+	 */
+	filemap_invalidate_lock(vi->i_mapping);
 	if (attr->ia_size > old_size) {
 		truncate_pagecache(vi, old_size);
 		i_size_write(vi, attr->ia_size);
 		pagecache_isize_extended(vi, old_size, attr->ia_size);
-	} else
+	} else {
 		truncate_setsize(vi, attr->ia_size);
+	}
 
 	err = ntfs_truncate_vfs(vi, attr->ia_size, old_size);
-	if (err) {
+	if (err)
 		i_size_write(vi, old_size);
-		return err;
-	}
+	filemap_invalidate_unlock(vi->i_mapping);
 
 	return err;
 }
-- 
2.25.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 2/2] ntfs: take invalidate_lock in ntfs_filemap_page_mkwrite()
  2026-08-28  6:16 [PATCH 1/2] ntfs: take invalidate_lock in ntfs_setattr_size() Hongling Zeng
@ 2026-08-28  6:16 ` Hongling Zeng
  2026-08-28  6:53   ` liubaolin
                     ` (2 more replies)
  2026-08-28  6:47 ` [PATCH 1/2] ntfs: take invalidate_lock in ntfs_setattr_size() liubaolin
  2026-08-31  5:54 ` Hyunchul Lee
  2 siblings, 3 replies; 8+ messages in thread
From: Hongling Zeng @ 2026-08-28  6:16 UTC (permalink / raw)
  To: linkinjeon, hyc.lee
  Cc: ntfs, linux-kernel, zhongling0719, Hongling Zeng, stable

ntfs_filemap_page_mkwrite() calls iomap_page_mkwrite() without holding
mapping->invalidate_lock, so a concurrent truncate or fallocate can be
in the middle of invalidating pagecache and rewriting the runlist while
the write fault maps blocks and dirties the folio.  This races with
ntfs_attr_fallocate(), which merges clusters into the in-memory
runlist, drops the runlist lock, and only afterwards zeroes the newly
allocated clusters on disk; and with the punch-hole/insert/collapse
paths that free clusters after truncating the cache.

Per Documentation/filesystems/locking.rst, ->page_mkwrite() must ensure
there are no truncate/invalidate races, "usually mapping->invalidate_lock
is suitable for proper serialization".  xfs takes its mmaplock (= the
invalidate_lock rwsem) shared in exactly this path.

Take invalidate_lock shared around iomap_page_mkwrite().  The read-only
fault path is already covered because filemap_fault() itself grabs
invalidate_lock shared on instantiation/read paths; only page_mkwrite
was bypassing it in this driver.

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

diff --git a/fs/ntfs/file.c b/fs/ntfs/file.c
index 585ab2145797..df5c9d79ed09 100644
--- a/fs/ntfs/file.c
+++ b/fs/ntfs/file.c
@@ -676,6 +676,7 @@ static ssize_t ntfs_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
 static vm_fault_t ntfs_filemap_page_mkwrite(struct vm_fault *vmf)
 {
 	struct inode *inode = file_inode(vmf->vma->vm_file);
+	struct address_space *mapping = inode->i_mapping;
 	vm_fault_t ret;
 
 	if (NInoWofCompressed(NTFS_I(inode)))
@@ -684,7 +685,14 @@ static vm_fault_t ntfs_filemap_page_mkwrite(struct vm_fault *vmf)
 	sb_start_pagefault(inode->i_sb);
 	file_update_time(vmf->vma->vm_file);
 
+	/*
+	 * Serialize against truncate/fallocate which hold the lock
+	 * exclusively while invalidating pagecache and changing extents.
+	 */
+	filemap_invalidate_lock_shared(mapping);
 	ret = iomap_page_mkwrite(vmf, &ntfs_page_mkwrite_iomap_ops, NULL);
+	filemap_invalidate_unlock_shared(mapping);
+
 	sb_end_pagefault(inode->i_sb);
 	return ret;
 }
-- 
2.25.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] ntfs: take invalidate_lock in ntfs_setattr_size()
  2026-08-28  6:16 [PATCH 1/2] ntfs: take invalidate_lock in ntfs_setattr_size() Hongling Zeng
  2026-08-28  6:16 ` [PATCH 2/2] ntfs: take invalidate_lock in ntfs_filemap_page_mkwrite() Hongling Zeng
@ 2026-08-28  6:47 ` liubaolin
  2026-08-31  5:54 ` Hyunchul Lee
  2 siblings, 0 replies; 8+ messages in thread
From: liubaolin @ 2026-08-28  6:47 UTC (permalink / raw)
  To: Hongling Zeng, linkinjeon, hyc.lee
  Cc: ntfs, linux-kernel, zhongling0719, stable



在 2026/8/28 14:16, Hongling Zeng 写道:
> ntfs_setattr_size() updates i_size and resizes the on-disk attribute
> without holding mapping->invalidate_lock.  Page faults take the lock
> shared, so a fault racing the resize can resolve a VCN against the
> transient runlist state of ntfs_non_resident_attr_expand() and fail
> with a spurious SIGBUS, and can interleave with the size-change
> epilogue (truncate_pagecache(), i_size_write(),
> pagecache_isize_extended()).
> 
> Take invalidate_lock exclusively around the whole resize after
> inode_dio_wait(), matching the fallocate path and other filesystems
> such as xfs, which wraps truncate in its mmaplock (= invalidate_lock).
> 
> 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, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/ntfs/file.c b/fs/ntfs/file.c
> index 1969e4f444f7..585ab2145797 100644
> --- a/fs/ntfs/file.c
> +++ b/fs/ntfs/file.c
> @@ -270,18 +270,25 @@ static int ntfs_setattr_size(struct inode *vi, struct iattr *attr)
>   		return err;
>   
>   	inode_dio_wait(vi);
> +
> +	/*
> +	 * Serialize with page faults and pagecache instantiation so that
> +	 * readers cannot observe the size change until the attribute
> +	 * updates below have completed.
> +	 */
> +	filemap_invalidate_lock(vi->i_mapping);
>   	if (attr->ia_size > old_size) {
>   		truncate_pagecache(vi, old_size);
>   		i_size_write(vi, attr->ia_size);
>   		pagecache_isize_extended(vi, old_size, attr->ia_size);
> -	} else
> +	} else {
>   		truncate_setsize(vi, attr->ia_size);
> +	}
>   
>   	err = ntfs_truncate_vfs(vi, attr->ia_size, old_size);
> -	if (err) {
> +	if (err)
>   		i_size_write(vi, old_size);
> -		return err;
> -	}
> +	filemap_invalidate_unlock(vi->i_mapping);
>   
>   	return err;
>   }

Looks good to me.
Reviewed-by: Baolin Liu <liubaolin@kylinos.cn>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] ntfs: take invalidate_lock in ntfs_filemap_page_mkwrite()
  2026-08-28  6:16 ` [PATCH 2/2] ntfs: take invalidate_lock in ntfs_filemap_page_mkwrite() Hongling Zeng
@ 2026-08-28  6:53   ` liubaolin
  2026-08-31  5:56   ` Hyunchul Lee
  2026-08-31  7:24   ` Namjae Jeon
  2 siblings, 0 replies; 8+ messages in thread
From: liubaolin @ 2026-08-28  6:53 UTC (permalink / raw)
  To: Hongling Zeng, linkinjeon, hyc.lee
  Cc: ntfs, linux-kernel, zhongling0719, stable



在 2026/8/28 14:16, Hongling Zeng 写道:
> ntfs_filemap_page_mkwrite() calls iomap_page_mkwrite() without holding
> mapping->invalidate_lock, so a concurrent truncate or fallocate can be
> in the middle of invalidating pagecache and rewriting the runlist while
> the write fault maps blocks and dirties the folio.  This races with
> ntfs_attr_fallocate(), which merges clusters into the in-memory
> runlist, drops the runlist lock, and only afterwards zeroes the newly
> allocated clusters on disk; and with the punch-hole/insert/collapse
> paths that free clusters after truncating the cache.
> 
> Per Documentation/filesystems/locking.rst, ->page_mkwrite() must ensure
> there are no truncate/invalidate races, "usually mapping->invalidate_lock
> is suitable for proper serialization".  xfs takes its mmaplock (= the
> invalidate_lock rwsem) shared in exactly this path.
> 
> Take invalidate_lock shared around iomap_page_mkwrite().  The read-only
> fault path is already covered because filemap_fault() itself grabs
> invalidate_lock shared on instantiation/read paths; only page_mkwrite
> was bypassing it in this driver.
> 
> Fixes: 9c87959601e8 ("ntfs: update file operations")
> Cc: stable@vger.kernel.org
> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
> ---
>   fs/ntfs/file.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/fs/ntfs/file.c b/fs/ntfs/file.c
> index 585ab2145797..df5c9d79ed09 100644
> --- a/fs/ntfs/file.c
> +++ b/fs/ntfs/file.c
> @@ -676,6 +676,7 @@ static ssize_t ntfs_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
>   static vm_fault_t ntfs_filemap_page_mkwrite(struct vm_fault *vmf)
>   {
>   	struct inode *inode = file_inode(vmf->vma->vm_file);
> +	struct address_space *mapping = inode->i_mapping;
>   	vm_fault_t ret;
>   
>   	if (NInoWofCompressed(NTFS_I(inode)))
> @@ -684,7 +685,14 @@ static vm_fault_t ntfs_filemap_page_mkwrite(struct vm_fault *vmf)
>   	sb_start_pagefault(inode->i_sb);
>   	file_update_time(vmf->vma->vm_file);
>   
> +	/*
> +	 * Serialize against truncate/fallocate which hold the lock
> +	 * exclusively while invalidating pagecache and changing extents.
> +	 */
> +	filemap_invalidate_lock_shared(mapping);
>   	ret = iomap_page_mkwrite(vmf, &ntfs_page_mkwrite_iomap_ops, NULL);
> +	filemap_invalidate_unlock_shared(mapping);
> +
>   	sb_end_pagefault(inode->i_sb);
>   	return ret;
>   }

Looks good to me.
Reviewed-by: Baolin Liu <liubaolin@kylinos.cn>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/2] ntfs: take invalidate_lock in ntfs_setattr_size()
  2026-08-28  6:16 [PATCH 1/2] ntfs: take invalidate_lock in ntfs_setattr_size() Hongling Zeng
  2026-08-28  6:16 ` [PATCH 2/2] ntfs: take invalidate_lock in ntfs_filemap_page_mkwrite() Hongling Zeng
  2026-08-28  6:47 ` [PATCH 1/2] ntfs: take invalidate_lock in ntfs_setattr_size() liubaolin
@ 2026-08-31  5:54 ` Hyunchul Lee
  2 siblings, 0 replies; 8+ messages in thread
From: Hyunchul Lee @ 2026-08-31  5:54 UTC (permalink / raw)
  To: Hongling Zeng; +Cc: linkinjeon, ntfs, linux-kernel, zhongling0719, stable

2026년 8월 28일 (금) 오후 3:16, Hongling Zeng <zenghongling@kylinos.cn>님이 작성:
>
> ntfs_setattr_size() updates i_size and resizes the on-disk attribute
> without holding mapping->invalidate_lock.  Page faults take the lock
> shared, so a fault racing the resize can resolve a VCN against the
> transient runlist state of ntfs_non_resident_attr_expand() and fail
> with a spurious SIGBUS, and can interleave with the size-change
> epilogue (truncate_pagecache(), i_size_write(),
> pagecache_isize_extended()).
>
> Take invalidate_lock exclusively around the whole resize after
> inode_dio_wait(), matching the fallocate path and other filesystems
> such as xfs, which wraps truncate in its mmaplock (= invalidate_lock).
>
> Fixes: 9c87959601e8 ("ntfs: update file operations")
> Cc: stable@vger.kernel.org
> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>

Looks good to me.

Reviewed-by: Hyunchul Lee <hyc.lee@gmail.com>

-- 
Thanks,
Hyunchul

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] ntfs: take invalidate_lock in ntfs_filemap_page_mkwrite()
  2026-08-28  6:16 ` [PATCH 2/2] ntfs: take invalidate_lock in ntfs_filemap_page_mkwrite() Hongling Zeng
  2026-08-28  6:53   ` liubaolin
@ 2026-08-31  5:56   ` Hyunchul Lee
  2026-08-31  7:24   ` Namjae Jeon
  2 siblings, 0 replies; 8+ messages in thread
From: Hyunchul Lee @ 2026-08-31  5:56 UTC (permalink / raw)
  To: Hongling Zeng; +Cc: linkinjeon, ntfs, linux-kernel, zhongling0719, stable

2026년 8월 28일 (금) 오후 3:16, Hongling Zeng <zenghongling@kylinos.cn>님이 작성:
>
> ntfs_filemap_page_mkwrite() calls iomap_page_mkwrite() without holding
> mapping->invalidate_lock, so a concurrent truncate or fallocate can be
> in the middle of invalidating pagecache and rewriting the runlist while
> the write fault maps blocks and dirties the folio.  This races with
> ntfs_attr_fallocate(), which merges clusters into the in-memory
> runlist, drops the runlist lock, and only afterwards zeroes the newly
> allocated clusters on disk; and with the punch-hole/insert/collapse
> paths that free clusters after truncating the cache.
>
> Per Documentation/filesystems/locking.rst, ->page_mkwrite() must ensure
> there are no truncate/invalidate races, "usually mapping->invalidate_lock
> is suitable for proper serialization".  xfs takes its mmaplock (= the
> invalidate_lock rwsem) shared in exactly this path.
>
> Take invalidate_lock shared around iomap_page_mkwrite().  The read-only
> fault path is already covered because filemap_fault() itself grabs
> invalidate_lock shared on instantiation/read paths; only page_mkwrite
> was bypassing it in this driver.
>
> Fixes: 9c87959601e8 ("ntfs: update file operations")
> Cc: stable@vger.kernel.org
> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>

Looks good to me.

Reviewed-by: Hyunchul Lee <hyc.lee@gmail.com>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] ntfs: take invalidate_lock in ntfs_filemap_page_mkwrite()
  2026-08-28  6:16 ` [PATCH 2/2] ntfs: take invalidate_lock in ntfs_filemap_page_mkwrite() Hongling Zeng
  2026-08-28  6:53   ` liubaolin
  2026-08-31  5:56   ` Hyunchul Lee
@ 2026-08-31  7:24   ` Namjae Jeon
  2026-08-31  8:21     ` Hongling Zeng
  2 siblings, 1 reply; 8+ messages in thread
From: Namjae Jeon @ 2026-08-31  7:24 UTC (permalink / raw)
  To: Hongling Zeng; +Cc: hyc.lee, ntfs, linux-kernel, zhongling0719, stable

>         if (NInoWofCompressed(NTFS_I(inode)))
> @@ -684,7 +685,14 @@ static vm_fault_t ntfs_filemap_page_mkwrite(struct vm_fault *vmf)
>         sb_start_pagefault(inode->i_sb);
>         file_update_time(vmf->vma->vm_file);
>
> +       /*
> +        * Serialize against truncate/fallocate which hold the lock
> +        * exclusively while invalidating pagecache and changing extents.
> +        */
> +       filemap_invalidate_lock_shared(mapping);
>         ret = iomap_page_mkwrite(vmf, &ntfs_page_mkwrite_iomap_ops, NULL);
> +       filemap_invalidate_unlock_shared(mapping);
> +
Please move filemap_invalidate_unlock() after truncate_pagecache() and
pagecache_isize_extended(). Otherwise, ntfs_filemap_page_mkwrite() can
race with the page-cache cleanup after fallocate extends the file.

diff --git a/fs/ntfs/file.c b/fs/ntfs/file.c
index 1969e4f444f7..a266105180a9 100644
--- a/fs/ntfs/file.c
+++ b/fs/ntfs/file.c
@@ -1178,13 +1178,15 @@ static long ntfs_fallocate(struct file *file,
int mode, loff_t offset, loff_t le

        err = file_modified(file);
 out:
+       if (!err && mode == 0 && NInoNonResident(ni) &&
+           offset > old_size) {
+               truncate_pagecache(vi, old_size);
+               pagecache_isize_extended(vi, old_size, offset);
+       }
+
        filemap_invalidate_unlock(vi->i_mapping);
+
        if (!err) {
-               if (mode == 0 && NInoNonResident(ni) &&
-                   offset > old_size) {
-                       truncate_pagecache(vi, old_size);
-                       pagecache_isize_extended(vi, old_size, offset);
-               }
                NInoSetFileNameDirty(ni);
                inode_set_mtime_to_ts(vi, inode_set_ctime_current(vi));
                mark_inode_dirty(vi);

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/2] ntfs: take invalidate_lock in ntfs_filemap_page_mkwrite()
  2026-08-31  7:24   ` Namjae Jeon
@ 2026-08-31  8:21     ` Hongling Zeng
  0 siblings, 0 replies; 8+ messages in thread
From: Hongling Zeng @ 2026-08-31  8:21 UTC (permalink / raw)
  To: Namjae Jeon, Hongling Zeng; +Cc: hyc.lee, ntfs, linux-kernel, stable


在 2026年08月31日 15:24, Namjae Jeon 写道:
>>          if (NInoWofCompressed(NTFS_I(inode)))
>> @@ -684,7 +685,14 @@ static vm_fault_t ntfs_filemap_page_mkwrite(struct vm_fault *vmf)
>>          sb_start_pagefault(inode->i_sb);
>>          file_update_time(vmf->vma->vm_file);
>>
>> +       /*
>> +        * Serialize against truncate/fallocate which hold the lock
>> +        * exclusively while invalidating pagecache and changing extents.
>> +        */
>> +       filemap_invalidate_lock_shared(mapping);
>>          ret = iomap_page_mkwrite(vmf, &ntfs_page_mkwrite_iomap_ops, NULL);
>> +       filemap_invalidate_unlock_shared(mapping);
>> +
> Please move filemap_invalidate_unlock() after truncate_pagecache() and
> pagecache_isize_extended(). Otherwise, ntfs_filemap_page_mkwrite() can
> race with the page-cache cleanup after fallocate extends the file.
>
> diff --git a/fs/ntfs/file.c b/fs/ntfs/file.c
> index 1969e4f444f7..a266105180a9 100644
> --- a/fs/ntfs/file.c
> +++ b/fs/ntfs/file.c
> @@ -1178,13 +1178,15 @@ static long ntfs_fallocate(struct file *file,
> int mode, loff_t offset, loff_t le
>
>          err = file_modified(file);
>   out:
> +       if (!err && mode == 0 && NInoNonResident(ni) &&
> +           offset > old_size) {
> +               truncate_pagecache(vi, old_size);
> +               pagecache_isize_extended(vi, old_size, offset);
> +       }
> +
>          filemap_invalidate_unlock(vi->i_mapping);
> +
>          if (!err) {
> -               if (mode == 0 && NInoNonResident(ni) &&
> -                   offset > old_size) {
> -                       truncate_pagecache(vi, old_size);
> -                       pagecache_isize_extended(vi, old_size, offset);
> -               }
>                  NInoSetFileNameDirty(ni);
>                  inode_set_mtime_to_ts(vi, inode_set_ctime_current(vi));
>                  mark_inode_dirty(vi);
>
   Thanks for the review!

   You're right. ntfs_fallocate() dropped invalidate_lock before calling
   truncate_pagecache() and pagecache_isize_extended(), so a write fault
   holding the lock shared could interleave with the page-cache cleanup
   and have its folio discarded by truncate_inode_pages(), losing the
   mmap write. Moving both calls inside the lock hold range fixes it.

   I've posted v2 and addresses your comment.


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-08-31  8:22 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-28  6:16 [PATCH 1/2] ntfs: take invalidate_lock in ntfs_setattr_size() Hongling Zeng
2026-08-28  6:16 ` [PATCH 2/2] ntfs: take invalidate_lock in ntfs_filemap_page_mkwrite() Hongling Zeng
2026-08-28  6:53   ` liubaolin
2026-08-31  5:56   ` Hyunchul Lee
2026-08-31  7:24   ` Namjae Jeon
2026-08-31  8:21     ` Hongling Zeng
2026-08-28  6:47 ` [PATCH 1/2] ntfs: take invalidate_lock in ntfs_setattr_size() liubaolin
2026-08-31  5:54 ` Hyunchul Lee

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®