Linux EXT4 FS development
 help / color / mirror / Atom feed
* [RFC PATCH] mm/truncate: fix data loss when splitting fails in truncate_inode_partial_folio()
@ 2026-09-03 11:50 Zhang Yi
  2026-09-03 12:08 ` sashiko-bot
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: Zhang Yi @ 2026-09-03 11:50 UTC (permalink / raw)
  To: linux-mm
  Cc: linux-fsdevel, linux-kernel, linux-ext4, akpm, david, ljs, liam,
	vbabka, rppt, surenb, mhocko, hughd, baolin.wang, willy, jack,
	ziy, bfoster, djwong, yi.zhang, yi.zhang, yizhang089, yangerkun,
	chengzhihao1, wangkefeng.wang, yukuai

From: Zhang Yi <yi.zhang@huawei.com>

truncate_inode_partial_folio() splits a large folio so that the caller's
truncate loop can drop the in-range sub-folios while keeping the
out-of-range tail. The first split at the punch start edge is
non-uniform, which leaves the sub-folio at the truncation end edge as
large as possible, this means it may still straddle the range, holding
both zeroed in-range and valid out-of-range data. The function then
attempts a second split at offset + length to isolate that tail.

If the second split fails the straddling sub-folio stays merged. The
function returned true unconditionally on all exit paths of the success
block, telling the caller it was fully handled. The caller kept its
default end and the truncate loop truncated every sub-folio below it,
including the merged straddler, discarding the valid out-of-range tail.

For example, a 4-page order-2 folio punched from offset 0 to the middle
of the last page:

  truncate_inode_pages_range()
    truncate_inode_partial_folio()      # same_folio == true
      1st split at page0 -> [p0, p1, p2-3]   # non-uniform, success
      folio2 = p2-3 # straddles: p2 zeroed, p3 tail valid
      2nd split of folio2 fails / cannot lock
      return true                       # BUG: caller keeps default end
    end = 3
    loop truncates p0, p1, p2-3        # p3's valid tail is lost

This became reachable after commit 7460b470a131 ("mm/truncate: use
folio_split() in truncate operation") replaced the atomic split_folio()
with folio_split(), whose non-uniform split can partially split a folio
and leave the end edge merged.

It has gone unnoticed because a dirty large folio normally carries the
filesystem's private data, for example buffer_head, so
filemap_release_folio() -> iomap_release_folio() returns false on a
dirty folio and folio_split() aborts with -EBUSY before any split,
leaving the straddler safely unsplit. The bug is only reachable on paths
that produce dirty large folios without filesystem private data, and it
was caught on the upcoming ext4 iomap buffered I/O path when no ifs is
attached.

Rework the contract so the caller is told where to stop instead of
silently truncating the straddler:

  - Return true only when a split occurred, false otherwise. This
    clarifies the existing confusing return value semantics.

  - Add an optional out-parameter pgoff_t *end, set to the index of the
    folio that contains @lend and must be kept by the caller's loop. It
    is only written when the folio actually straddles @lend. On the
    success path it defaults to the page index of the end edge and is
    refined to folio2->index when the second split fails to isolate the
    tail.

  - Rename the byte-range parameters start/end to lstart/lend to avoid
    clashing with the new @end output and to separate byte offsets from
    folio indices.

Callers in truncate_inode_pages_range() and shmem_undo_range() pass &end
only when the folio straddles lend. After all, no caller discards a
straddling folio anymore, the in-range cleanly-split sub-folios below it
are still dropped.

Suggested-by: Brian Foster <bfoster@redhat.com>
Link: https://lore.kernel.org/linux-fsdevel/anH-WKA1coW6wtfG@bfoster/
Fixes: 7460b470a131 ("mm/truncate: use folio_split() in truncate operation")
Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
---
 mm/internal.h |  4 ++--
 mm/shmem.c    | 12 +++++------
 mm/truncate.c | 57 ++++++++++++++++++++++++++++++---------------------
 3 files changed, 41 insertions(+), 32 deletions(-)

diff --git a/mm/internal.h b/mm/internal.h
index 68db5abd0a4c..db7d9d9fb5c9 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -627,8 +627,8 @@ unsigned find_lock_entries(struct address_space *mapping, pgoff_t *start,
 unsigned find_get_entries(struct address_space *mapping, pgoff_t *start,
 		pgoff_t end, struct folio_batch *fbatch, pgoff_t *indices);
 int truncate_inode_folio(struct address_space *mapping, struct folio *folio);
-bool truncate_inode_partial_folio(struct folio *folio, loff_t start,
-		loff_t end);
+bool truncate_inode_partial_folio(struct folio *folio, loff_t lstart,
+		loff_t lend, pgoff_t *end);
 long mapping_evict_folio(struct address_space *mapping, struct folio *folio);
 unsigned long mapping_try_invalidate(struct address_space *mapping,
 		pgoff_t start, pgoff_t end, unsigned long *nr_failed);
diff --git a/mm/shmem.c b/mm/shmem.c
index 89a1495e55f7..cc1548ff509a 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -1175,11 +1175,9 @@ static void shmem_undo_range(struct inode *inode, loff_t lstart, uoff_t lend,
 	if (folio) {
 		same_folio = lend < folio_next_pos(folio);
 		folio_mark_dirty(folio);
-		if (!truncate_inode_partial_folio(folio, lstart, lend)) {
+		if (!truncate_inode_partial_folio(folio, lstart, lend,
+						  same_folio ? &end : NULL))
 			start = folio_next_index(folio);
-			if (same_folio)
-				end = folio->index;
-		}
 		folio_unlock(folio);
 		folio_put(folio);
 		folio = NULL;
@@ -1189,8 +1187,7 @@ static void shmem_undo_range(struct inode *inode, loff_t lstart, uoff_t lend,
 		folio = shmem_get_partial_folio(inode, lend >> PAGE_SHIFT);
 	if (folio) {
 		folio_mark_dirty(folio);
-		if (!truncate_inode_partial_folio(folio, lstart, lend))
-			end = folio->index;
+		truncate_inode_partial_folio(folio, lstart, lend, &end);
 		folio_unlock(folio);
 		folio_put(folio);
 	}
@@ -1258,7 +1255,8 @@ static void shmem_undo_range(struct inode *inode, loff_t lstart, uoff_t lend,
 
 				if (!folio_test_large(folio)) {
 					truncate_inode_folio(mapping, folio);
-				} else if (truncate_inode_partial_folio(folio, lstart, lend)) {
+				} else if (truncate_inode_partial_folio(folio,
+							lstart, lend, NULL)) {
 					/*
 					 * If we split a page, reset the loop so
 					 * that we pick up the new sub pages.
diff --git a/mm/truncate.c b/mm/truncate.c
index b58ba940be47..2ebb00f6c379 100644
--- a/mm/truncate.c
+++ b/mm/truncate.c
@@ -206,15 +206,18 @@ static int folio_split_or_unmap(struct folio *folio, struct page *split_at,
 /*
  * Handle partial folios.  The folio may be entirely within the
  * range if a split has raced with us.  If not, we zero the part of the
- * folio that's within the [start, end] range, and then split the folio if
+ * folio that's within the [lstart, lend] range, and then split the folio if
  * it's large.  split_page_range() will discard pages which now lie beyond
  * i_size, and we rely on the caller to discard pages which lie within a
  * newly created hole.
  *
- * Returns false if splitting failed so the caller can avoid
- * discarding the entire folio which is stubbornly unsplit.
+ * When @end non-NULL, set to the index of the folio that contains @lend
+ * and must be kept by the caller's truncate loop.  Return %true if the
+ * folio was split, %false otherwise, in which case the folio is dropped or
+ * may still straddle the range, so the caller must not discard it.
  */
-bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
+bool truncate_inode_partial_folio(struct folio *folio, loff_t lstart,
+				  loff_t lend, pgoff_t *end)
 {
 	loff_t pos = folio_pos(folio);
 	size_t size = folio_size(folio);
@@ -222,19 +225,22 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
 	struct page *split_at, *split_at2;
 	unsigned int min_order;
 
-	if (pos < start)
-		offset = start - pos;
+	if (end && pos + size > (u64)lend)
+		*end = folio->index;
+
+	if (pos < lstart)
+		offset = lstart - pos;
 	else
 		offset = 0;
-	if (pos + size <= (u64)end)
+	if (pos + size <= (u64)lend)
 		length = size - offset;
 	else
-		length = end + 1 - pos - offset;
+		length = lend + 1 - pos - offset;
 
 	folio_wait_writeback(folio);
 	if (length == size) {
 		truncate_inode_folio(folio->mapping, folio);
-		return true;
+		return false;
 	}
 
 	/*
@@ -248,7 +254,7 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
 	if (folio_needs_release(folio))
 		folio_invalidate(folio, offset, length);
 	if (!folio_test_large(folio))
-		return true;
+		return false;
 
 	min_order = mapping_min_folio_order(folio->mapping);
 	split_at = folio_page(folio, PAGE_ALIGN_DOWN(offset) / PAGE_SIZE);
@@ -259,6 +265,10 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
 		 * for shmem truncate
 		 */
 		struct folio *folio2;
+		bool tail_isolated = true;
+
+		if (end)
+			*end = (pos + offset + length) >> PAGE_SHIFT;
 
 		if (offset + length == size)
 			goto no_split;
@@ -273,24 +283,28 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
 		if (!folio_test_large(folio2))
 			goto out;
 
-		if (!folio_trylock(folio2))
+		if (!folio_trylock(folio2)) {
+			tail_isolated = false;
 			goto out;
+		}
 
 		/* make sure folio2 is large and does not change its mapping */
 		if (folio_test_large(folio2) &&
-		    folio2->mapping == folio->mapping)
-			folio_split_or_unmap(folio2, split_at2, min_order);
+		    folio2->mapping == folio->mapping &&
+		    folio_split_or_unmap(folio2, split_at2, min_order))
+			tail_isolated = false;
 
 		folio_unlock(folio2);
 out:
+		if (!tail_isolated && end)
+			*end = folio2->index;
 		folio_put(folio2);
 no_split:
 		return true;
 	}
-	if (folio_test_dirty(folio))
-		return false;
-	truncate_inode_folio(folio->mapping, folio);
-	return true;
+	if (!folio_test_dirty(folio))
+		truncate_inode_folio(folio->mapping, folio);
+	return false;
 }
 
 /*
@@ -413,11 +427,9 @@ void truncate_inode_pages_range(struct address_space *mapping,
 	folio = __filemap_get_folio(mapping, lstart >> PAGE_SHIFT, FGP_LOCK, 0);
 	if (!IS_ERR(folio)) {
 		same_folio = lend < folio_next_pos(folio);
-		if (!truncate_inode_partial_folio(folio, lstart, lend)) {
+		if (!truncate_inode_partial_folio(folio, lstart, lend,
+						  same_folio ? &end : NULL))
 			start = folio_next_index(folio);
-			if (same_folio)
-				end = folio->index;
-		}
 		folio_unlock(folio);
 		folio_put(folio);
 		folio = NULL;
@@ -427,8 +439,7 @@ void truncate_inode_pages_range(struct address_space *mapping,
 		folio = __filemap_get_folio(mapping, lend >> PAGE_SHIFT,
 						FGP_LOCK, 0);
 		if (!IS_ERR(folio)) {
-			if (!truncate_inode_partial_folio(folio, lstart, lend))
-				end = folio->index;
+			truncate_inode_partial_folio(folio, lstart, lend, &end);
 			folio_unlock(folio);
 			folio_put(folio);
 		}
-- 
2.52.0


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

* Re: [RFC PATCH] mm/truncate: fix data loss when splitting fails in truncate_inode_partial_folio()
  2026-09-03 11:50 [RFC PATCH] mm/truncate: fix data loss when splitting fails in truncate_inode_partial_folio() Zhang Yi
@ 2026-09-03 12:08 ` sashiko-bot
  2026-09-03 19:01 ` Joanne Koong
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-09-03 12:08 UTC (permalink / raw)
  To: Zhang Yi; +Cc: tytso, linux-ext4

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing re-validation of `page_folio()` result after `folio_try_get()` enables TOCTOU race, leading to out-of-bounds page splits and kernel panics.
--

commit 34ec9be8b97f90a8067df5949df00d3bb219ade3
Author: Zhang Yi <yi.zhang@huawei.com>

mm/truncate: fix data loss when splitting fails in truncate_inode_partial_folio()

truncate_inode_partial_folio() splits a large folio so that the caller's
truncate loop can drop the in-range sub-folios while keeping the
out-of-range tail.

Link: https://lore.kernel.org/linux-fsdevel/anH-WKA1coW6wtfG@bfoster/

> diff --git a/mm/truncate.c b/mm/truncate.c
> index b58ba940be474..2ebb00f6c3793 100644
> --- a/mm/truncate.c
> +++ b/mm/truncate.c
> @@ -273,24 +283,28 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
>  		if (!folio_test_large(folio2))
>  			goto out;
>  
> -		if (!folio_trylock(folio2))
> +		if (!folio_trylock(folio2)) {
> +			tail_isolated = false;
>  			goto out;
> +		}
>  
>  		/* make sure folio2 is large and does not change its mapping */
>  		if (folio_test_large(folio2) &&
> -		    folio2->mapping == folio->mapping)
> -			folio_split_or_unmap(folio2, split_at2, min_order);
> +		    folio2->mapping == folio->mapping &&
> +		    folio_split_or_unmap(folio2, split_at2, min_order))
> +			tail_isolated = false;

[Severity: High]
Does this code have a TOCTOU race that could lead to an out-of-bounds page
split?

In truncate_inode_partial_folio() in mm/truncate.c, a speculative folio
lookup occurs just prior to this block:

	split_at2 = folio_page(folio,
			PAGE_ALIGN_DOWN(offset + length) / PAGE_SIZE);
	folio2 = page_folio(split_at2);

	if (!folio_try_get(folio2))
		goto no_split;

Without a re-validation check after folio_try_get(), couldn't a concurrent
operation (like memory reclaim or shmem readahead reallocation) change the
compound structure of folio2?

If the compound structure changes, a thread could successfully take a
reference on the new, smaller folio2. However, split_at2 would now be
outside the bounds of this new folio2.

When folio2 is later passed into folio_split_or_unmap() in the modified
code above, would this out-of-bounds split_at2 cause a kernel panic via
VM_BUG_ON_FOLIO() inside folio_split(), or corrupt adjacent page structures
if debug is disabled?

Should there be a check like if (unlikely(page_folio(split_at2) != folio2))
added after folio_try_get() to stabilize the reference?

>  
>  		folio_unlock(folio2);
>  out:

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903115018.2034541-1-yi.zhang@huaweicloud.com?part=1

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

* Re: [RFC PATCH] mm/truncate: fix data loss when splitting fails in truncate_inode_partial_folio()
  2026-09-03 11:50 [RFC PATCH] mm/truncate: fix data loss when splitting fails in truncate_inode_partial_folio() Zhang Yi
  2026-09-03 12:08 ` sashiko-bot
