diff mbox series

[v5,1/4] mm/truncate: align truncation boundaries to mapping minimum folio order

Message ID 20260928120833.3440834-2-yi.zhang@huaweicloud.com
State Not Applicable
Headers show
Series mm/truncate: fix data loss when truncating straddling large folios | expand

Commit Message

Zhang Yi Sept. 28, 2026, 12:08 p.m. UTC 
From: Zhang Yi <yi.zhang@huawei.com>

When the mapping has a non-zero minimum folio order (min_order),
folio_split() in truncate_inode_partial_folio() stops at min_order
instead of order 0, so the sub-folio containing a split point stays
aligned to 1 << min_order rather than to a single page. The original
boundaries in truncate_inode_pages_range() were based on page
granularity, so either boundary could land inside the min_order chunk at
its edge, and the truncation loop would drop that whole chunk, valid
out-of-range tail included.

For example, a 64K (order-4) folio with min_order = 2 (16K) punched from
offset 0 to 36K:

  split @p0 -> [p0-p3, p4-p7, p8-p15]         # non-uniform, min_order
  folio2 = p8-p15           # straddles: p8 in range, p9-p15 tail valid
  2nd split of folio2 -> [p8-p11, p12-p15]    # success
  end(old) = p9                               # BUG: p9 inside [p8-p11]
  loop truncates ... p8-p11               # p9-p11's valid tail is lost

It has gone unnoticed so far for two reasons. A non-zero min_order is
only used by filesystems with a block or sector size larger than the
page size, and those either always write back the affected range before
punching a hole or truncating, or they carry filesystem private data on
dirty folios (e.g. buffer_head), which makes filemap_release_folio()
fail and folio_split() abort with -EBUSY, so the folio is never split
and the old start/end boundaries remain valid. The bug only becomes
reachable on paths that truncate dirty large folios without prior
writeback and without filesystem private data, such as the upcoming ext4
iomap buffered I/O path.

Align both start (rounded up) and end (rounded down) to the mapping
minimum folio order so they always fall on a folio boundary.

Reported-by: Joanne Koong <joannelkoong@gmail.com>
Link: https://lore.kernel.org/linux-mm/CAJnrk1bQYUe6+1ryyJur5EEnZYrC+_5AYsy=OWzVRgD4202y1g@mail.gmail.com/
Fixes: e220917fa5077 ("mm: split a folio in minimum folio order chunks")
Suggested-by: Zi Yan <ziy@nvidia.com>
Reviewed-by: Jan Kara <jack@suse.cz>
Reviewed-by: Brian Foster <bfoster@redhat.com>
Reviewed-by: Zi Yan <ziy@nvidia.com>
Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
---
 mm/truncate.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

Comments

sashiko-bot@kernel.org Sept. 28, 2026, 12:31 p.m. UTC | #1 
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] mm: integer overflow in round_up() during truncation causes complete file data loss
--

commit 583804d300760dcf60e3ec5aab6503a560ad3e7e
Author: Zhang Yi <yi.zhang@huawei.com>

mm/truncate: align truncation boundaries to mapping minimum folio order

When the mapping has a non-zero minimum folio order, this patch changes
folio_split() in truncate_inode_partial_folio() to stop at min_order instead
of order 0. It also aligns both start and end to the mapping minimum folio
order so they always fall on a folio boundary, preventing valid out-of-range
tails from being dropped.

Link: https://lore.kernel.org/linux-mm/CAJnrk1bQYUe6+1ryyJur5EEnZYrC+_5AYsy=OWzVRgD4202y1g@mail.gmail.com/

