linux-kernel - [PATCH] PM: hibernate: Fix a bug in copying the zero bitmap to safe pages

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-next>] [day] [month] [year] [list]

Message-ID: <20230929-hib_zero_bitmap_fix-v1-1-6cfdcb785250@quicinc.com>
Date:   Fri, 29 Sep 2023 23:01:15 +0530
From:   Pavankumar Kondeti <quic_pkondeti@...cinc.com>
To:     "Rafael J. Wysocki" <rafael@...nel.org>,
        Pavel Machek <pavel@....cz>, "Len Brown" <len.brown@...el.com>
CC:     Brian Geffon <bgeffon@...gle.com>, <kernel@...cinc.com>,
        "Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
        <linux-pm@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        Pavankumar Kondeti <quic_pkondeti@...cinc.com>
Subject: [PATCH] PM: hibernate: Fix a bug in copying the zero bitmap to
 safe pages

The following crash is observed 100% of the time during resume from
the hibernation on a x86 QEMU system.

[   12.931887]  ? __die_body+0x1a/0x60
[   12.932324]  ? page_fault_oops+0x156/0x420
[   12.932824]  ? search_exception_tables+0x37/0x50
[   12.933389]  ? fixup_exception+0x21/0x300
[   12.933889]  ? exc_page_fault+0x69/0x150
[   12.934371]  ? asm_exc_page_fault+0x26/0x30
[   12.934869]  ? get_buffer.constprop.0+0xac/0x100
[   12.935428]  snapshot_write_next+0x7c/0x9f0
[   12.935929]  ? submit_bio_noacct_nocheck+0x2c2/0x370
[   12.936530]  ? submit_bio_noacct+0x44/0x2c0
[   12.937035]  ? hib_submit_io+0xa5/0x110
[   12.937501]  load_image+0x83/0x1a0
[   12.937919]  swsusp_read+0x17f/0x1d0
[   12.938355]  ? create_basic_memory_bitmaps+0x1b7/0x240
[   12.938967]  load_image_and_restore+0x45/0xc0
[   12.939494]  software_resume+0x13c/0x180
[   12.939994]  resume_store+0xa3/0x1d0

The commit being fixed introduced a bug in copying the zero bitmap
to safe pages. A temporary bitmap is allocated in prepare_image()
to make a copy of zero bitmap after the unsafe pages are marked.
Freeing this temporary bitmap later results in an inconsistent state
of unsafe pages. Since free bit is left as is for this temporary bitmap
after free, these pages are treated as unsafe pages when they are
allocated again. This results in incorrect calculation of the number
of pages pre-allocated for the image.

nr_pages = (nr_zero_pages + nr_copy_pages) - nr_highmem - allocated_unsafe_pages;

The allocate_unsafe_pages is estimated to be higher than the actual
which results in running short of pages in safe_pages_list. Hence the
crash is observed in get_buffer() due to NULL pointer access of
safe_pages_list.

Fixes: 005e8dddd497 ("PM: hibernate: don't store zero pages in the image file")
Signed-off-by: Pavankumar Kondeti <quic_pkondeti@...cinc.com>
---
 kernel/power/snapshot.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c
index 87e9f7e2bdc0..cb7341a71a21 100644
--- a/kernel/power/snapshot.c
+++ b/kernel/power/snapshot.c
@@ -2628,7 +2628,7 @@ static int prepare_image(struct memory_bitmap *new_bm, struct memory_bitmap *bm,
 		struct memory_bitmap *zero_bm)
 {
 	unsigned int nr_pages, nr_highmem;
-	struct memory_bitmap tmp;
+	struct memory_bitmap tmp_zero_bm;
 	struct linked_page *lp;
 	int error;
 
@@ -2636,6 +2636,16 @@ static int prepare_image(struct memory_bitmap *new_bm, struct memory_bitmap *bm,
 	free_image_page(buffer, PG_UNSAFE_CLEAR);
 	buffer = NULL;
 
+	/*
+	 * Allocate a temporary bitmap to create a copy of zero_bm in
+	 * safe pages. This allocation needs to be done before marking
+	 * unsafe pages below so that it can be freed without altering
+	 * the state of unsafe pages.
+	 */
+	error = memory_bm_create(&tmp_zero_bm, GFP_ATOMIC, PG_ANY);
+	if (error)
+		goto Free;
+
 	nr_highmem = count_highmem_image_pages(bm);
 	mark_unsafe_pages(bm);
 
@@ -2646,12 +2656,7 @@ static int prepare_image(struct memory_bitmap *new_bm, struct memory_bitmap *bm,
 	duplicate_memory_bitmap(new_bm, bm);
 	memory_bm_free(bm, PG_UNSAFE_KEEP);
 
-	/* Make a copy of zero_bm so it can be created in safe pages */
-	error = memory_bm_create(&tmp, GFP_ATOMIC, PG_ANY);
-	if (error)
-		goto Free;
-
-	duplicate_memory_bitmap(&tmp, zero_bm);
+	duplicate_memory_bitmap(&tmp_zero_bm, zero_bm);
 	memory_bm_free(zero_bm, PG_UNSAFE_KEEP);
 
 	/* Recreate zero_bm in safe pages */
@@ -2659,8 +2664,8 @@ static int prepare_image(struct memory_bitmap *new_bm, struct memory_bitmap *bm,
 	if (error)
 		goto Free;
 
-	duplicate_memory_bitmap(zero_bm, &tmp);
-	memory_bm_free(&tmp, PG_UNSAFE_KEEP);
+	duplicate_memory_bitmap(zero_bm, &tmp_zero_bm);
+	memory_bm_free(&tmp_zero_bm, PG_UNSAFE_KEEP);
 	/* At this point zero_bm is in safe pages and it can be used for restoring. */
 
 	if (nr_highmem > 0) {

---
base-commit: 6465e260f48790807eef06b583b38ca9789b6072
change-id: 20230929-hib_zero_bitmap_fix-bc5884eba0ae

Best regards,
-- 
Pavankumar Kondeti <quic_pkondeti@...cinc.com>