Re: [PATCH 20/22] PM: hibernate: size the image from common memory

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] PM: hibernate: count_free_highmem_pages() ignores
  N_MEMORY_COMMON
--

commit 7cc6e673516327174024379e9585052d08c1578f
Author: Gregory Price (Meta) <gourry@gourry.net>

PM: hibernate: size the image from common memory

This commit restricts free page accounting in hibernate_preallocate_memory()
and enough_free_mem() to zones on N_MEMORY_COMMON nodes while keeping the
metadata estimate across all populated zones.

> diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c
> index d5dba0e50b2eb..8aeb086a3fd3d 100644
> --- a/kernel/power/snapshot.c
> +++ b/kernel/power/snapshot.c

[ ... ]

> @@ -2011,9 +2013,10 @@ static int enough_free_mem(unsigned int nr_pages, unsigned int nr_highmem)
>  	struct zone *zone;
>  	unsigned int free = alloc_normal;
>  
> -	for_each_populated_zone(zone)
> +	for_each_populated_zone_state(zone, N_MEMORY_COMMON) {
>  		if (!is_highmem(zone))
>  			free += zone_page_state(zone, NR_FREE_PAGES);
> +	}
>  
>  	nr_pages += count_pages_for_highmem(nr_highmem);
>  	pr_debug("Normal pages needed: %u + %u, available pages: %u\n",

[Severity: Medium]
Should count_free_highmem_pages() also iterate over N_MEMORY_COMMON zones?

On CONFIG_HIGHMEM systems, enough_free_mem() calls count_pages_for_highmem(),
which calls count_free_highmem_pages():

kernel/power/snapshot.c:count_free_highmem_pages() {
	struct zone *zone;
	unsigned int cnt = 0;

	for_each_populated_zone(zone)
		if (is_highmem(zone))
			cnt += zone_page_state(zone, NR_FREE_PAGES);

	return cnt;
}

Because count_free_highmem_pages() still sums all populated zones,
enough_free_mem() continues to count free highmem pages on non-common nodes as
available room for the image, undercounting the normal pages needed to back
highmem copies.

In addition, alloc_highmem_pages() and prepare_highmem_image() use
count_free_highmem_pages() to bound loops that call alloc_image_page() or
alloc_page() with __GFP_HIGHMEM and pass the returned page directly to
page_to_pfn() or swsusp_page_is_free() without a NULL check, which can
dereference NULL when highmem on N_MEMORY_COMMON nodes is exhausted while
non-common nodes still have free highmem pages.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261114070959.3490008-1-gourry@gourry.net?part=20