@ 2026-09-03 19:01 ` Joanne Koong
  2026-09-04  6:27   ` Zhang Yi
  2026-09-04  9:06 ` Zhang Yi
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 8+ messages in thread
From: Joanne Koong @ 2026-09-03 19:01 UTC (permalink / raw)
  To: Zhang Yi
  Cc: linux-mm, linux-fsdevel, linux-kernel, linux-ext4, akpm, david,
	ljs, liam, vbabka, rppt, surenb, mhocko, hughd, baolin.wang,
	willy, jack, ziy, bfoster, djwong, yi.zhang, yizhang089,
	yangerkun, chengzhihao1, wangkefeng.wang, yukuai

On Thu, Sep 3, 2026 at 5:01 AM Zhang Yi <yi.zhang@huaweicloud.com> wrote:
>
> From: Zhang Yi <yi.zhang@huawei.com>
>
> truncate_inode_partial_folio() splits a large folio so that the caller's
> truncate loop can drop the in-range sub-folios while keeping the
> out-of-range tail. The first split at the punch start edge is
> non-uniform, which leaves the sub-folio at the truncation end edge as
> large as possible, this means it may still straddle the range, holding
> both zeroed in-range and valid out-of-range data. The function then
> attempts a second split at offset + length to isolate that tail.
>
> If the second split fails the straddling sub-folio stays merged. The
> function returned true unconditionally on all exit paths of the success
> block, telling the caller it was fully handled. The caller kept its
> default end and the truncate loop truncated every sub-folio below it,
> including the merged straddler, discarding the valid out-of-range tail.
>
> For example, a 4-page order-2 folio punched from offset 0 to the middle
> of the last page:
>
>   truncate_inode_pages_range()
>     truncate_inode_partial_folio()      # same_folio == true
>       1st split at page0 -> [p0, p1, p2-3]   # non-uniform, success
>       folio2 = p2-3 # straddles: p2 zeroed, p3 tail valid
>       2nd split of folio2 fails / cannot lock
>       return true                       # BUG: caller keeps default end
>     end = 3
>     loop truncates p0, p1, p2-3        # p3's valid tail is lost
>
> This became reachable after commit 7460b470a131 ("mm/truncate: use
> folio_split() in truncate operation") replaced the atomic split_folio()
> with folio_split(), whose non-uniform split can partially split a folio
> and leave the end edge merged.
>
> It has gone unnoticed because a dirty large folio normally carries the
> filesystem's private data, for example buffer_head, so
> filemap_release_folio() -> iomap_release_folio() returns false on a
> dirty folio and folio_split() aborts with -EBUSY before any split,
> leaving the straddler safely unsplit. The bug is only reachable on paths
> that produce dirty large folios without filesystem private data, and it
> was caught on the upcoming ext4 iomap buffered I/O path when no ifs is
> attached.
>
> Rework the contract so the caller is told where to stop instead of
> silently truncating the straddler:
>
>   - Return true only when a split occurred, false otherwise. This
>     clarifies the existing confusing return value semantics.
>
>   - Add an optional out-parameter pgoff_t *end, set to the index of the
>     folio that contains @lend and must be kept by the caller's loop. It
>     is only written when the folio actually straddles @lend. On the
>     success path it defaults to the page index of the end edge and is
>     refined to folio2->index when the second split fails to isolate the
>     tail.
>
>   - Rename the byte-range parameters start/end to lstart/lend to avoid
>     clashing with the new @end output and to separate byte offsets from
>     folio indices.
>
> Callers in truncate_inode_pages_range() and shmem_undo_range() pass &end
> only when the folio straddles lend. After all, no caller discards a
> straddling folio anymore, the in-range cleanly-split sub-folios below it
> are still dropped.
>
> Suggested-by: Brian Foster <bfoster@redhat.com>
> Link: https://lore.kernel.org/linux-fsdevel/anH-WKA1coW6wtfG@bfoster/
> Fixes: 7460b470a131 ("mm/truncate: use folio_split() in truncate operation")
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
> ---
>  mm/internal.h |  4 ++--
>  mm/shmem.c    | 12 +++++------
>  mm/truncate.c | 57 ++++++++++++++++++++++++++++++---------------------
>  3 files changed, 41 insertions(+), 32 deletions(-)
>
> diff --git a/mm/truncate.c b/mm/truncate.c
> index b58ba940be47..2ebb00f6c379 100644
> --- a/mm/truncate.c
> +++ b/mm/truncate.c
> @@ -206,15 +206,18 @@ static int folio_split_or_unmap(struct folio *folio, struct page *split_at,
>  /*
>   * Handle partial folios.  The folio may be entirely within the
>   * range if a split has raced with us.  If not, we zero the part of the
> - * folio that's within the [start, end] range, and then split the folio if
> + * folio that's within the [lstart, lend] range, and then split the folio if
>   * it's large.  split_page_range() will discard pages which now lie beyond
>   * i_size, and we rely on the caller to discard pages which lie within a
>   * newly created hole.
>   *
> - * Returns false if splitting failed so the caller can avoid
> - * discarding the entire folio which is stubbornly unsplit.
> + * When @end non-NULL, set to the index of the folio that contains @lend
> + * and must be kept by the caller's truncate loop.  Return %true if the
> + * folio was split, %false otherwise, in which case the folio is dropped or
> + * may still straddle the range, so the caller must not discard it.
>   */
> -bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
> +bool truncate_inode_partial_folio(struct folio *folio, loff_t lstart,
> +                                 loff_t lend, pgoff_t *end)
>  {
>         loff_t pos = folio_pos(folio);
>         size_t size = folio_size(folio);
> @@ -222,19 +225,22 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
>         struct page *split_at, *split_at2;
>         unsigned int min_order;
>
> -       if (pos < start)
> -               offset = start - pos;
> +       if (end && pos + size > (u64)lend)
> +               *end = folio->index;
> +
> +       if (pos < lstart)
> +               offset = lstart - pos;
>         else
>                 offset = 0;
> -       if (pos + size <= (u64)end)
> +       if (pos + size <= (u64)lend)
>                 length = size - offset;
>         else
> -               length = end + 1 - pos - offset;
> +               length = lend + 1 - pos - offset;
>
>         folio_wait_writeback(folio);
>         if (length == size) {
>                 truncate_inode_folio(folio->mapping, folio);
> -               return true;
> +               return false;
>         }
>
>         /*
> @@ -248,7 +254,7 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
>         if (folio_needs_release(folio))
>                 folio_invalidate(folio, offset, length);
>         if (!folio_test_large(folio))
> -               return true;
> +               return false;
>
>         min_order = mapping_min_folio_order(folio->mapping);
>         split_at = folio_page(folio, PAGE_ALIGN_DOWN(offset) / PAGE_SIZE);
> @@ -259,6 +265,10 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
>                  * for shmem truncate
>                  */
>                 struct folio *folio2;
> +               bool tail_isolated = true;
> +
> +               if (end)
> +                       *end = (pos + offset + length) >> PAGE_SHIFT;

If I'm understanding it correctly, based on how
truncate_inode_pages_range() uses the end value (eg the "while (index
< end)" loop condition and the find_get_entries(..., end - 1, ...)),
end needs to point to the start of the folio if the tail folio from
the split is a large folio, in order to exclude that folio from then
being truncated. But with the (pos + offset + length) >> PAGE_SHIFT
calculation here, does that result in some cases in it pointing to the
middle of a large folio? Maybe some logic is needed to make sure that
it points to the start?

Thanks,
Joanne

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

* Re: [RFC PATCH] mm/truncate: fix data loss when splitting fails in truncate_inode_partial_folio()
  2026-09-03 19:01 ` Joanne Koong
