mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] ntfs: fix race between fallocate and mmap reads
@ 2026-08-26  5:42 Hongling Zeng
  2026-08-26 12:09 ` liubaolin
  2026-08-27  2:43 ` Hyunchul Lee
  0 siblings, 2 replies; 5+ messages in thread
From: Hongling Zeng @ 2026-08-26  5:42 UTC (permalink / raw)
  To: linkinjeon, hyc.lee
  Cc: ntfs, linux-kernel, zhongling0719, Hongling Zeng, stable

The fallocate implementation only takes invalidate_lock for punch hole,
collapse range, and insert range operations. For standard allocation modes
(mode == 0, FALLOC_FL_KEEP_SIZE), the lock is not held.

During ntfs_attr_fallocate(), new clusters are mapped to the runlist via
ntfs_attr_map_cluster() before being zeroed by ntfs_dio_zero_range(). This
creates a window where concurrent mmap page faults can read uninitialized
disk data.

Since mmap uses filemap_fault() which takes invalidate_lock in shared mode,
it can fault in pages during this window and expose old disk contents to
userspace. This is an information leak and data integrity issue.

Fix by taking invalidate_lock for all fallocate operations, not just for
punch/collapse/insert modes. This prevents concurrent page faults from
accessing unzeroed clusters during the allocation window.

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

diff --git a/fs/ntfs/file.c b/fs/ntfs/file.c
index 88747217ba61..6958a1469fb0 100644
--- a/fs/ntfs/file.c
+++ b/fs/ntfs/file.c
@@ -1153,11 +1153,9 @@ static long ntfs_fallocate(struct file *file, int mode, loff_t offset, loff_t le
 	}
 
 	inode_dio_wait(vi);
