diff mbox series

[v4,2/4] mm/truncate: look up the end-edge straddler by index

Message ID 20260922110703.468389-3-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. 22, 2026, 11:07 a.m. UTC 
From: Zhang Yi <yi.zhang@huawei.com>

In truncate_inode_partial_folio(), after the first split at the start
edge, folio_split() unlocks and drops the refcount of the after-split
sub-folios. The sub-folio straddling the end of the truncation range is
therefore unlocked and only transiently ref'd in the page cache while
the code still derives it from a page pointer inside the original folio.

Between the first split finishing and page_folio() resolving split_at2,
that tail page can be reclaimed, freed and reallocated as a new large
folio in the same mapping at a different file offset. folio2 then points
at a folio that does not cover the end boundary, yet
folio2->mapping == folio->mapping still holds, so the stale pointer
passes the mapping check and folio_split_or_unmap() splits a folio at a
wrong position (or, with a transient refcount, a use-after-free window
opens between try_get and the split). __folio_split()'s own
folio != page_folio(split_at) check cannot catch this either since
split_at2 has been reallocated as part of the new folio, so
page_folio(split_at2) resolves back to folio2.

Look the straddler up by its page index instead. __filemap_get_folio()
returns the folio currently covering the boundary, ref'd and locked,
with the mapping validated under the lock, so the split target is always
the real folio at the end edge.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://sashiko.dev/#/message/20260916094500.C30061F00893%40smtp.kernel.org
Link: https://lore.kernel.org/linux-mm/DLGXT0ERY79Z.3C5DYVJVX6S9Z@nvidia.com/
Fixes: 7460b470a131 ("mm/truncate: use folio_split() in truncate operation")
Cc: stable@vger.kernel.org
Suggested-by: Jan Kara <jack@suse.cz>
Suggested-by: Zi Yan <ziy@nvidia.com>
Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
---
 mm/truncate.c | 30 ++++++++++++++++--------------
 1 file changed, 16 insertions(+), 14 deletions(-)

Comments

