* [PATCH v2] PM: hibernate: Fix a bug in copying the zero bitmap to safe pages
@ 2023-10-04 5:01 Pavankumar Kondeti
2023-10-04 12:18 ` Brian Geffon
2023-10-04 18:42 ` Rafael J. Wysocki
0 siblings, 2 replies; 6+ messages in thread
From: Pavankumar Kondeti @ 2023-10-04 5:01 UTC (permalink / raw)
To: Rafael J. Wysocki, Pavel Machek, Len Brown
Cc: Brian Geffon, kernel, Rafael J. Wysocki, linux-pm, linux-kernel,
stable, Pavankumar Kondeti
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 with PG_ANY flag in
prepare_image() to make a copy of zero bitmap after the unsafe pages
are marked. Freeing this temporary bitmap with PG_UNSAFE_KEEP 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.
Fix this issue by creating the temporary zero bitmap from safe pages
(free bit not set) so that the corresponding free bits can be cleared while
freeing this bitmap.
Cc: stable <stable@kernel.org>
Fixes: 005e8dddd497 ("PM: hibernate: don't store zero pages in the image file")
Suggested-by:: Brian Geffon <bgeffon@google.com>
Signed-off-by: Pavankumar Kondeti <quic_pkondeti@quicinc.com>
---
Changes in v2:
- Allocate zero bit map from safe pages as suggested by Brian
- Link to v1: https://lore.kernel.org/r/20230929-hib_zero_bitmap_fix-v1-1-6cfdcb785250@quicinc.com
---
kernel/power/snapshot.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c
index 87e9f7e2bdc0..0f12e0a97e43 100644
--- a/kernel/power/snapshot.c
+++ b/kernel/power/snapshot.c
@@ -2647,7 +2647,7 @@ static int prepare_image(struct memory_bitmap *new_bm, struct memory_bitmap *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);
+ error = memory_bm_create(&tmp, GFP_ATOMIC, PG_SAFE);
if (error)
goto Free;
@@ -2660,7 +2660,7 @@ static int prepare_image(struct memory_bitmap *new_bm, struct memory_bitmap *bm,
goto Free;
duplicate_memory_bitmap(zero_bm, &tmp);
- memory_bm_free(&tmp, PG_UNSAFE_KEEP);
+ memory_bm_free(&tmp, PG_UNSAFE_CLEAR);
/* At this point zero_bm is in safe pages and it can be used for restoring. */
if (nr_highmem > 0) {
---
base-commit: 8a749fd1a8720d4619c91c8b6e7528c0a355c0aa
change-id: 20230929-hib_zero_bitmap_fix-bc5884eba0ae
Best regards,
--
Pavankumar Kondeti <quic_pkondeti@quicinc.com>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] PM: hibernate: Fix a bug in copying the zero bitmap to safe pages
2023-10-04 5:01 [PATCH v2] PM: hibernate: Fix a bug in copying the zero bitmap to safe pages Pavankumar Kondeti
@ 2023-10-04 12:18 ` Brian Geffon
2023-10-04 18:46 ` Rafael J. Wysocki
2023-10-04 18:42 ` Rafael J. Wysocki
1 sibling, 1 reply; 6+ messages in thread
From: Brian Geffon @ 2023-10-04 12:18 UTC (permalink / raw)
To: Pavankumar Kondeti
Cc: Rafael J. Wysocki, Pavel Machek, Len Brown, kernel,
Rafael J. Wysocki, linux-pm, linux-kernel, stable
On Wed, Oct 4, 2023 at 1:01 AM Pavankumar Kondeti
<quic_pkondeti@quicinc.com> wrote:
>
> 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 with PG_ANY flag in
> prepare_image() to make a copy of zero bitmap after the unsafe pages
> are marked. Freeing this temporary bitmap with PG_UNSAFE_KEEP 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.
>
> Fix this issue by creating the temporary zero bitmap from safe pages
> (free bit not set) so that the corresponding free bits can be cleared while
> freeing this bitmap.
>
> Cc: stable <stable@kernel.org>
> Fixes: 005e8dddd497 ("PM: hibernate: don't store zero pages in the image file")
> Suggested-by:: Brian Geffon <bgeffon@google.com>
> Signed-off-by: Pavankumar Kondeti <quic_pkondeti@quicinc.com>
Reviewed-by: Brian Geffon <bgeffon@google.com>
> ---
> Changes in v2:
> - Allocate zero bit map from safe pages as suggested by Brian
> - Link to v1: https://lore.kernel.org/r/20230929-hib_zero_bitmap_fix-v1-1-6cfdcb785250@quicinc.com
> ---
> kernel/power/snapshot.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c
> index 87e9f7e2bdc0..0f12e0a97e43 100644
> --- a/kernel/power/snapshot.c
> +++ b/kernel/power/snapshot.c
> @@ -2647,7 +2647,7 @@ static int prepare_image(struct memory_bitmap *new_bm, struct memory_bitmap *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);
> + error = memory_bm_create(&tmp, GFP_ATOMIC, PG_SAFE);
> if (error)
> goto Free;
>
> @@ -2660,7 +2660,7 @@ static int prepare_image(struct memory_bitmap *new_bm, struct memory_bitmap *bm,
> goto Free;
>
> duplicate_memory_bitmap(zero_bm, &tmp);
> - memory_bm_free(&tmp, PG_UNSAFE_KEEP);
> + memory_bm_free(&tmp, PG_UNSAFE_CLEAR);
> /* At this point zero_bm is in safe pages and it can be used for restoring. */
>
> if (nr_highmem > 0) {
>
> ---
> base-commit: 8a749fd1a8720d4619c91c8b6e7528c0a355c0aa
> change-id: 20230929-hib_zero_bitmap_fix-bc5884eba0ae
>
Thanks!
Brian
> Best regards,
> --
> Pavankumar Kondeti <quic_pkondeti@quicinc.com>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] PM: hibernate: Fix a bug in copying the zero bitmap to safe pages
2023-10-04 12:18 ` Brian Geffon
@ 2023-10-04 18:46 ` Rafael J. Wysocki
2023-10-06 17:21 ` Mark Rutland
0 siblings, 1 reply; 6+ messages in thread
From: Rafael J. Wysocki @ 2023-10-04 18:46 UTC (permalink / raw)
To: Brian Geffon, Pavankumar Kondeti
Cc: Pavel Machek, Len Brown, kernel, Rafael J. Wysocki, linux-pm,
linux-kernel
On Wed, Oct 4, 2023 at 2:19 PM Brian Geffon <bgeffon@google.com> wrote:
>
> On Wed, Oct 4, 2023 at 1:01 AM Pavankumar Kondeti
> <quic_pkondeti@quicinc.com> wrote:
> >
> > 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 with PG_ANY flag in
> > prepare_image() to make a copy of zero bitmap after the unsafe pages
> > are marked. Freeing this temporary bitmap with PG_UNSAFE_KEEP 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.
> >
> > Fix this issue by creating the temporary zero bitmap from safe pages
> > (free bit not set) so that the corresponding free bits can be cleared while
> > freeing this bitmap.
> >
> > Cc: stable <stable@kernel.org>
> > Fixes: 005e8dddd497 ("PM: hibernate: don't store zero pages in the image file")
> > Suggested-by:: Brian Geffon <bgeffon@google.com>
> > Signed-off-by: Pavankumar Kondeti <quic_pkondeti@quicinc.com>
>
> Reviewed-by: Brian Geffon <bgeffon@google.com>
Applied as 6.7 material, but without the Cc: stable tag that is (a)
invalid (there should be vger.kernel.org in the host part) and (b)
unnecessary AFAICS.
Thanks!
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] PM: hibernate: Fix a bug in copying the zero bitmap to safe pages
2023-10-04 18:46 ` Rafael J. Wysocki
@ 2023-10-06 17:21 ` Mark Rutland
2023-10-06 17:57 ` Rafael J. Wysocki
0 siblings, 1 reply; 6+ messages in thread
From: Mark Rutland @ 2023-10-06 17:21 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Brian Geffon, Pavankumar Kondeti, Pavel Machek, Len Brown,
kernel, Rafael J. Wysocki, linux-pm, linux-kernel
Hi Rafael,
On Wed, Oct 04, 2023 at 08:46:56PM +0200, Rafael J. Wysocki wrote:
> On Wed, Oct 4, 2023 at 2:19 PM Brian Geffon <bgeffon@google.com> wrote:
> >
> > On Wed, Oct 4, 2023 at 1:01 AM Pavankumar Kondeti
> > <quic_pkondeti@quicinc.com> wrote:
> > >
> > > 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 with PG_ANY flag in
> > > prepare_image() to make a copy of zero bitmap after the unsafe pages
> > > are marked. Freeing this temporary bitmap with PG_UNSAFE_KEEP 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.
> > >
> > > Fix this issue by creating the temporary zero bitmap from safe pages
> > > (free bit not set) so that the corresponding free bits can be cleared while
> > > freeing this bitmap.
> > >
> > > Cc: stable <stable@kernel.org>
> > > Fixes: 005e8dddd497 ("PM: hibernate: don't store zero pages in the image file")
> > > Suggested-by:: Brian Geffon <bgeffon@google.com>
> > > Signed-off-by: Pavankumar Kondeti <quic_pkondeti@quicinc.com>
> >
> > Reviewed-by: Brian Geffon <bgeffon@google.com>
>
> Applied as 6.7 material, but without the Cc: stable tag that is (a)
> invalid (there should be vger.kernel.org in the host part) and (b)
> unnecessary AFAICS.
Just to check, did you mean as v6.6 material?
I'm consistently hitting this on real arm64 hardware with v6.6-rc*.
If this is v6.7 material, are we going to revert 005e8dddd497 for now?
I've tested the above patch atop v6.6-rc3, and it solves the problem for me, so
FWIW:
Tested-by: Mark Rutland <mark.rutland@arm.com>
Thanks,
Mark.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] PM: hibernate: Fix a bug in copying the zero bitmap to safe pages
2023-10-06 17:21 ` Mark Rutland
@ 2023-10-06 17:57 ` Rafael J. Wysocki
0 siblings, 0 replies; 6+ messages in thread
From: Rafael J. Wysocki @ 2023-10-06 17:57 UTC (permalink / raw)
To: Mark Rutland
Cc: Rafael J. Wysocki, Brian Geffon, Pavankumar Kondeti,
Pavel Machek, Len Brown, kernel, Rafael J. Wysocki, linux-pm,
linux-kernel
On Fri, Oct 6, 2023 at 7:21 PM Mark Rutland <mark.rutland@arm.com> wrote:
>
> Hi Rafael,
>
> On Wed, Oct 04, 2023 at 08:46:56PM +0200, Rafael J. Wysocki wrote:
> > On Wed, Oct 4, 2023 at 2:19 PM Brian Geffon <bgeffon@google.com> wrote:
> > >
> > > On Wed, Oct 4, 2023 at 1:01 AM Pavankumar Kondeti
> > > <quic_pkondeti@quicinc.com> wrote:
> > > >
> > > > 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 with PG_ANY flag in
> > > > prepare_image() to make a copy of zero bitmap after the unsafe pages
> > > > are marked. Freeing this temporary bitmap with PG_UNSAFE_KEEP 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.
> > > >
> > > > Fix this issue by creating the temporary zero bitmap from safe pages
> > > > (free bit not set) so that the corresponding free bits can be cleared while
> > > > freeing this bitmap.
> > > >
> > > > Cc: stable <stable@kernel.org>
> > > > Fixes: 005e8dddd497 ("PM: hibernate: don't store zero pages in the image file")
> > > > Suggested-by:: Brian Geffon <bgeffon@google.com>
> > > > Signed-off-by: Pavankumar Kondeti <quic_pkondeti@quicinc.com>
> > >
> > > Reviewed-by: Brian Geffon <bgeffon@google.com>
> >
> > Applied as 6.7 material, but without the Cc: stable tag that is (a)
> > invalid (there should be vger.kernel.org in the host part) and (b)
> > unnecessary AFAICS.
>
> Just to check, did you mean as v6.6 material?
Yes, I did, sorry.
Actually, I've just sent a pull request with it.
> I'm consistently hitting this on real arm64 hardware with v6.6-rc*.
>
> If this is v6.7 material, are we going to revert 005e8dddd497 for now?
>
> I've tested the above patch atop v6.6-rc3, and it solves the problem for me, so
> FWIW:
>
> Tested-by: Mark Rutland <mark.rutland@arm.com>
Thanks a lot for verifying this! Alas, it is too late to add the tag ...
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] PM: hibernate: Fix a bug in copying the zero bitmap to safe pages
2023-10-04 5:01 [PATCH v2] PM: hibernate: Fix a bug in copying the zero bitmap to safe pages Pavankumar Kondeti
2023-10-04 12:18 ` Brian Geffon
@ 2023-10-04 18:42 ` Rafael J. Wysocki
1 sibling, 0 replies; 6+ messages in thread
From: Rafael J. Wysocki @ 2023-10-04 18:42 UTC (permalink / raw)
To: Pavankumar Kondeti
Cc: Rafael J. Wysocki, Pavel Machek, Len Brown, Brian Geffon, kernel,
Rafael J. Wysocki, linux-pm, linux-kernel, stable
On Wed, Oct 4, 2023 at 7:01 AM Pavankumar Kondeti
<quic_pkondeti@quicinc.com> wrote:
>
> 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 with PG_ANY flag in
> prepare_image() to make a copy of zero bitmap after the unsafe pages
> are marked. Freeing this temporary bitmap with PG_UNSAFE_KEEP 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.
>
> Fix this issue by creating the temporary zero bitmap from safe pages
> (free bit not set) so that the corresponding free bits can be cleared while
> freeing this bitmap.
>
> Cc: stable <stable@kernel.org>
Why is this tag present? The commit below hasn't made it to a major
release yet AFAICS.
> Fixes: 005e8dddd497 ("PM: hibernate: don't store zero pages in the image file")
> Suggested-by:: Brian Geffon <bgeffon@google.com>
> Signed-off-by: Pavankumar Kondeti <quic_pkondeti@quicinc.com>
> ---
> Changes in v2:
> - Allocate zero bit map from safe pages as suggested by Brian
> - Link to v1: https://lore.kernel.org/r/20230929-hib_zero_bitmap_fix-v1-1-6cfdcb785250@quicinc.com
> ---
> kernel/power/snapshot.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c
> index 87e9f7e2bdc0..0f12e0a97e43 100644
> --- a/kernel/power/snapshot.c
> +++ b/kernel/power/snapshot.c
> @@ -2647,7 +2647,7 @@ static int prepare_image(struct memory_bitmap *new_bm, struct memory_bitmap *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);
> + error = memory_bm_create(&tmp, GFP_ATOMIC, PG_SAFE);
> if (error)
> goto Free;
>
> @@ -2660,7 +2660,7 @@ static int prepare_image(struct memory_bitmap *new_bm, struct memory_bitmap *bm,
> goto Free;
>
> duplicate_memory_bitmap(zero_bm, &tmp);
> - memory_bm_free(&tmp, PG_UNSAFE_KEEP);
> + memory_bm_free(&tmp, PG_UNSAFE_CLEAR);
> /* At this point zero_bm is in safe pages and it can be used for restoring. */
>
> if (nr_highmem > 0) {
>
> ---
> base-commit: 8a749fd1a8720d4619c91c8b6e7528c0a355c0aa
> change-id: 20230929-hib_zero_bitmap_fix-bc5884eba0ae
>
> Best regards,
> --
> Pavankumar Kondeti <quic_pkondeti@quicinc.com>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-10-06 17:57 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-04 5:01 [PATCH v2] PM: hibernate: Fix a bug in copying the zero bitmap to safe pages Pavankumar Kondeti
2023-10-04 12:18 ` Brian Geffon
2023-10-04 18:46 ` Rafael J. Wysocki
2023-10-06 17:21 ` Mark Rutland
2023-10-06 17:57 ` Rafael J. Wysocki
2023-10-04 18:42 ` Rafael J. Wysocki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®