-	if (mode & (FALLOC_FL_PUNCH_HOLE | FALLOC_FL_COLLAPSE_RANGE |
-		    FALLOC_FL_INSERT_RANGE)) {
-		filemap_invalidate_lock(vi->i_mapping);
-		map_locked = true;
-	}
+	/* Take invalidate_lock for all fallocate operations to prevent races */
+	filemap_invalidate_lock(vi->i_mapping);
+	map_locked = true;
 
 	switch (mode & FALLOC_FL_MODE_MASK) {
 	case FALLOC_FL_ALLOCATE_RANGE:
-- 
2.25.1


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

* Re: [PATCH] ntfs: fix race between fallocate and mmap reads
  2026-08-26  5:42 [PATCH] ntfs: fix race between fallocate and mmap reads Hongling Zeng
@ 2026-08-26 12:09 ` liubaolin
  2026-08-27  2:43 ` Hyunchul Lee
  1 sibling, 0 replies; 5+ messages in thread
From: liubaolin @ 2026-08-26 12:09 UTC (permalink / raw)
  To: Hongling Zeng, linkinjeon, hyc.lee
  Cc: ntfs, linux-kernel, zhongling0719, stable



在 2026/8/26 13:42, Hongling Zeng 写道:
> The fallocate implementation only takes invalidate_lock for punch hole,
> collapse range, and insert range operations. For standard allocation modes
> (mode == 0, FALLOC_FL_KEEP_SIZE), the lock is not held.
> 
> During ntfs_attr_fallocate(), new clusters are mapped to the runlist via
> ntfs_attr_map_cluster() before being zeroed by ntfs_dio_zero_range(). This
> creates a window where concurrent mmap page faults can read uninitialized
> disk data.
> 
> Since mmap uses filemap_fault() which takes invalidate_lock in shared mode,
> it can fault in pages during this window and expose old disk contents to
> userspace. This is an information leak and data integrity issue.
> 
> Fix by taking invalidate_lock for all fallocate operations, not just for
> punch/collapse/insert modes. This prevents concurrent page faults from
> accessing unzeroed clusters during the allocation window.
> 
> Fixes: 495e90fa3348 ("ntfs: update attrib operations")
> Cc: stable@vger.kernel.org
> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
> ---
>   fs/ntfs/file.c | 8 +++-----
>   1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/ntfs/file.c b/fs/ntfs/file.c
> index 88747217ba61..6958a1469fb0 100644
> --- a/fs/ntfs/file.c
> +++ b/fs/ntfs/file.c
> @@ -1153,11 +1153,9 @@ static long ntfs_fallocate(struct file *file, int mode, loff_t offset, loff_t le
>   	}
>   
>   	inode_dio_wait(vi);
> -	if (mode & (FALLOC_FL_PUNCH_HOLE | FALLOC_FL_COLLAPSE_RANGE |
> -		    FALLOC_FL_INSERT_RANGE)) {
> -		filemap_invalidate_lock(vi->i_mapping);
> -		map_locked = true;
> -	}
> +	/* Take invalidate_lock for all fallocate operations to prevent races */
> +	filemap_invalidate_lock(vi->i_mapping);
> +	map_locked = true;
>   
>   	switch (mode & FALLOC_FL_MODE_MASK) {
>   	case FALLOC_FL_ALLOCATE_RANGE:

I verified the race window exists and the fix looks correct.

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


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

* Re: [PATCH] ntfs: fix race between fallocate and mmap reads
  2026-08-26  5:42 [PATCH] ntfs: fix race between fallocate and mmap reads Hongling Zeng
  2026-08-26 12:09 ` liubaolin
@ 2026-08-27  2:43 ` Hyunchul Lee
  2026-08-27  3:11   ` Hongling Zeng
  2026-08-27  3:30   ` liubaolin
  1 sibling, 2 replies; 5+ messages in thread
From: Hyunchul Lee @ 2026-08-27  2:43 UTC (permalink / raw)
  To: Hongling Zeng; +Cc: linkinjeon, ntfs, linux-kernel, zhongling0719, stable

Hi Hongling,

2026년 8월 26일 (수) 오후 2:42, Hongling Zeng <zenghongling@kylinos.cn>님이 작성:
>
> The fallocate implementation only takes invalidate_lock for punch hole,
> collapse range, and insert range operations. For standard allocation modes
> (mode == 0, FALLOC_FL_KEEP_SIZE), the lock is not held.
>
> During ntfs_attr_fallocate(), new clusters are mapped to the runlist via
> ntfs_attr_map_cluster() before being zeroed by ntfs_dio_zero_range(). This
> creates a window where concurrent mmap page faults can read uninitialized
> disk data.
>
> Since mmap uses filemap_fault() which takes invalidate_lock in shared mode,
> it can fault in pages during this window and expose old disk contents to
> userspace. This is an information leak and data integrity issue.
>
> Fix by taking invalidate_lock for all fallocate operations, not just for
> punch/collapse/insert modes. This prevents concurrent page faults from
> accessing unzeroed clusters during the allocation window.
>
> Fixes: 495e90fa3348 ("ntfs: update attrib operations")
> Cc: stable@vger.kernel.org
> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
> ---
>  fs/ntfs/file.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/fs/ntfs/file.c b/fs/ntfs/file.c
> index 88747217ba61..6958a1469fb0 100644
> --- a/fs/ntfs/file.c
> +++ b/fs/ntfs/file.c
> @@ -1153,11 +1153,9 @@ static long ntfs_fallocate(struct file *file, int mode, loff_t offset, loff_t le
>         }
>
>         inode_dio_wait(vi);
> -       if (mode & (FALLOC_FL_PUNCH_HOLE | FALLOC_FL_COLLAPSE_RANGE |
> -                   FALLOC_FL_INSERT_RANGE)) {
> -               filemap_invalidate_lock(vi->i_mapping);
> -               map_locked = true;
> -       }
> +       /* Take invalidate_lock for all fallocate operations to prevent races */
> +       filemap_invalidate_lock(vi->i_mapping);
> +       map_locked = true;

map_locked can be removed.
Otherwise it looks good to me.

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


>
>         switch (mode & FALLOC_FL_MODE_MASK) {
>         case FALLOC_FL_ALLOCATE_RANGE:
> --
> 2.25.1
>


--
Thanks,
Hyunchul

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

* Re: [PATCH] ntfs: fix race between fallocate and mmap reads
  2026-08-27  2:43 ` Hyunchul Lee
@ 2026-08-27  3:11   ` Hongling Zeng
  2026-08-27  3:30   ` liubaolin
  1 sibling, 0 replies; 5+ messages in thread
From: Hongling Zeng @ 2026-08-27  3:11 UTC (permalink / raw)
  To: Hyunchul Lee, Hongling Zeng; +Cc: linkinjeon, ntfs, linux-kernel, stable


在 2026年08月27日 10:43, Hyunchul Lee 写道:
> Hi Hongling,
>
> 2026년 8월 26일 (수) 오후 2:42, Hongling Zeng <zenghongling@kylinos.cn>님이 작성:
>> The fallocate implementation only takes invalidate_lock for punch hole,
>> collapse range, and insert range operations. For standard allocation modes
>> (mode == 0, FALLOC_FL_KEEP_SIZE), the lock is not held.
>>
>> During ntfs_attr_fallocate(), new clusters are mapped to the runlist via
>> ntfs_attr_map_cluster() before being zeroed by ntfs_dio_zero_range(). This
>> creates a window where concurrent mmap page faults can read uninitialized
>> disk data.
>>
>> Since mmap uses filemap_fault() which takes invalidate_lock in shared mode,
>> it can fault in pages during this window and expose old disk contents to
>> userspace. This is an information leak and data integrity issue.
>>
>> Fix by taking invalidate_lock for all fallocate operations, not just for
>> punch/collapse/insert modes. This prevents concurrent page faults from
>> accessing unzeroed clusters during the allocation window.
>>
>> Fixes: 495e90fa3348 ("ntfs: update attrib operations")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
>> ---
>>   fs/ntfs/file.c | 8 +++-----
>>   1 file changed, 3 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/ntfs/file.c b/fs/ntfs/file.c
>> index 88747217ba61..6958a1469fb0 100644
>> --- a/fs/ntfs/file.c
>> +++ b/fs/ntfs/file.c
>> @@ -1153,11 +1153,9 @@ static long ntfs_fallocate(struct file *file, int mode, loff_t offset, loff_t le
>>          }
>>
>>          inode_dio_wait(vi);
>> -       if (mode & (FALLOC_FL_PUNCH_HOLE | FALLOC_FL_COLLAPSE_RANGE |
>> -                   FALLOC_FL_INSERT_RANGE)) {
>> -               filemap_invalidate_lock(vi->i_mapping);
>> -               map_locked = true;
>> -       }
>> +       /* Take invalidate_lock for all fallocate operations to prevent races */
>> +       filemap_invalidate_lock(vi->i_mapping);
>> +       map_locked = true;
> map_locked can be removed.
> Otherwise it looks good to me.
>
> Reviewed-by: Hyunchul Lee <hyc.lee@gmail.com>

> |Hi Hyunchul,||||Thanks for reviewing.
>
> You're right, since invalidate_lock is now taken unconditionally,
> map_locked is no longer needed. I'll remove it in v2.
>
> Thanks|
>
>>          switch (mode & FALLOC_FL_MODE_MASK) {
>>          case FALLOC_FL_ALLOCATE_RANGE:
>> --
>> 2.25.1
>>
>
> --
> Thanks,
> Hyunchul


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

* Re: [PATCH] ntfs: fix race between fallocate and mmap reads
  2026-08-27  2:43 ` Hyunchul Lee
  2026-08-27  3:11   ` Hongling Zeng
@ 2026-08-27  3:30   ` liubaolin
  1 sibling, 0 replies; 5+ messages in thread
From: liubaolin @ 2026-08-27  3:30 UTC (permalink / raw)
  To: Hyunchul Lee, Hongling Zeng
  Cc: linkinjeon, ntfs, linux-kernel, zhongling0719, stable



在 2026/8/27 10:43, Hyunchul Lee 写道:
> Hi Hongling,
> 
> 2026년 8월 26일 (수) 오후 2:42, Hongling Zeng <zenghongling@kylinos.cn>님이 작성:
>>
>> The fallocate implementation only takes invalidate_lock for punch hole,
>> collapse range, and insert range operations. For standard allocation modes
>> (mode == 0, FALLOC_FL_KEEP_SIZE), the lock is not held.
>>
>> During ntfs_attr_fallocate(), new clusters are mapped to the runlist via
>> ntfs_attr_map_cluster() before being zeroed by ntfs_dio_zero_range(). This
>> creates a window where concurrent mmap page faults can read uninitialized
>> disk data.
>>
>> Since mmap uses filemap_fault() which takes invalidate_lock in shared mode,
>> it can fault in pages during this window and expose old disk contents to
>> userspace. This is an information leak and data integrity issue.
>>
>> Fix by taking invalidate_lock for all fallocate operations, not just for
>> punch/collapse/insert modes. This prevents concurrent page faults from
>> accessing unzeroed clusters during the allocation window.
>>
>> Fixes: 495e90fa3348 ("ntfs: update attrib operations")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
>> ---
>>   fs/ntfs/file.c | 8 +++-----
>>   1 file changed, 3 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/ntfs/file.c b/fs/ntfs/file.c
>> index 88747217ba61..6958a1469fb0 100644
>> --- a/fs/ntfs/file.c
>> +++ b/fs/ntfs/file.c
>> @@ -1153,11 +1153,9 @@ static long ntfs_fallocate(struct file *file, int mode, loff_t offset, loff_t le
>>          }
>>
>>          inode_dio_wait(vi);
>> -       if (mode & (FALLOC_FL_PUNCH_HOLE | FALLOC_FL_COLLAPSE_RANGE |
>> -                   FALLOC_FL_INSERT_RANGE)) {
>> -               filemap_invalidate_lock(vi->i_mapping);
>> -               map_locked = true;
>> -       }
>> +       /* Take invalidate_lock for all fallocate operations to prevent races */
>> +       filemap_invalidate_lock(vi->i_mapping);
>> +       map_locked = true;
> 
> map_locked can be removed.
> Otherwise it looks good to me.
> 
> Reviewed-by: Hyunchul Lee <hyc.lee@gmail.com>
> 
> 
>>
>>          switch (mode & FALLOC_FL_MODE_MASK) {
>>          case FALLOC_FL_ALLOCATE_RANGE:
>> --
>> 2.25.1
>>
> 
> 
> --
> Thanks,
> Hyunchul

Hi Hyunchul and Hongling,

Thanks for reviewing. I think map_locked might still be needed though.

Before filemap_invalidate_lock() is taken, there's this check:

         inode_lock(vi);
         if (NInoCompressed(ni) || NInoEncrypted(ni) || 
NInoWofCompressed(ni)) {
                 err = -EOPNOTSUPP;
                 goto out;
         }

         inode_dio_wait(vi);
         filemap_invalidate_lock(vi->i_mapping);

If that goto is taken, we reach the exit path without having locked.
Without map_locked, we'd call filemap_invalidate_unlock() on a lock
we never took.

Would it make sense to either:
1. Keep map_locked as a guard for the unlock, or
2. Change that goto to inode_unlock() + direct return, so the shared
      exit path always holds the lock?

I'd appreciate your thoughts on this.

Thanks,
Baolin.


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

end of thread, other threads:[~2026-08-27  3:30 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-26  5:42 [PATCH] ntfs: fix race between fallocate and mmap reads Hongling Zeng
2026-08-26 12:09 ` liubaolin
2026-08-27  2:43 ` Hyunchul Lee
2026-08-27  3:11   ` Hongling Zeng
2026-08-27  3:30   ` liubaolin

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®