Skip to content

Commit 9425c59

Browse files
mjkravetzakpm00
authored andcommitted
page cache: fix page_cache_next/prev_miss off by one
Ackerley Tng reported an issue with hugetlbfs fallocate here[1]. The issue showed up after the conversion of hugetlb page cache lookup code to use page_cache_next_miss. Code in hugetlb fallocate, userfaultfd and GUP is now using page_cache_next_miss to determine if a page is present the page cache. The following statement is used. present = page_cache_next_miss(mapping, index, 1) != index; There are two issues with page_cache_next_miss when used in this way. 1) If the passed value for index is equal to the 'wrap-around' value, the same index will always be returned. This wrap-around value is 0, so 0 will be returned even if page is present at index 0. 2) If there is no gap in the range passed, the last index in the range will be returned. When passed a range of 1 as above, the passed index value will be returned even if the page is present. The end result is the statement above will NEVER indicate a page is present in the cache, even if it is. As noted by Ackerley in [1], users can see this by hugetlb fallocate incorrectly returning EEXIST if pages are already present in the file. In addition, hugetlb pages will not be included in core dumps if they need to be brought in via GUP. userfaultfd UFFDIO_COPY also uses this code and will not notice pages already present in the cache. It may try to allocate a new page and potentially return ENOMEM as opposed to EEXIST. Both page_cache_next_miss and page_cache_prev_miss have similar issues. Fix by: - Check for index equal to 'wrap-around' value and do not exit early. - If no gap is found in range, return index outside range. - Update function description to say 'wrap-around' value could be returned if passed as index. [1] https://lore.kernel.org/linux-mm/[email protected]/ Link: https://lkml.kernel.org/r/[email protected] Fixes: d0ce0e4 ("mm/hugetlb: convert hugetlb fault paths to use alloc_hugetlb_folio()") Signed-off-by: Mike Kravetz <[email protected]> Reported-by: Ackerley Tng <[email protected]> Reviewed-by: Ackerley Tng <[email protected]> Tested-by: Ackerley Tng <[email protected]> Cc: Erdem Aktas <[email protected]> Cc: Matthew Wilcox (Oracle) <[email protected]> Cc: Mike Kravetz <[email protected]> Cc: Muchun Song <[email protected]> Cc: Sidhartha Kumar <[email protected]> Cc: Vishal Annapurve <[email protected]> Cc: Greg Kroah-Hartman <[email protected]> Signed-off-by: Andrew Morton <[email protected]>
1 parent 26a6fff commit 9425c59

File tree

1 file changed

+16
-10
lines changed

1 file changed

+16
-10
lines changed

mm/filemap.c

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1760,7 +1760,9 @@ bool __folio_lock_or_retry(struct folio *folio, struct mm_struct *mm,
17601760
*
17611761
* Return: The index of the gap if found, otherwise an index outside the
17621762
* range specified (in which case 'return - index >= max_scan' will be true).
1763-
* In the rare case of index wrap-around, 0 will be returned.
1763+
* In the rare case of index wrap-around, 0 will be returned. 0 will also
1764+
* be returned if index == 0 and there is a gap at the index. We can not
1765+
* wrap-around if passed index == 0.
17641766
*/
17651767
pgoff_t page_cache_next_miss(struct address_space *mapping,
17661768
pgoff_t index, unsigned long max_scan)
@@ -1770,12 +1772,13 @@ pgoff_t page_cache_next_miss(struct address_space *mapping,
17701772
while (max_scan--) {
17711773
void *entry = xas_next(&xas);
17721774
if (!entry || xa_is_value(entry))
1773-
break;
1774-
if (xas.xa_index == 0)
1775-
break;
1775+
return xas.xa_index;
1776+
if (xas.xa_index == 0 && index != 0)
1777+
return xas.xa_index;
17761778
}
17771779

1778-
return xas.xa_index;
1780+
/* No gaps in range and no wrap-around, return index beyond range */
1781+
return xas.xa_index + 1;
17791782
}
17801783
EXPORT_SYMBOL(page_cache_next_miss);
17811784

@@ -1796,7 +1799,9 @@ EXPORT_SYMBOL(page_cache_next_miss);
17961799
*
17971800
* Return: The index of the gap if found, otherwise an index outside the
17981801
* range specified (in which case 'index - return >= max_scan' will be true).
1799-
* In the rare case of wrap-around, ULONG_MAX will be returned.
1802+
* In the rare case of wrap-around, ULONG_MAX will be returned. ULONG_MAX
1803+
* will also be returned if index == ULONG_MAX and there is a gap at the
1804+
* index. We can not wrap-around if passed index == ULONG_MAX.
18001805
*/
18011806
pgoff_t page_cache_prev_miss(struct address_space *mapping,
18021807
pgoff_t index, unsigned long max_scan)
@@ -1806,12 +1811,13 @@ pgoff_t page_cache_prev_miss(struct address_space *mapping,
18061811
while (max_scan--) {
18071812
void *entry = xas_prev(&xas);
18081813
if (!entry || xa_is_value(entry))
1809-
break;
1810-
if (xas.xa_index == ULONG_MAX)
1811-
break;
1814+
return xas.xa_index;
1815+
if (xas.xa_index == ULONG_MAX && index != ULONG_MAX)
1816+
return xas.xa_index;
18121817
}
18131818

1814-
return xas.xa_index;
1819+
/* No gaps in range and no wrap-around, return index beyond range */
1820+
return xas.xa_index - 1;
18151821
}
18161822
EXPORT_SYMBOL(page_cache_prev_miss);
18171823

0 commit comments

Comments
 (0)