Re: [PATCH v2] mm/truncate: fix data loss when truncating straddling large folios
flat view
From: sashiko-bot@kernel.org
Date: 2026-09-09 06:43:27
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Speculative folio access without proper re-validation causes corrupted truncation bounds and massive data loss.
--
commit fca88c7c0a18efec3a5bc4b07763e9cc7f4d03f7
Author: Zhang Yi [off-list ref]
mm/truncate: fix data loss when truncating straddling large folios
This commit fixes a data loss issue where a straddling large folio
might be incorrectly truncated if the second split fails. It modifies
truncate_inode_partial_folio() to clarify return semantics and return
the safe-to-discard page range to the caller via out-parameters.
Link: https://lore.kernel.org/linux-fsdevel/anH-WKA1coW6wtfG@bfoster/ (local)
quoted hunk ↗ jump to hunk
diff --git a/mm/truncate.c b/mm/truncate.c --- a/mm/truncate.c +++ b/mm/truncate.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -259,38 +266,67 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
[ ... ]
quoted hunk ↗ jump to hunk
if (!folio_try_get(folio2)) - goto no_split; + goto split; if (!folio_test_large(folio2)) goto out; - if (!folio_trylock(folio2)) + if (!folio_trylock(folio2)) { + tail_isolated = false; goto out; + }
[Severity: Critical] Can jumping to the out label here bypass the validation of the speculatively looked-up folio? If folio2 was evicted and the physical page was reallocated to an unrelated mapping (like an anonymous THP) right before folio_try_get(folio2), folio_trylock(folio2) could fail because the new owner holds the lock.
quoted hunk ↗ jump to hunk
+ + /* + * split_at2 may no longer belong to folio2 due to concurrent + * split. Retry to find the correct folio in case it's still + * large. + */ + if (page_folio(split_at2) != folio2) { + folio_unlock(folio2); + folio_put(folio2); + goto retry; + } /* 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 && pend) + *pend = folio2->index;
[Severity: Critical] Does this assignment risk reading folio2->index from an unrelated folio? If the folio_trylock(folio2) failure above jumps here without re-validating that page_folio(split_at2) equals folio2, we could read the index of a completely different folio. If that happens, could truncate_inode_pages_range() receive a garbage, arbitrarily large index for its truncation end boundary and unintentionally drop all pages up to that index?
folio_put(folio2);
-- Sashiko AI review · https://sashiko.dev/#/patchset/20260909062339.473816-1-yi.zhang@huaweicloud.com?part=1