@ 2026-09-04  6:27   ` Zhang Yi
  2026-09-04 17:33     ` Joanne Koong
  0 siblings, 1 reply; 8+ messages in thread
From: Zhang Yi @ 2026-09-04  6:27 UTC (permalink / raw)
  To: Joanne Koong
  Cc: linux-mm, linux-fsdevel, linux-kernel, linux-ext4, akpm, david,
	ljs, liam, vbabka, rppt, surenb, mhocko, hughd, baolin.wang,
	willy, jack, ziy, bfoster, djwong, yi.zhang, yizhang089,
	yangerkun, chengzhihao1, wangkefeng.wang, yukuai

On 9/4/2026 3:01 AM, Joanne Koong wrote:
> On Thu, Sep 3, 2026 at 5:01 AM Zhang Yi <yi.zhang@huaweicloud.com> wrote:
>>
>> From: Zhang Yi <yi.zhang@huawei.com>
>>
>> truncate_inode_partial_folio() splits a large folio so that the caller's
>> truncate loop can drop the in-range sub-folios while keeping the
>> out-of-range tail. The first split at the punch start edge is
>> non-uniform, which leaves the sub-folio at the truncation end edge as
>> large as possible, this means it may still straddle the range, holding
>> both zeroed in-range and valid out-of-range data. The function then
>> attempts a second split at offset + length to isolate that tail.
>>
>> If the second split fails the straddling sub-folio stays merged. The
>> function returned true unconditionally on all exit paths of the success
>> block, telling the caller it was fully handled. The caller kept its
>> default end and the truncate loop truncated every sub-folio below it,
>> including the merged straddler, discarding the valid out-of-range tail.
>>
>> For example, a 4-page order-2 folio punched from offset 0 to the middle
>> of the last page:
>>
>>   truncate_inode_pages_range()
>>     truncate_inode_partial_folio()      # same_folio == true
>>       1st split at page0 -> [p0, p1, p2-3]   # non-uniform, success
>>       folio2 = p2-3 # straddles: p2 zeroed, p3 tail valid
>>       2nd split of folio2 fails / cannot lock
>>       return true                       # BUG: caller keeps default end
>>     end = 3
>>     loop truncates p0, p1, p2-3        # p3's valid tail is lost
>>
>> This became reachable after commit 7460b470a131 ("mm/truncate: use
>> folio_split() in truncate operation") replaced the atomic split_folio()
>> with folio_split(), whose non-uniform split can partially split a folio
>> and leave the end edge merged.
>>
>> It has gone unnoticed because a dirty large folio normally carries the
>> filesystem's private data, for example buffer_head, so
>> filemap_release_folio() -> iomap_release_folio() returns false on a
>> dirty folio and folio_split() aborts with -EBUSY before any split,
>> leaving the straddler safely unsplit. The bug is only reachable on paths
>> that produce dirty large folios without filesystem private data, and it
>> was caught on the upcoming ext4 iomap buffered I/O path when no ifs is
>> attached.
>>
>> Rework the contract so the caller is told where to stop instead of
>> silently truncating the straddler:
>>
>>   - Return true only when a split occurred, false otherwise. This
>>     clarifies the existing confusing return value semantics.
>>
>>   - Add an optional out-parameter pgoff_t *end, set to the index of the
>>     folio that contains @lend and must be kept by the caller's loop. It
>>     is only written when the folio actually straddles @lend. On the
>>     success path it defaults to the page index of the end edge and is
>>     refined to folio2->index when the second split fails to isolate the
>>     tail.
>>
>>   - Rename the byte-range parameters start/end to lstart/lend to avoid
>>     clashing with the new @end output and to separate byte offsets from
>>     folio indices.
>>
>> Callers in truncate_inode_pages_range() and shmem_undo_range() pass &end
>> only when the folio straddles lend. After all, no caller discards a
>> straddling folio anymore, the in-range cleanly-split sub-folios below it
>> are still dropped.
>>
>> Suggested-by: Brian Foster <bfoster@redhat.com>
>> Link: https://lore.kernel.org/linux-fsdevel/anH-WKA1coW6wtfG@bfoster/
>> Fixes: 7460b470a131 ("mm/truncate: use folio_split() in truncate operation")
>> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
>> ---
>>  mm/internal.h |  4 ++--
>>  mm/shmem.c    | 12 +++++------
>>  mm/truncate.c | 57 ++++++++++++++++++++++++++++++---------------------
>>  3 files changed, 41 insertions(+), 32 deletions(-)
>>
>> diff --git a/mm/truncate.c b/mm/truncate.c
>> index b58ba940be47..2ebb00f6c379 100644
>> --- a/mm/truncate.c
>> +++ b/mm/truncate.c
>> @@ -206,15 +206,18 @@ static int folio_split_or_unmap(struct folio *folio, struct page *split_at,
>>  /*
>>   * Handle partial folios.  The folio may be entirely within the
>>   * range if a split has raced with us.  If not, we zero the part of the
>> - * folio that's within the [start, end] range, and then split the folio if
>> + * folio that's within the [lstart, lend] range, and then split the folio if
>>   * it's large.  split_page_range() will discard pages which now lie beyond
>>   * i_size, and we rely on the caller to discard pages which lie within a
>>   * newly created hole.
>>   *
>> - * Returns false if splitting failed so the caller can avoid
>> - * discarding the entire folio which is stubbornly unsplit.
>> + * When @end non-NULL, set to the index of the folio that contains @lend
>> + * and must be kept by the caller's truncate loop.  Return %true if the
>> + * folio was split, %false otherwise, in which case the folio is dropped or
>> + * may still straddle the range, so the caller must not discard it.
>>   */
>> -bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
>> +bool truncate_inode_partial_folio(struct folio *folio, loff_t lstart,
>> +                                 loff_t lend, pgoff_t *end)
>>  {
>>         loff_t pos = folio_pos(folio);
>>         size_t size = folio_size(folio);
>> @@ -222,19 +225,22 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
>>         struct page *split_at, *split_at2;
>>         unsigned int min_order;
>>
>> -       if (pos < start)
>> -               offset = start - pos;
>> +       if (end && pos + size > (u64)lend)
>> +               *end = folio->index;
>> +
>> +       if (pos < lstart)
>> +               offset = lstart - pos;
>>         else
>>                 offset = 0;
>> -       if (pos + size <= (u64)end)
>> +       if (pos + size <= (u64)lend)
>>                 length = size - offset;
>>         else
>> -               length = end + 1 - pos - offset;
>> +               length = lend + 1 - pos - offset;
>>
>>         folio_wait_writeback(folio);
>>         if (length == size) {
>>                 truncate_inode_folio(folio->mapping, folio);
>> -               return true;
>> +               return false;
>>         }
>>
>>         /*
>> @@ -248,7 +254,7 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
>>         if (folio_needs_release(folio))
>>                 folio_invalidate(folio, offset, length);
>>         if (!folio_test_large(folio))
>> -               return true;
>> +               return false;
>>
>>         min_order = mapping_min_folio_order(folio->mapping);
>>         split_at = folio_page(folio, PAGE_ALIGN_DOWN(offset) / PAGE_SIZE);
>> @@ -259,6 +265,10 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
>>                  * for shmem truncate
>>                  */
>>                 struct folio *folio2;
>> +               bool tail_isolated = true;
>> +
>> +               if (end)
>> +                       *end = (pos + offset + length) >> PAGE_SHIFT;
> 
> If I'm understanding it correctly, based on how
> truncate_inode_pages_range() uses the end value (eg the "while (index
> < end)" loop condition and the find_get_entries(..., end - 1, ...)),
> end needs to point to the start of the folio if the tail folio from
> the split is a large folio, in order to exclude that folio from then
> being truncated. But with the (pos + offset + length) >> PAGE_SHIFT
> calculation here, does that result in some cases in it pointing to the
> middle of a large folio? Maybe some logic is needed to make sure that
> it points to the start?
> 

Hi Joanne,

Thanks for your careful review! I don't think that case can actually
occur. Let me walk through the scenarios where the
(pos + offset + length) >> PAGE_SHIFT calculation is kept as the final
end value:

1) offset + length == size:
   The truncate range aligns exactly with the folio boundary, so
   (pos + offset + length) >> PAGE_SHIFT points to the start of the next
   folio, not the middle of one.

2) !folio_try_get(folio2):
   The folio at that position has already been freed or is being freed,
   so there is no folio in the page cache at that location. The
   subsequent find_get_entries() won't find anything there.

3) !folio_test_large(folio2):
   folio2 is no longer large, likely split to order-0 by a concurrent
   operation. For an order-0 folio, the page index and folio index are
   the same, so the calculation is correct.

