linux-kernel - Re: [PATCH v8 8/8] memblock: Unpreserve memory in case of error

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aP5Mcr9WCt5CHon6@kernel.org>
Date: Sun, 26 Oct 2025 18:29:38 +0200
From: Mike Rapoport <rppt@...nel.org>
To: Pasha Tatashin <pasha.tatashin@...een.com>
Cc: akpm@...ux-foundation.org, brauner@...nel.org, corbet@....net,
	graf@...zon.com, jgg@...pe.ca, linux-kernel@...r.kernel.org,
	linux-kselftest@...r.kernel.org, linux-mm@...ck.org,
	masahiroy@...nel.org, ojeda@...nel.org, pratyush@...nel.org,
	rdunlap@...radead.org, tj@...nel.org
Subject: Re: [PATCH v8 8/8] memblock: Unpreserve memory in case of error

On Fri, Oct 24, 2025 at 12:10:02PM -0400, Pasha Tatashin wrote:
> If there is an error half way through KHO memory preservation, we should
> rollback and unpreserve everything that is partially preserved.
> 
> Signed-off-by: Pasha Tatashin <pasha.tatashin@...een.com>
> Suggested-by: Pratyush Yadav <pratyush@...nel.org>
> ---
>  mm/memblock.c | 26 ++++++++++++++++++++++----
>  1 file changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/mm/memblock.c b/mm/memblock.c
> index e3bef9b35d63..5ceaa02af7d6 100644
> --- a/mm/memblock.c
> +++ b/mm/memblock.c
> @@ -2447,6 +2447,7 @@ int reserve_mem_release_by_name(const char *name)
>  
>  static int __init prepare_kho_fdt(void)
>  {
> +	bool fdt_folio_preserved = false;

fdt_preserved is enough IMHO.

>  	int err = 0, i;
>  	struct page *fdt_page;
>  	void *fdt;
> @@ -2462,12 +2463,14 @@ static int __init prepare_kho_fdt(void)
>  
>  	err |= fdt_begin_node(fdt, "");
>  	err |= fdt_property_string(fdt, "compatible", MEMBLOCK_KHO_NODE_COMPATIBLE);
> -	for (i = 0; i < reserved_mem_count; i++) {
> +	for (i = 0; !err && i < reserved_mem_count; i++) {
>  		struct reserve_mem_table *map = &reserved_mem_table[i];
>  		struct page *page = phys_to_page(map->start);
>  		unsigned int nr_pages = map->size >> PAGE_SHIFT;
>  
> -		err |= kho_preserve_pages(page, nr_pages);
> +		err = kho_preserve_pages(page, nr_pages);
> +		if (err)
> +			break;

Please

	goto err_unpreserve;

>  		err |= fdt_begin_node(fdt, map->name);
>  		err |= fdt_property_string(fdt, "compatible", RESERVE_MEM_KHO_NODE_COMPATIBLE);
>  		err |= fdt_property(fdt, "start", &map->start, sizeof(map->start));

	if (err)
		goto err_unpreserve;

and drop !err from the loop condition.

> @@ -2477,12 +2480,27 @@ static int __init prepare_kho_fdt(void)
>  	err |= fdt_end_node(fdt);
>  	err |= fdt_finish(fdt);
>  
> -	err |= kho_preserve_folio(page_folio(fdt_page));
> -
>  	if (!err)
> +		err = kho_preserve_folio(page_folio(fdt_page));
> +
> +	if (!err) {
> +		fdt_folio_preserved = true;
>  		err = kho_add_subtree(MEMBLOCK_KHO_FDT, fdt);
> +	}
>  
>  	if (err) {
> +		int nr_reserve_map_preserved = i;

nr_preserved is clear enough.
Also let's declare it before the preservation loop and count it there. Than
we can make loop variable local which makes it safer against certain side
channel attacks. I.e the loop that preserves the memory would be

	for (unsigned int i = 0; i < reserve_mem_count; i++ nr_preserved++) 

> +
> +		for (i = 0; i < nr_reserve_map_preserved; i++) {
> +			struct reserve_mem_table *map = &reserved_mem_table[i];
> +			struct page *page = phys_to_page(map->start);
> +			unsigned int nr_pages = map->size >> PAGE_SHIFT;
> +
> +			kho_unpreserve_pages(page, nr_pages);
> +		}
> +		if (fdt_folio_preserved)
> +			kho_unpreserve_folio(page_folio(fdt_page));
> +
>  		pr_err("failed to prepare memblock FDT for KHO: %d\n", err);
>  		put_page(fdt_page);
>  	}
> -- 
> 2.51.1.821.gb6fe4d2222-goog
> 

-- 
Sincerely yours,
Mike.