Re: page refcount race between prep_compound_gigantic_page() and __page_cache_add_speculative()?

From: Matthew Wilcox
Date: Tue Jun 15 2021 - 08:40:54 EST


On Tue, Jun 15, 2021 at 01:03:53PM +0200, Jann Horn wrote:
> The messier path, as the original commit describes, is "gigantic" page
> allocation. In that case, we'll go through the following path (if we
> ignore CMA):
>
> alloc_fresh_huge_page():
> alloc_gigantic_page()
> alloc_contig_pages()
> __alloc_contig_pages()
> alloc_contig_range()
> isolate_freepages_range()
> split_map_pages()
> post_alloc_hook() [FOR EVERY PAGE]
> set_page_refcounted()
> set_page_count(page, 1)
> prep_compound_gigantic_page()
> set_page_count(p, 0) [FOR EVERY TAIL PAGE]
>
> so all the tail pages are initially allocated with refcount 1 by the
> page allocator, and then we overwrite those refcounts with zeroes.
>
>
> Luckily, the only non-__init codepath that can get here is
> __nr_hugepages_store_common(), which is only invoked from privileged
> writes to sysfs/sysctls.

Argh. What if we passed __GFP_COMP into alloc_contig_pages()?
The current callers of alloc_contig_range() do not pass __GFP_COMP,
so it's no behaviour change for them, and __GFP_COMP implies this
kind of behaviour. I think that would imply _not_ calling
split_map_pages(), which implies not calling post_alloc_hook(),
which means we probably need to do a lot of the parts of
post_alloc_hook() in alloc_gigantic_page(). Yuck.