Re: [RFC PATCH] mm/truncate: fix data loss when splitting fails in truncate_inode_partial_folio()

From: Zi Yan

Date: Fri Sep 04 2026 - 15:31:52 EST


On Thu Sep 3, 2026 at 7:50 AM EDT, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@xxxxxxxxxx>
>
> 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);

?