4) folio2 becomes stale:
   The same to case 2), folio2 is removed from the address space, so
   there is no large folio straddling the boundary that needs
   protection. The caller won't get folio from here through
   find_get_entries(). Using the page index is safe here.

If a large folio straddles the boundary at (offset + length), we will
successfully get a reference via folio_try_get(folio2) and
folio_test_large(folio2) will be true. In that case, if the split fails
(cannot lock or split operation fails), we set tail_isolated to false
and set *end = folio2->index to point to the start of that large folio
since tail_isolated.

The page index from this setting is only kept when no large folio exists
at the boundary, which makes it safe to use. What is particularly
noteworthy is that for cases 2 and 4 above, aside from setting it to
(pos + offset + length) >> PAGE_SHIFT, there does not seem to be any
better alternative.

Does this make sense or am I missing something?

Thanks,
Yi.



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

* Re: [RFC PATCH] mm/truncate: fix data loss when splitting fails in truncate_inode_partial_folio()
  2026-09-03 11:50 [RFC PATCH] mm/truncate: fix data loss when splitting fails in truncate_inode_partial_folio() Zhang Yi
  2026-09-03 12:08 ` sashiko-bot
  2026-09-03 19:01 ` Joanne Koong
@ 2026-09-04  9:06 ` Zhang Yi
  2026-09-04 18:03 ` Brian Foster
  2026-09-04 19:31 ` Zi Yan
  4 siblings, 0 replies; 8+ messages in thread
From: Zhang Yi @ 2026-09-04  9:06 UTC (permalink / raw)
  To: linux-mm
  Cc: linux-fsdevel, linux-kernel, linux-ext4, akpm, david, ljs, liam,
	vbabka, rppt, surenb, mhocko, hughd, baolin.wang, willy, jack,
	ziy, bfoster, djwong, yi.zhang, yizhang089, yangerkun,
	chengzhihao1, wangkefeng.wang, yukuai

On 9/3/2026 7:50 PM, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
> 
> truncate_inode_partial_folio() splits a large folio so that the caller's
> truncate loop can drop the in-range sub-folios while keeping the
> out-of-range tail. The first split at the punch start edge is
> non-uniform, which leaves the sub-folio at the truncation end edge as
> large as possible, this means it may still straddle the range, holding
> both zeroed in-range and valid out-of-range data. The function then
> attempts a second split at offset + length to isolate that tail.
> 
> If the second split fails the straddling sub-folio stays merged. The
> function returned true unconditionally on all exit paths of the success
> block, telling the caller it was fully handled. The caller kept its
> default end and the truncate loop truncated every sub-folio below it,
> including the merged straddler, discarding the valid out-of-range tail.
> 
> For example, a 4-page order-2 folio punched from offset 0 to the middle
> of the last page:
> 
>   truncate_inode_pages_range()
>     truncate_inode_partial_folio()      # same_folio == true
>       1st split at page0 -> [p0, p1, p2-3]   # non-uniform, success
>       folio2 = p2-3 # straddles: p2 zeroed, p3 tail valid
>       2nd split of folio2 fails / cannot lock
>       return true                       # BUG: caller keeps default end
>     end = 3
>     loop truncates p0, p1, p2-3        # p3's valid tail is lost
> 
> This became reachable after commit 7460b470a131 ("mm/truncate: use
> folio_split() in truncate operation") replaced the atomic split_folio()
> with folio_split(), whose non-uniform split can partially split a folio
> and leave the end edge merged.
> 
> It has gone unnoticed because a dirty large folio normally carries the
> filesystem's private data, for example buffer_head, so
> filemap_release_folio() -> iomap_release_folio() returns false on a
> dirty folio and folio_split() aborts with -EBUSY before any split,
> leaving the straddler safely unsplit. The bug is only reachable on paths
> that produce dirty large folios without filesystem private data, and it
> was caught on the upcoming ext4 iomap buffered I/O path when no ifs is
> attached.
> 
> Rework the contract so the caller is told where to stop instead of
> silently truncating the straddler:
> 
>   - Return true only when a split occurred, false otherwise. This
>     clarifies the existing confusing return value semantics.
> 
>   - Add an optional out-parameter pgoff_t *end, set to the index of the
>     folio that contains @lend and must be kept by the caller's loop. It
>     is only written when the folio actually straddles @lend. On the
>     success path it defaults to the page index of the end edge and is
>     refined to folio2->index when the second split fails to isolate the
>     tail.
> 
>   - Rename the byte-range parameters start/end to lstart/lend to avoid
>     clashing with the new @end output and to separate byte offsets from
>     folio indices.
> 
> Callers in truncate_inode_pages_range() and shmem_undo_range() pass &end
> only when the folio straddles lend. After all, no caller discards a
> straddling folio anymore, the in-range cleanly-split sub-folios below it
> are still dropped.
> 
> Suggested-by: Brian Foster <bfoster@redhat.com>
> Link: https://lore.kernel.org/linux-fsdevel/anH-WKA1coW6wtfG@bfoster/
> Fixes: 7460b470a131 ("mm/truncate: use folio_split() in truncate operation")
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
> ---
>  mm/internal.h |  4 ++--
>  mm/shmem.c    | 12 +++++------
>  mm/truncate.c | 57 ++++++++++++++++++++++++++++++---------------------
>  3 files changed, 41 insertions(+), 32 deletions(-)
> 
> diff --git a/mm/internal.h b/mm/internal.h
> index 68db5abd0a4c..db7d9d9fb5c9 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -627,8 +627,8 @@ unsigned find_lock_entries(struct address_space *mapping, pgoff_t *start,
>  unsigned find_get_entries(struct address_space *mapping, pgoff_t *start,
>  		pgoff_t end, struct folio_batch *fbatch, pgoff_t *indices);
>  int truncate_inode_folio(struct address_space *mapping, struct folio *folio);
> -bool truncate_inode_partial_folio(struct folio *folio, loff_t start,
> -		loff_t end);
> +bool truncate_inode_partial_folio(struct folio *folio, loff_t lstart,
> +		loff_t lend, pgoff_t *end);
>  long mapping_evict_folio(struct address_space *mapping, struct folio *folio);
>  unsigned long mapping_try_invalidate(struct address_space *mapping,
>  		pgoff_t start, pgoff_t end, unsigned long *nr_failed);
> diff --git a/mm/shmem.c b/mm/shmem.c
> index 89a1495e55f7..cc1548ff509a 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -1175,11 +1175,9 @@ static void shmem_undo_range(struct inode *inode, loff_t lstart, uoff_t lend,
>  	if (folio) {
>  		same_folio = lend < folio_next_pos(folio);
>  		folio_mark_dirty(folio);
> -		if (!truncate_inode_partial_folio(folio, lstart, lend)) {
> +		if (!truncate_inode_partial_folio(folio, lstart, lend,
> +						  same_folio ? &end : NULL))
>  			start = folio_next_index(folio);
> -			if (same_folio)
> -				end = folio->index;
> -		}
>  		folio_unlock(folio);
>  		folio_put(folio);
>  		folio = NULL;
> @@ -1189,8 +1187,7 @@ static void shmem_undo_range(struct inode *inode, loff_t lstart, uoff_t lend,
>  		folio = shmem_get_partial_folio(inode, lend >> PAGE_SHIFT);
>  	if (folio) {
>  		folio_mark_dirty(folio);
> -		if (!truncate_inode_partial_folio(folio, lstart, lend))
> -			end = folio->index;
> +		truncate_inode_partial_folio(folio, lstart, lend, &end);
>  		folio_unlock(folio);
>  		folio_put(folio);
>  	}
> @@ -1258,7 +1255,8 @@ static void shmem_undo_range(struct inode *inode, loff_t lstart, uoff_t lend,
>  
>  				if (!folio_test_large(folio)) {
>  					truncate_inode_folio(mapping, folio);
> -				} else if (truncate_inode_partial_folio(folio, lstart, lend)) {
> +				} else if (truncate_inode_partial_folio(folio,
> +							lstart, lend, NULL)) {
>  					/*
>  					 * If we split a page, reset the loop so
>  					 * that we pick up the new sub pages.
> diff --git a/mm/truncate.c b/mm/truncate.c
> index b58ba940be47..2ebb00f6c379 100644
> --- a/mm/truncate.c
> +++ b/mm/truncate.c
> @@ -206,15 +206,18 @@ static int folio_split_or_unmap(struct folio *folio, struct page *split_at,
>  /*
>   * Handle partial folios.  The folio may be entirely within the
>   * range if a split has raced with us.  If not, we zero the part of the
> - * folio that's within the [start, end] range, and then split the folio if
> + * folio that's within the [lstart, lend] range, and then split the folio if
>   * it's large.  split_page_range() will discard pages which now lie beyond
>   * i_size, and we rely on the caller to discard pages which lie within a
>   * newly created hole.
>   *
> - * Returns false if splitting failed so the caller can avoid
> - * discarding the entire folio which is stubbornly unsplit.
> + * When @end non-NULL, set to the index of the folio that contains @lend
> + * and must be kept by the caller's truncate loop.  Return %true if the
> + * folio was split, %false otherwise, in which case the folio is dropped or
> + * may still straddle the range, so the caller must not discard it.
>   */
> -bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
> +bool truncate_inode_partial_folio(struct folio *folio, loff_t lstart,
> +				  loff_t lend, pgoff_t *end)
>  {
>  	loff_t pos = folio_pos(folio);
>  	size_t size = folio_size(folio);
> @@ -222,19 +225,22 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
>  	struct page *split_at, *split_at2;
>  	unsigned int min_order;
>  
> -	if (pos < start)
> -		offset = start - pos;
> +	if (end && pos + size > (u64)lend)
> +		*end = folio->index;
> +
> +	if (pos < lstart)
> +		offset = lstart - pos;
>  	else
>  		offset = 0;
> -	if (pos + size <= (u64)end)
> +	if (pos + size <= (u64)lend)
>  		length = size - offset;
>  	else
> -		length = end + 1 - pos - offset;
> +		length = lend + 1 - pos - offset;
>  
>  	folio_wait_writeback(folio);
>  	if (length == size) {
>  		truncate_inode_folio(folio->mapping, folio);
> -		return true;
> +		return false;
>  	}
>  
>  	/*
> @@ -248,7 +254,7 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
>  	if (folio_needs_release(folio))
>  		folio_invalidate(folio, offset, length);
>  	if (!folio_test_large(folio))
> -		return true;
> +		return false;
>  
>  	min_order = mapping_min_folio_order(folio->mapping);
>  	split_at = folio_page(folio, PAGE_ALIGN_DOWN(offset) / PAGE_SIZE);
> @@ -259,6 +265,10 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
>  		 * for shmem truncate
>  		 */
>  		struct folio *folio2;
> +		bool tail_isolated = true;
> +
> +		if (end)
> +			*end = (pos + offset + length) >> PAGE_SHIFT;
>  
>  		if (offset + length == size)
>  			goto no_split;
> @@ -273,24 +283,28 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
>  		if (!folio_test_large(folio2))
>  			goto out;
>  
> -		if (!folio_trylock(folio2))
> +		if (!folio_trylock(folio2)) {
> +			tail_isolated = false;
>  			goto out;
> +		}
>  
>  		/* make sure folio2 is large and does not change its mapping */
>  		if (folio_test_large(folio2) &&
> -		    folio2->mapping == folio->mapping)
> -			folio_split_or_unmap(folio2, split_at2, min_order);
> +		    folio2->mapping == folio->mapping &&
> +		    folio_split_or_unmap(folio2, split_at2, min_order))
> +			tail_isolated = false;

Sashiko pointed out:

> [Severity: High]
> Does this code have a TOCTOU race that could lead to an out-of-bounds page
> split?
>
> In truncate_inode_partial_folio() in mm/truncate.c, a speculative folio
> lookup occurs just prior to this block:
>
> 	split_at2 = folio_page(folio,
> 			PAGE_ALIGN_DOWN(offset + length) / PAGE_SIZE);
> 	folio2 = page_folio(split_at2);
>
> 	if (!folio_try_get(folio2))
> 		goto no_split;
>
> Without a re-validation check after folio_try_get(), couldn't a concurrent
> operation (like memory reclaim or shmem readahead reallocation) change the
> compound structure of folio2?
>
> If the compound structure changes, a thread could successfully take a
> reference on the new, smaller folio2. However, split_at2 would now be
> outside the bounds of this new folio2.
>
> When folio2 is later passed into folio_split_or_unmap() in the modified
> code above, would this out-of-bounds split_at2 cause a kernel panic via
> VM_BUG_ON_FOLIO() inside folio_split(), or corrupt adjacent page structures
> if debug is disabled?
>
> Should there be a check like if (unlikely(page_folio(split_at2) != folio2))
> added after folio_try_get() to stabilize the reference?

I think this is right. This is a pre-existing issue in the current code.
There is a TOCTOU window between page_folio() and folio_try_get(). A
concurrent split could change folio2's compound structure, making
split_at2 point outside the new folio2's bounds. This would cause an
out-of-bounds split when we later call folio_split_or_unmap().

However, folio_try_get() alone isn't sufficient to close this window. We
need to re-validate after acquiring folio2's lock, since I think the
folio could still be split between folio_try_get() and folio_trylock().

I can send a separate fix patch to address this issue prior to this one.

Thanks,
Yi.

>  
>  		folio_unlock(folio2);
>  out:
> +		if (!tail_isolated && end)
> +			*end = folio2->index;
>  		folio_put(folio2);
>  no_split:
>  		return true;
>  	}
> -	if (folio_test_dirty(folio))
> -		return false;
> -	truncate_inode_folio(folio->mapping, folio);
> -	return true;
> +	if (!folio_test_dirty(folio))
> +		truncate_inode_folio(folio->mapping, folio);
> +	return false;
>  }
>  
>  /*
> @@ -413,11 +427,9 @@ void truncate_inode_pages_range(struct address_space *mapping,
>  	folio = __filemap_get_folio(mapping, lstart >> PAGE_SHIFT, FGP_LOCK, 0);
>  	if (!IS_ERR(folio)) {
>  		same_folio = lend < folio_next_pos(folio);
> -		if (!truncate_inode_partial_folio(folio, lstart, lend)) {
> +		if (!truncate_inode_partial_folio(folio, lstart, lend,
> +						  same_folio ? &end : NULL))
>  			start = folio_next_index(folio);
> -			if (same_folio)
> -				end = folio->index;
> -		}
>  		folio_unlock(folio);
>  		folio_put(folio);
>  		folio = NULL;
> @@ -427,8 +439,7 @@ void truncate_inode_pages_range(struct address_space *mapping,
>  		folio = __filemap_get_folio(mapping, lend >> PAGE_SHIFT,
>  						FGP_LOCK, 0);
>  		if (!IS_ERR(folio)) {
> -			if (!truncate_inode_partial_folio(folio, lstart, lend))
> -				end = folio->index;
> +			truncate_inode_partial_folio(folio, lstart, lend, &end);
>  			folio_unlock(folio);
>  			folio_put(folio);
>  		}


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

* Re: [RFC PATCH] mm/truncate: fix data loss when splitting fails in truncate_inode_partial_folio()
  2026-09-04  6:27   ` Zhang Yi
