On Fri, 2 Jun 2023 15:57:47 -0700 Mike Kravetz <mike.kravetz@xxxxxxxxxx> wrote:
> 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.
So I'm assuming
Fixes: d0ce0e47b323 ("mm/hugetlb: convert hugetlb fault paths to use alloc_hugetlb_folio()")
?
> 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/cover.1683069252.git.ackerleytng@xxxxxxxxxx/
>