> diff --git a/mm/truncate.c b/mm/truncate.c
> index b58ba940be474..8a28f4a212677 100644
> --- a/mm/truncate.c
> +++ b/mm/truncate.c
[ ... ]
> @@ -395,6 +397,10 @@ void truncate_inode_pages_range(struct address_space *mapping,
>  	else
>  		end = (lend + 1) >> PAGE_SHIFT;
>  
> +	start = round_up(start, min_nrpages);

[Severity: Critical]
Could this round_up() result in an integer overflow that changes a very large
start index to 0 on 32-bit architectures?

When an unprivileged user issues fallocate(PUNCH_HOLE) or truncate at an offset
near ULONG_MAX << PAGE_SHIFT (e.g., 16TB on 32-bit systems) on a filesystem with
min_nrpages > 1 (like ext4 with large folio/iomap), start approaches
ULONG_MAX (e.g., 0xFFFFFFFF on 32-bit x86).

The macro round_up(start, min_nrpages) performs 32-bit arithmetic. With
start approaching 0xFFFFFFFF, start + min_nrpages - 1 can overflow to a value
less than min_nrpages, causing round_up to evaluate to 0.

This would corrupt the start index to 0 in truncate_inode_pages_range(), and
the subsequent loop would unintentionally truncate the entire page cache from
index 0 to end, causing complete data loss of unwritten data.

> +	if (end != (pgoff_t)-1)
> +		end = round_down(end, min_nrpages);
> +
>  	folio_batch_init(&fbatch);
>  	index = start;
>  	while (index < end && find_lock_entries(mapping, &index, end - 1,
Zhang Yi Sept. 28, 2026, 12:46 p.m. UTC | #2 
On 9/28/2026 8:31 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Critical] mm: integer overflow in round_up() during truncation causes complete file data loss
> --

As mentioned in the cover litter,  this is a pre-existing issue related
to the file system that needs to be addressed separately.

Thanks,
Yi.

> 
> commit 583804d300760dcf60e3ec5aab6503a560ad3e7e
> Author: Zhang Yi <yi.zhang@huawei.com>
> 
> mm/truncate: align truncation boundaries to mapping minimum folio order
> 
> When the mapping has a non-zero minimum folio order, this patch changes
> folio_split() in truncate_inode_partial_folio() to stop at min_order instead
> of order 0. It also aligns both start and end to the mapping minimum folio
> order so they always fall on a folio boundary, preventing valid out-of-range
> tails from being dropped.
> 
> Link: https://lore.kernel.org/linux-mm/CAJnrk1bQYUe6+1ryyJur5EEnZYrC+_5AYsy=OWzVRgD4202y1g@mail.gmail.com/
> 
>> diff --git a/mm/truncate.c b/mm/truncate.c
>> index b58ba940be474..8a28f4a212677 100644
>> --- a/mm/truncate.c
>> +++ b/mm/truncate.c
> [ ... ]
>> @@ -395,6 +397,10 @@ void truncate_inode_pages_range(struct address_space *mapping,
>>  	else
>>  		end = (lend + 1) >> PAGE_SHIFT;
>>  
>> +	start = round_up(start, min_nrpages);
> 
> [Severity: Critical]
> Could this round_up() result in an integer overflow that changes a very large
> start index to 0 on 32-bit architectures?
> 
> When an unprivileged user issues fallocate(PUNCH_HOLE) or truncate at an offset
> near ULONG_MAX << PAGE_SHIFT (e.g., 16TB on 32-bit systems) on a filesystem with
> min_nrpages > 1 (like ext4 with large folio/iomap), start approaches
> ULONG_MAX (e.g., 0xFFFFFFFF on 32-bit x86).
> 
> The macro round_up(start, min_nrpages) performs 32-bit arithmetic. With
> start approaching 0xFFFFFFFF, start + min_nrpages - 1 can overflow to a value
> less than min_nrpages, causing round_up to evaluate to 0.
> 
> This would corrupt the start index to 0 in truncate_inode_pages_range(), and
> the subsequent loop would unintentionally truncate the entire page cache from
> index 0 to end, causing complete data loss of unwritten data.
> 
>> +	if (end != (pgoff_t)-1)
>> +		end = round_down(end, min_nrpages);
>> +
>>  	folio_batch_init(&fbatch);
>>  	index = start;
>>  	while (index < end && find_lock_entries(mapping, &index, end - 1,
>
diff mbox series

Patch

diff --git a/mm/truncate.c b/mm/truncate.c
index b58ba940be47..8a28f4a21267 100644
--- a/mm/truncate.c
+++ b/mm/truncate.c
@@ -345,9 +345,11 @@  long mapping_evict_folio(struct address_space *mapping, struct folio *folio)
  * @lstart: offset from which to truncate
  * @lend: offset to which to truncate (inclusive)
  *
- * Truncate the page cache, removing the pages that are between
- * specified offsets (and zeroing out partial pages
- * if lstart or lend + 1 is not page aligned).
+ * Truncate the page cache, removing the folios that are between specified
+ * offsets (and zeroing out partial folios if lstart or lend + 1 is not
+ * folio aligned).  For mappings with a non-zero minimum folio order, the
+ * boundaries are aligned inwards to 1 << min_order so the edge sub-folio
+ * straddling the range is kept.
  *
  * Truncate takes two passes - the first pass is nonblocking.  It will not
  * block on page locks and it will not block on writeback.  The second pass
@@ -366,6 +368,7 @@  long mapping_evict_folio(struct address_space *mapping, struct folio *folio)
 void truncate_inode_pages_range(struct address_space *mapping,
 				loff_t lstart, uoff_t lend)
 {
+	pgoff_t		min_nrpages = mapping_min_folio_nrpages(mapping);
 	pgoff_t		start;		/* inclusive */
 	pgoff_t		end;		/* exclusive */
 	struct folio_batch fbatch;
@@ -379,9 +382,8 @@  void truncate_inode_pages_range(struct address_space *mapping,
 		return;
 
 	/*
-	 * 'start' and 'end' always covers the range of pages to be fully
-	 * truncated. Partial pages are covered with 'partial_start' at the
-	 * start of the range and 'partial_end' at the end of the range.
+	 * 'start' and 'end' always covers the range of folios to be fully
+	 * truncated, with both boundaries aligned inwards to 1 << min_order.
 	 * Note that 'end' is exclusive while 'lend' is inclusive.
 	 */
 	start = (lstart + PAGE_SIZE - 1) >> PAGE_SHIFT;
@@ -395,6 +397,10 @@  void truncate_inode_pages_range(struct address_space *mapping,
 	else
 		end = (lend + 1) >> PAGE_SHIFT;
 
+	start = round_up(start, min_nrpages);
+	if (end != (pgoff_t)-1)
+		end = round_down(end, min_nrpages);
+
 	folio_batch_init(&fbatch);
 	index = start;
 	while (index < end && find_lock_entries(mapping, &index, end - 1,