@ 2026-09-04 17:33     ` Joanne Koong
  0 siblings, 0 replies; 8+ messages in thread
From: Joanne Koong @ 2026-09-04 17:33 UTC (permalink / raw)
  To: Zhang Yi
  Cc: linux-mm, linux-fsdevel, linux-kernel, linux-ext4, akpm, david,
	ljs, liam, vbabka, rppt, surenb, mhocko, hughd, baolin.wang,
	willy, jack, ziy, bfoster, djwong, yi.zhang, yizhang089,
	yangerkun, chengzhihao1, wangkefeng.wang, yukuai

On Thu, Sep 3, 2026 at 11:27 PM Zhang Yi <yi.zhang@huaweicloud.com> wrote:
>
> On 9/4/2026 3:01 AM, Joanne Koong wrote:
> >>         /*
> >> @@ -259,6 +265,10 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
> >>                  * for shmem truncate
> >>                  */
> >>                 struct folio *folio2;
> >> +               bool tail_isolated = true;
> >> +
> >> +               if (end)
> >> +                       *end = (pos + offset + length) >> PAGE_SHIFT;
> >
> > If I'm understanding it correctly, based on how
> > truncate_inode_pages_range() uses the end value (eg the "while (index
> > < end)" loop condition and the find_get_entries(..., end - 1, ...)),
> > end needs to point to the start of the folio if the tail folio from
> > the split is a large folio, in order to exclude that folio from then
> > being truncated. But with the (pos + offset + length) >> PAGE_SHIFT
> > calculation here, does that result in some cases in it pointing to the
> > middle of a large folio? Maybe some logic is needed to make sure that
> > it points to the start?
> >
>
> Hi Joanne,
>
> Thanks for your careful review! I don't think that case can actually
> occur. Let me walk through the scenarios where the
> (pos + offset + length) >> PAGE_SHIFT calculation is kept as the final
> end value:

Hi Yi,

Thanks for your reply and for walking through the logic and explaining it.

>
> 1) offset + length == size:
>    The truncate range aligns exactly with the folio boundary, so
>    (pos + offset + length) >> PAGE_SHIFT points to the start of the next
>    folio, not the middle of one.
>
> 2) !folio_try_get(folio2):
>    The folio at that position has already been freed or is being freed,
>    so there is no folio in the page cache at that location. The
>    subsequent find_get_entries() won't find anything there.
>
> 3) !folio_test_large(folio2):
>    folio2 is no longer large, likely split to order-0 by a concurrent
>    operation. For an order-0 folio, the page index and folio index are
>    the same, so the calculation is correct.
>
> 4) folio2 becomes stale:
>    The same to case 2), folio2 is removed from the address space, so
>    there is no large folio straddling the boundary that needs
>    protection. The caller won't get folio from here through
>    find_get_entries(). Using the page index is safe here.
>
> If a large folio straddles the boundary at (offset + length), we will
> successfully get a reference via folio_try_get(folio2) and
> folio_test_large(folio2) will be true. In that case, if the split fails
> (cannot lock or split operation fails), we set tail_isolated to false
> and set *end = folio2->index to point to the start of that large folio
> since tail_isolated.

The case I have in mind is the case where the 2nd split succeeds
(returns 0) and tail_isolated will not be set to false, and *end still
gets returned back to the caller as the original "(pos + offset +
length) >> PAGE_SHIFT" calculation. I don't think it's guaranteed that
the split will create a folio that starts at that page.

The example I'm thinking about is a 64k folio being truncated at
offset 0 to 36k on a 16k blocksize filesystem with 4k pages:
*end = (pos + offset + length) >> PAGE_SHIFT = 36k >> PAGE_SHIFT = page index 9
start: [0 - 15]
after 1st split: [0 - 3] [4 - 7] [8 - 15]
split_at2 = 36k / PAGE_SIZE = 9
2nd split will try splitting [8 - 15] at split_at2
after 2nd split: [8 - 11] [12 - 15]

in the truncate_inode_pages_range() logic, start = 0, end = 9, so
find_get_entries(..., end - 1 (= 8) ...) returns the [8 - 11] folio
and truncate_inode_folio() will drop all 4 of those pages (including
36k to 48k which might have dirty data).

Do you think this makes sense or am I missing something?

>
> The page index from this setting is only kept when no large folio exists
> at the boundary, which makes it safe to use. What is particularly
> noteworthy is that for cases 2 and 4 above, aside from setting it to
> (pos + offset + length) >> PAGE_SHIFT, there does not seem to be any
> better alternative.

I think if we just rounded down *end by the min order (eg *end =
round_down((pos + offset + length) >> PAGE_SHIFT, 1UL << min_order);),
that would ensure end is always on a folio boundary and can't be
inside a folio.

Thanks,
Joanne

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

* Re: [RFC PATCH] mm/truncate: fix data loss when splitting fails in truncate_inode_partial_folio()
  2026-09-03 11:50 [RFC PATCH] mm/truncate: fix data loss when splitting fails in truncate_inode_partial_folio() Zhang Yi
                   ` (2 preceding siblings ...)
  2026-09-04  9:06 ` Zhang Yi
@ 2026-09-04 18:03 ` Brian Foster
  2026-09-04 19:31 ` Zi Yan
  4 siblings, 0 replies; 8+ messages in thread
From: Brian Foster @ 2026-09-04 18:03 UTC (permalink / raw)
  To: Zhang Yi
  Cc: linux-mm, linux-fsdevel, linux-kernel, linux-ext4, akpm, david,
	ljs, liam, vbabka, rppt, surenb, mhocko, hughd, baolin.wang,
	willy, jack, ziy, djwong, yi.zhang, yizhang089, yangerkun,
	chengzhihao1, wangkefeng.wang, yukuai

On Thu, Sep 03, 2026 at 07:50:18PM +0800, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
> 
> truncate_inode_partial_folio() splits a large folio so that the caller's
> truncate loop can drop the in-range sub-folios while keeping the
> out-of-range tail. The first split at the punch start edge is
> non-uniform, which leaves the sub-folio at the truncation end edge as
> large as possible, this means it may still straddle the range, holding
> both zeroed in-range and valid out-of-range data. The function then
> attempts a second split at offset + length to isolate that tail.
> 
> If the second split fails the straddling sub-folio stays merged. The
> function returned true unconditionally on all exit paths of the success
> block, telling the caller it was fully handled. The caller kept its
> default end and the truncate loop truncated every sub-folio below it,
> including the merged straddler, discarding the valid out-of-range tail.
> 
> For example, a 4-page order-2 folio punched from offset 0 to the middle
> of the last page:
> 
>   truncate_inode_pages_range()
>     truncate_inode_partial_folio()      # same_folio == true
>       1st split at page0 -> [p0, p1, p2-3]   # non-uniform, success
>       folio2 = p2-3 # straddles: p2 zeroed, p3 tail valid
>       2nd split of folio2 fails / cannot lock
>       return true                       # BUG: caller keeps default end
>     end = 3
>     loop truncates p0, p1, p2-3        # p3's valid tail is lost
> 
> This became reachable after commit 7460b470a131 ("mm/truncate: use
> folio_split() in truncate operation") replaced the atomic split_folio()
> with folio_split(), whose non-uniform split can partially split a folio
> and leave the end edge merged.
> 
> It has gone unnoticed because a dirty large folio normally carries the
> filesystem's private data, for example buffer_head, so
> filemap_release_folio() -> iomap_release_folio() returns false on a
> dirty folio and folio_split() aborts with -EBUSY before any split,
> leaving the straddler safely unsplit. The bug is only reachable on paths
> that produce dirty large folios without filesystem private data, and it
> was caught on the upcoming ext4 iomap buffered I/O path when no ifs is
> attached.
> 
> Rework the contract so the caller is told where to stop instead of
> silently truncating the straddler:
> 
>   - Return true only when a split occurred, false otherwise. This
>     clarifies the existing confusing return value semantics.
> 
>   - Add an optional out-parameter pgoff_t *end, set to the index of the
>     folio that contains @lend and must be kept by the caller's loop. It
>     is only written when the folio actually straddles @lend. On the
>     success path it defaults to the page index of the end edge and is
>     refined to folio2->index when the second split fails to isolate the
>     tail.
> 
>   - Rename the byte-range parameters start/end to lstart/lend to avoid
>     clashing with the new @end output and to separate byte offsets from
>     folio indices.
> 
> Callers in truncate_inode_pages_range() and shmem_undo_range() pass &end
> only when the folio straddles lend. After all, no caller discards a
> straddling folio anymore, the in-range cleanly-split sub-folios below it
> are still dropped.
> 
> Suggested-by: Brian Foster <bfoster@redhat.com>
> Link: https://lore.kernel.org/linux-fsdevel/anH-WKA1coW6wtfG@bfoster/
> Fixes: 7460b470a131 ("mm/truncate: use folio_split() in truncate operation")
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
> ---

Thanks for the patch and description. AFAICT this looks good in terms of
I think it fixes the issue and generally otherwise preserves existing
behavior (modulo the thing Joanne is poking at that I haven't grok'd).
That said...

>  mm/internal.h |  4 ++--
>  mm/shmem.c    | 12 +++++------
>  mm/truncate.c | 57 ++++++++++++++++++++++++++++++---------------------
>  3 files changed, 41 insertions(+), 32 deletions(-)
> 
...
> diff --git a/mm/truncate.c b/mm/truncate.c
> index b58ba940be47..2ebb00f6c379 100644
> --- a/mm/truncate.c
> +++ b/mm/truncate.c
> @@ -206,15 +206,18 @@ static int folio_split_or_unmap(struct folio *folio, struct page *split_at,
>  /*
>   * Handle partial folios.  The folio may be entirely within the
>   * range if a split has raced with us.  If not, we zero the part of the
> - * folio that's within the [start, end] range, and then split the folio if
> + * folio that's within the [lstart, lend] range, and then split the folio if
>   * it's large.  split_page_range() will discard pages which now lie beyond
>   * i_size, and we rely on the caller to discard pages which lie within a
>   * newly created hole.
>   *
> - * Returns false if splitting failed so the caller can avoid
> - * discarding the entire folio which is stubbornly unsplit.
> + * When @end non-NULL, set to the index of the folio that contains @lend
> + * and must be kept by the caller's truncate loop.  Return %true if the
> + * folio was split, %false otherwise, in which case the folio is dropped or
> + * may still straddle the range, so the caller must not discard it.
>   */
> -bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
> +bool truncate_inode_partial_folio(struct folio *folio, loff_t lstart,
> +				  loff_t lend, pgoff_t *end)

... I find the interface kind of confusing and I wonder if we can come
up with something cleaner. After some time rubber ducking with $LLM,
what do you think about something where we'd have this function take
lstart/lend and always return something like pstart/pend page offsets
(instead of just end)? Those page offsets would essentially refer to the
indexes for the page range fully covered by lstart/lend, after whatever
splitting occurred (or didn't).

So in this particular example of a 4-page folio where we truncate from 0
to the middle of the last page, pstart would refer to p0 and pend to p3
in the successful case. This tells the caller that pages p0 thru p2 can
be punched out.

If the second split fails as in the commit log example, then you have
pstart == p0 and pend == p2-3. The caller (truncate_inode_pages_range())
would reflect this into start/end such that we only punch out p0 thru p1
in that case.

I think that means for the case where same_folio == false, the range to
walk in the caller would be defined by pstart of the first
partial_folio() call and pend of the second, so that code would have to
change a little bit. That said, I wonder if that means we could also
just get rid of the return value and let the pstart/pend values infer
whether splits occurred or not. I see you've already removed the need
for the return check in the second partial_folio() call. I haven't run
through the other users of this function though. Thoughts on something
like that?

Brian

>  {
>  	loff_t pos = folio_pos(folio);
>  	size_t size = folio_size(folio);
> @@ -222,19 +225,22 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
>  	struct page *split_at, *split_at2;
>  	unsigned int min_order;
>  
> -	if (pos < start)
> -		offset = start - pos;
> +	if (end && pos + size > (u64)lend)
> +		*end = folio->index;
> +
> +	if (pos < lstart)
> +		offset = lstart - pos;
>  	else
>  		offset = 0;
> -	if (pos + size <= (u64)end)
> +	if (pos + size <= (u64)lend)
>  		length = size - offset;
>  	else
> -		length = end + 1 - pos - offset;
> +		length = lend + 1 - pos - offset;
>  
>  	folio_wait_writeback(folio);
>  	if (length == size) {
>  		truncate_inode_folio(folio->mapping, folio);
> -		return true;
> +		return false;
>  	}
>  
>  	/*
> @@ -248,7 +254,7 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
>  	if (folio_needs_release(folio))
>  		folio_invalidate(folio, offset, length);
>  	if (!folio_test_large(folio))
> -		return true;
> +		return false;
>  
>  	min_order = mapping_min_folio_order(folio->mapping);
>  	split_at = folio_page(folio, PAGE_ALIGN_DOWN(offset) / PAGE_SIZE);
> @@ -259,6 +265,10 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
>  		 * for shmem truncate
>  		 */
>  		struct folio *folio2;
> +		bool tail_isolated = true;
> +
> +		if (end)
> +			*end = (pos + offset + length) >> PAGE_SHIFT;
>  
>  		if (offset + length == size)
>  			goto no_split;
> @@ -273,24 +283,28 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
>  		if (!folio_test_large(folio2))
>  			goto out;
>  
> -		if (!folio_trylock(folio2))
> +		if (!folio_trylock(folio2)) {
> +			tail_isolated = false;
>  			goto out;
> +		}
>  
>  		/* make sure folio2 is large and does not change its mapping */
>  		if (folio_test_large(folio2) &&
> -		    folio2->mapping == folio->mapping)
> -			folio_split_or_unmap(folio2, split_at2, min_order);
> +		    folio2->mapping == folio->mapping &&
> +		    folio_split_or_unmap(folio2, split_at2, min_order))
> +			tail_isolated = false;
>  
>  		folio_unlock(folio2);
>  out:
> +		if (!tail_isolated && end)
> +			*end = folio2->index;
>  		folio_put(folio2);
>  no_split:
>  		return true;
>  	}
> -	if (folio_test_dirty(folio))
> -		return false;
> -	truncate_inode_folio(folio->mapping, folio);
> -	return true;
> +	if (!folio_test_dirty(folio))
> +		truncate_inode_folio(folio->mapping, folio);
> +	return false;
>  }
>  
>  /*
> @@ -413,11 +427,9 @@ void truncate_inode_pages_range(struct address_space *mapping,
>  	folio = __filemap_get_folio(mapping, lstart >> PAGE_SHIFT, FGP_LOCK, 0);
>  	if (!IS_ERR(folio)) {
>  		same_folio = lend < folio_next_pos(folio);
> -		if (!truncate_inode_partial_folio(folio, lstart, lend)) {
> +		if (!truncate_inode_partial_folio(folio, lstart, lend,
> +						  same_folio ? &end : NULL))
>  			start = folio_next_index(folio);
> -			if (same_folio)
> -				end = folio->index;
> -		}
>  		folio_unlock(folio);
>  		folio_put(folio);
>  		folio = NULL;
> @@ -427,8 +439,7 @@ void truncate_inode_pages_range(struct address_space *mapping,
>  		folio = __filemap_get_folio(mapping, lend >> PAGE_SHIFT,
>  						FGP_LOCK, 0);
>  		if (!IS_ERR(folio)) {
> -			if (!truncate_inode_partial_folio(folio, lstart, lend))
> -				end = folio->index;
> +			truncate_inode_partial_folio(folio, lstart, lend, &end);
>  			folio_unlock(folio);
>  			folio_put(folio);
>  		}
> -- 
> 2.52.0
> 


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

* Re: [RFC PATCH] mm/truncate: fix data loss when splitting fails in truncate_inode_partial_folio()
  2026-09-03 11:50 [RFC PATCH] mm/truncate: fix data loss when splitting fails in truncate_inode_partial_folio() Zhang Yi
                   ` (3 preceding siblings ...)
  2026-09-04 18:03 ` Brian Foster
@ 2026-09-04 19:31 ` Zi Yan
  4 siblings, 0 replies; 8+ messages in thread
