Re: [PATCH v2 1/2] mm: decrement MTHP_STAT_NR_ANON in free_zone_device_folio()

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Large anonymous ZONE_DEVICE folios are leaked on the deferred split list when freed, causing list corruption, Use-After-Free, and permanently elevated MTHP partially-mapped stats.
--

--- Patch [1]: [PATCH v2 1/2] mm: decrement MTHP_STAT_NR_ANON in free_zone_device_folio() ---
commit cd3d1728ead3da706aada7ea30df9436a9ec5060
Author: Nico Pache <[email protected]>

mm: decrement MTHP_STAT_NR_ANON in free_zone_device_folio()

When a zone device folio is mapped as anonymous, folio_add_new_anon_rmap()
increments MTHP_STAT_NR_ANON. The corresponding decrement lives in
__free_pages_prepare() in page_alloc.c, but zone device folios are freed
via free_zone_device_folio() which never calls __free_pages_prepare().
This causes nr_anon to remain permanently elevated after zone device
folios are freed.

Fixes: 5d65c8d758f2 ("mm: count the number of anonymous THPs per size")

> diff --git a/mm/memremap.c b/mm/memremap.c
> index 81766d8224009..accba23aef28c 100644
> --- a/mm/memremap.c
> +++ b/mm/memremap.c
> @@ -425,6 +425,7 @@ void free_zone_device_folio(struct folio *folio)
>  	mem_cgroup_uncharge(folio);
>  
>  	if (folio_test_anon(folio)) {
> +		mod_mthp_stat(folio_order(folio), MTHP_STAT_NR_ANON, -1);

[Severity: Critical]
This isn't a bug introduced by this patch, but since we are handling large
anonymous zone device folios here, is there a missing cleanup for the
deferred split list?

When a large device coherent folio is partially unmapped,
__folio_remove_rmap() can place it on the deferred split list because it is
not device private:

__folio_remove_rmap() {
    ...
    if (partially_mapped && folio_test_anon(folio) &&
        !folio_test_partially_mapped(folio) &&
        !folio_is_device_private(folio))
        deferred_split_folio(folio, true);
    ...
}

Later, when the folio's refcount drops to zero, __folio_put() short-circuits
directly to free_zone_device_folio() and returns, bypassing the call to
folio_unqueue_deferred_split() entirely:

__folio_put() {
    ...
    if (unlikely(folio_is_zone_device(folio))) {
        free_zone_device_folio(folio);
        return;
    }
    ...
    folio_unqueue_deferred_split(folio);
    ...
}

Does free_zone_device_folio() need to call folio_unqueue_deferred_split()
before freeing the folio back to the device allocator?

If the freed folio remains on the deferred split list, could the deferred
split shrinker iterate over a freed or reused list node? It seems this might
cause list corruption or a use-after-free panic, and leave
MTHP_STAT_NR_ANON_PARTIALLY_MAPPED permanently elevated.

>  		for (i = 0; i < nr; i++)
>  			__ClearPageAnonExclusive(folio_page(folio, i));
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1