Re: [PATCH v2] mm/truncate: fix data loss when truncating straddling large folios

[Zhang Yi]

On 9/10/2026 2:29 AM, Joanne Koong wrote:
> On Tue, Sep 8, 2026 at 11:31 PM Zhang Yi <yi.zhang@xxxxxxxxxxxxxxx> 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.
>>
>> In addition, even when both splits succeed, data can still be lost when
>> the mapping's minimum folio order (min_order) is non-zero. folio_split()
>> 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 page. The
>> original success path left start at the page-aligned head of the range
>> and set end to the exact page index of the end edge, neither of which is
>> a folio boundary in general. Either one could land inside the large
>> folio at its edge, and the truncate loop would drop that straddling
>> folio together with its valid out-of-range tail.
>>
>> For example, a 64K (order-4) folio with min_order = 2 punched from
>> offset 0 to 36K:
>>
>>   truncate_inode_pages_range()
>>     truncate_inode_partial_folio()        # same_folio == true
>>       1st split at 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 = p9                           # BUG: p9 inside [p8-p11]
>>     loop truncates ... p8-p11           # p9-p11's valid tail is lost
>>
>> Rework the contract so the caller is told the page range to discard:
>>
>>   - Return true only when a split occurred, false otherwise. This
>>     clarifies the existing confusing return value semantics.
>>
>>   - Add pgoff_t *pstart and *pend out-parameters that receive the page
>>     range fully covered by [lstart, lend] after any split (or none),
>>     i.e. the pages wholly within the range and safe to discard. They are
>>     aligned up (pstart) and down (pend) to the mapping's minimum folio
>>     order so they always fall on a folio boundary.
>>
>>   - Rename the byte-range parameters start/end to lstart/lend to avoid
>>     clashing with the new outputs and to separate byte offsets from
>>     folio indices.
>>
>> Callers in truncate_inode_pages_range() and shmem_undo_range() pass
>> &pstart for the folio at the start edge and &pend for the folio at the
>> end edge, so the truncate loop drops exactly the fully covered pages and
>> never touches a straddling folio that still holds valid out-of-range
>> data.
>>
>> Suggested-by: Brian Foster <bfoster@xxxxxxxxxx>
>> 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@xxxxxxxxxx>
> 
> Reviewed-by: Joanne Koong <joannelkoong@xxxxxxxxx>
> 
>> ---
>> v1->v2:
>>  - Export pstart as a new parameter so that the generic and shmem
>>    truncate paths don't need to recompute the start value from the
>>    return value. (Brian)
>>  - When min_order is nonzero, align [pstart, pend] to the inner
>>    boundaries of the folio to ensure they do not point into the middle
>>    of a large folio, which could otherwise cause valid data within the
>>    folio to be incorrectly cleared. (Joanne)
>>
>> v1: https://lore.kernel.org/linux-mm/20260903115018.2034541-1-yi.zhang@xxxxxxxxxxxxxxx/
>>
>>  mm/internal.h |  4 +--
>>  mm/shmem.c    | 13 +++-----
>>  mm/truncate.c | 88 +++++++++++++++++++++++++++++++++++----------------
>>  3 files changed, 67 insertions(+), 38 deletions(-)
>>

[...]

>> diff --git a/mm/truncate.c b/mm/truncate.c
>> index b58ba940be47..d88a1b159084 100644
>> --- a/mm/truncate.c
>> +++ b/mm/truncate.c
>> @@ -206,35 +206,41 @@ 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 @pstart and/or @pend are non-NULL they receive the indexes of the
>> + * page range fully covered by [lstart, lend] after any split (or none),
>> + * i.e. the range of pages that are wholly within [lstart, lend] and so safe
>> + * to discard.
> 
> I think with themin_order > 0 case, pages wholly within [lstart, lend]
> can still be excluded, so maybe worth editing this to reflect that.
> 
> Thanks,
> Joanne

Good point, I will fix this in next iteration.

Thanks,
Yi.

> 
>> + *
>> + * Return %true if the folio was split, %false otherwise.
>>   */
>> -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 *pstart, pgoff_t *pend)
>>  {
>>         loff_t pos = folio_pos(folio);
>>         size_t size = folio_size(folio);