From: Zi Yan @ 2026-09-04 19:31 UTC (permalink / raw)
  To: Zhang Yi, linux-mm
  Cc: linux-fsdevel, linux-kernel, linux-ext4, akpm, david, ljs, liam,
	vbabka, rppt, surenb, mhocko, hughd, baolin.wang, willy, jack,
	bfoster, djwong, yi.zhang, yizhang089, yangerkun, chengzhihao1,
	wangkefeng.wang, yukuai, Joanne Koong

On Thu Sep 3, 2026 at 7:50 AM EDT, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
>
> truncate_inode_partial_folio() splits a large folio so that the caller's
> truncate loop can drop the in-range sub-folios while keeping the
> out-of-range tail. The first split at the punch start edge is
> non-uniform, which leaves the sub-folio at the truncation end edge as
> large as possible, this means it may still straddle the range, holding
> both zeroed in-range and valid out-of-range data. The function then
> attempts a second split at offset + length to isolate that tail.
>
> If the second split fails the straddling sub-folio stays merged. The
> function returned true unconditionally on all exit paths of the success
> block, telling the caller it was fully handled. The caller kept its
> default end and the truncate loop truncated every sub-folio below it,
> including the merged straddler, discarding the valid out-of-range tail.
>
> For example, a 4-page order-2 folio punched from offset 0 to the middle
> of the last page:
>
>   truncate_inode_pages_range()
>     truncate_inode_partial_folio()      # same_folio == true
>       1st split at page0 -> [p0, p1, p2-3]   # non-uniform, success
>       folio2 = p2-3 # straddles: p2 zeroed, p3 tail valid
>       2nd split of folio2 fails / cannot lock
>       return true                       # BUG: caller keeps default end
>     end = 3
>     loop truncates p0, p1, p2-3        # p3's valid tail is lost
>
> This became reachable after commit 7460b470a131 ("mm/truncate: use
> folio_split() in truncate operation") replaced the atomic split_folio()
> with folio_split(), whose non-uniform split can partially split a folio
> and leave the end edge merged.
>
> It has gone unnoticed because a dirty large folio normally carries the
> filesystem's private data, for example buffer_head, so
> filemap_release_folio() -> iomap_release_folio() returns false on a
> dirty folio and folio_split() aborts with -EBUSY before any split,
> leaving the straddler safely unsplit. The bug is only reachable on paths
> that produce dirty large folios without filesystem private data, and it
> was caught on the upcoming ext4 iomap buffered I/O path when no ifs is
> attached.

Thank you for the analysis.

>
> Rework the contract so the caller is told where to stop instead of
> silently truncating the straddler:
>
>   - Return true only when a split occurred, false otherwise. This
>     clarifies the existing confusing return value semantics.

Should we do "return false" for not split case as a minmal fix first?

Something like below. A second patch can optimize on top of it. Let me
know if I miss anything.

BTW, Claude also mentioned that if min_order > 0 and end is not aligned
to 1UL << min_order, there could be some issue. So

ret = !folio_split_or_unmap(folio2, split_at2, min_order);

should be

unsigned long idx2 = PAGE_ALIGN_DOWN(offset + length) / PAGE_SIZE;

ret = !folio_split_or_unmap(folio2, split_at2, min_order) &&
IS_ALIGNED(idx2, 1UL << min_order);

?

From 564fd753071be9d59d9e45e4609a812bb81f778a Mon Sep 17 00:00:00 2001
From: Zi Yan <ziy@nvidia.com>
Date: Fri, 4 Sep 2026 15:25:06 -0400
Subject: [PATCH] fix unsuccessful folio2 split

Signed-off-by: Zi Yan <ziy@nvidia.com>
---
 mm/truncate.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/mm/truncate.c b/mm/truncate.c
index b58ba940be474..2c575f5e61889 100644
--- a/mm/truncate.c
+++ b/mm/truncate.c
@@ -259,6 +259,7 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
 		 * for shmem truncate
 		 */
 		struct folio *folio2;
+		bool ret = true;
 
 		if (offset + length == size)
 			goto no_split;
@@ -273,19 +274,23 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
 		if (!folio_test_large(folio2))
 			goto out;
 
-		if (!folio_trylock(folio2))
+		if (!folio_trylock(folio2)) {
+			ret = false;
 			goto out;
+		}
 
 		/* make sure folio2 is large and does not change its mapping */
 		if (folio_test_large(folio2) &&
 		    folio2->mapping == folio->mapping)
-			folio_split_or_unmap(folio2, split_at2, min_order);
+			ret = !folio_split_or_unmap(folio2, split_at2, min_order);
+		else
+			ret = false;
 
 		folio_unlock(folio2);
 out:
 		folio_put(folio2);
 no_split:
-		return true;
+		return ret;
 	}
 	if (folio_test_dirty(folio))
 		return false;