sashiko-bot@kernel.org Sept. 22, 2026, 11:23 a.m. UTC | #1 
> From: Zhang Yi <yi.zhang@huawei.com>
> 
> In truncate_inode_partial_folio(), after the first split at the start
> edge, folio_split() unlocks and drops the refcount of the after-split
> sub-folios. The sub-folio straddling the end of the truncation range is
> therefore unlocked and only transiently ref'd in the page cache while
> the code still derives it from a page pointer inside the original folio.
> 
> Between the first split finishing and page_folio() resolving split_at2,
> that tail page can be reclaimed, freed and reallocated as a new large
> folio in the same mapping at a different file offset. folio2 then points
> at a folio that does not cover the end boundary, yet
> folio2->mapping == folio->mapping still holds, so the stale pointer
> passes the mapping check and folio_split_or_unmap() splits a folio at a
> wrong position (or, with a transient refcount, a use-after-free window
> [ ... ]
> Fixes: 7460b470a131 ("mm/truncate: use folio_split() in truncate operation")
> Cc: stable@vger.kernel.org
> Suggested-by: Jan Kara <jack@suse.cz>
> Suggested-by: Zi Yan <ziy@nvidia.com>
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>

Sashiko has reviewed this patch and found no issues. It looks great!
Jan Kara Sept. 22, 2026, 11:35 a.m. UTC | #2 
On Tue 22-09-26 19:07:01, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
> 
> In truncate_inode_partial_folio(), after the first split at the start
> edge, folio_split() unlocks and drops the refcount of the after-split
> sub-folios. The sub-folio straddling the end of the truncation range is
> therefore unlocked and only transiently ref'd in the page cache while
> the code still derives it from a page pointer inside the original folio.
> 
> Between the first split finishing and page_folio() resolving split_at2,
> that tail page can be reclaimed, freed and reallocated as a new large
> folio in the same mapping at a different file offset. folio2 then points
> at a folio that does not cover the end boundary, yet
> folio2->mapping == folio->mapping still holds, so the stale pointer
> passes the mapping check and folio_split_or_unmap() splits a folio at a
> wrong position (or, with a transient refcount, a use-after-free window
> opens between try_get and the split). __folio_split()'s own
> folio != page_folio(split_at) check cannot catch this either since
> split_at2 has been reallocated as part of the new folio, so
> page_folio(split_at2) resolves back to folio2.
> 
> Look the straddler up by its page index instead. __filemap_get_folio()
> returns the folio currently covering the boundary, ref'd and locked,
> with the mapping validated under the lock, so the split target is always
> the real folio at the end edge.
> 
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Link: https://sashiko.dev/#/message/20260916094500.C30061F00893%40smtp.kernel.org
> Link: https://lore.kernel.org/linux-mm/DLGXT0ERY79Z.3C5DYVJVX6S9Z@nvidia.com/
> Fixes: 7460b470a131 ("mm/truncate: use folio_split() in truncate operation")
> Cc: stable@vger.kernel.org
> Suggested-by: Jan Kara <jack@suse.cz>
> Suggested-by: Zi Yan <ziy@nvidia.com>
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  mm/truncate.c | 30 ++++++++++++++++--------------
>  1 file changed, 16 insertions(+), 14 deletions(-)
> 
> diff --git a/mm/truncate.c b/mm/truncate.c
> index f9625bb4916f..a8a179b38252 100644
> --- a/mm/truncate.c
> +++ b/mm/truncate.c
> @@ -259,30 +259,32 @@ bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
>  		 * for shmem truncate
>  		 */
>  		struct folio *folio2;
> +		pgoff_t end_idx;
>  
>  		if (offset + length == size)
>  			goto no_split;
>  
> -		split_at2 = folio_page(folio,
> -				PAGE_ALIGN_DOWN(offset + length) / PAGE_SIZE);
> -		folio2 = page_folio(split_at2);
> -
> -		if (!folio_try_get(folio2))
> +		/*
> +		 * After the first split at the start edge, the folio at the
> +		 * end edge may be freed and reused concurrently.
> +		 * __filemap_get_folio() looks up the straddler at end_idx
> +		 * and returns it locked and ref'd with the mapping
> +		 * validated.
> +		 */
> +		end_idx = (pos + offset + length) >> PAGE_SHIFT;
> +		folio2 = __filemap_get_folio(folio->mapping, end_idx,
> +					     FGP_LOCK | FGP_NOWAIT, 0);
> +		if (IS_ERR(folio2))
>  			goto no_split;
>  
> +		/* make sure folio2 is large */
>  		if (!folio_test_large(folio2))
>  			goto out;
>  
> -		if (!folio_trylock(folio2))
> -			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);
> -
> -		folio_unlock(folio2);
> +		split_at2 = folio_page(folio2, (end_idx - folio2->index));
> +		folio_split_or_unmap(folio2, split_at2, min_order);
>  out:
> +		folio_unlock(folio2);
>  		folio_put(folio2);
>  no_split:
>  		return true;
> -- 
> 2.52.0
>
Zi Yan Sept. 22, 2026, 3:30 p.m. UTC | #3 
On Tue Sep 22, 2026 at 7:07 AM EDT, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
>
> In truncate_inode_partial_folio(), after the first split at the start
> edge, folio_split() unlocks and drops the refcount of the after-split
> sub-folios. The sub-folio straddling the end of the truncation range is
> therefore unlocked and only transiently ref'd in the page cache while
> the code still derives it from a page pointer inside the original folio.
>
> Between the first split finishing and page_folio() resolving split_at2,
> that tail page can be reclaimed, freed and reallocated as a new large
> folio in the same mapping at a different file offset. folio2 then points
> at a folio that does not cover the end boundary, yet
> folio2->mapping == folio->mapping still holds, so the stale pointer
> passes the mapping check and folio_split_or_unmap() splits a folio at a
> wrong position (or, with a transient refcount, a use-after-free window
> opens between try_get and the split). __folio_split()'s own
> folio != page_folio(split_at) check cannot catch this either since
> split_at2 has been reallocated as part of the new folio, so
> page_folio(split_at2) resolves back to folio2.
>
> Look the straddler up by its page index instead. __filemap_get_folio()
> returns the folio currently covering the boundary, ref'd and locked,
> with the mapping validated under the lock, so the split target is always
> the real folio at the end edge.
>
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Link: https://sashiko.dev/#/message/20260916094500.C30061F00893%40smtp.kernel.org
> Link: https://lore.kernel.org/linux-mm/DLGXT0ERY79Z.3C5DYVJVX6S9Z@nvidia.com/
> Fixes: 7460b470a131 ("mm/truncate: use folio_split() in truncate operation")
> Cc: stable@vger.kernel.org
> Suggested-by: Jan Kara <jack@suse.cz>
> Suggested-by: Zi Yan <ziy@nvidia.com>
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>
> ---
>  mm/truncate.c | 30 ++++++++++++++++--------------
>  1 file changed, 16 insertions(+), 14 deletions(-)
>
Thanks.

Acked-by: Zi Yan <ziy@nvidia.com>
diff mbox series

Patch

diff --git a/mm/truncate.c b/mm/truncate.c
index f9625bb4916f..a8a179b38252 100644
--- a/mm/truncate.c
+++ b/mm/truncate.c
@@ -259,30 +259,32 @@  bool truncate_inode_partial_folio(struct folio *folio, loff_t start, loff_t end)
 		 * for shmem truncate
 		 */
 		struct folio *folio2;
+		pgoff_t end_idx;
 
 		if (offset + length == size)
 			goto no_split;
 
-		split_at2 = folio_page(folio,
-				PAGE_ALIGN_DOWN(offset + length) / PAGE_SIZE);
-		folio2 = page_folio(split_at2);
-
-		if (!folio_try_get(folio2))
+		/*
+		 * After the first split at the start edge, the folio at the
+		 * end edge may be freed and reused concurrently.
+		 * __filemap_get_folio() looks up the straddler at end_idx
+		 * and returns it locked and ref'd with the mapping
+		 * validated.
+		 */
+		end_idx = (pos + offset + length) >> PAGE_SHIFT;
+		folio2 = __filemap_get_folio(folio->mapping, end_idx,
+					     FGP_LOCK | FGP_NOWAIT, 0);
+		if (IS_ERR(folio2))
 			goto no_split;
 
+		/* make sure folio2 is large */
 		if (!folio_test_large(folio2))
 			goto out;
 
-		if (!folio_trylock(folio2))
-			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);
-
-		folio_unlock(folio2);
+		split_at2 = folio_page(folio2, (end_idx - folio2->index));
+		folio_split_or_unmap(folio2, split_at2, min_order);
 out:
+		folio_unlock(folio2);
 		folio_put(folio2);
 no_split:
 		return true;