-- 
2.53.0


>
>   - Add an optional out-parameter pgoff_t *end, set to the index of the
>     folio that contains @lend and must be kept by the caller's loop. It
>     is only written when the folio actually straddles @lend. On the
>     success path it defaults to the page index of the end edge and is
>     refined to folio2->index when the second split fails to isolate the
>     tail.
>
>   - Rename the byte-range parameters start/end to lstart/lend to avoid
>     clashing with the new @end output and to separate byte offsets from
>     folio indices.
>
> Callers in truncate_inode_pages_range() and shmem_undo_range() pass &end
> only when the folio straddles lend. After all, no caller discards a
> straddling folio anymore, the in-range cleanly-split sub-folios below it
> are still dropped.
>
> Suggested-by: Brian Foster <bfoster@redhat.com>
> Link: https://lore.kernel.org/linux-fsdevel/anH-WKA1coW6wtfG@bfoster/
> Fixes: 7460b470a131 ("mm/truncate: use folio_split() in truncate operation")
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
> ---
>  mm/internal.h |  4 ++--
>  mm/shmem.c    | 12 +++++------
>  mm/truncate.c | 57 ++++++++++++++++++++++++++++++---------------------
>  3 files changed, 41 insertions(+), 32 deletions(-)
>


-- 
Best Regards,
Yan, Zi


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

end of thread, other threads:[~2026-09-04 19:31 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-03 11:50 [RFC PATCH] mm/truncate: fix data loss when splitting fails in truncate_inode_partial_folio() Zhang Yi
2026-09-03 12:08 ` sashiko-bot
2026-09-03 19:01 ` Joanne Koong
2026-09-04  6:27   ` Zhang Yi
2026-09-04 17:33     ` Joanne Koong
2026-09-04  9:06 ` Zhang Yi
2026-09-04 18:03 ` Brian Foster
2026-09-04 19:31 ` Zi Yan

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