[PATCH v2 0/2] Fix the hibernation test_resume's swapfile open mode

[PATCH v2 0/2] Fix the hibernation test_resume's swapfile open mode

[Chen Yu]

Fix the regression brought by
commit 5904de0d735b ("PM: hibernate: Do not get block device exclusively
in test_resume mode"), which incorrectly open the swapfile in a wrong
mode, and causes unexpected behavior during non-hibernation operation.

Fix it by really disabling exclusive mode under test_resume.

This is the v2 of the fix for a hibernation test_resume swapfile open issue.
Chenzhou found that v1 has a warning during the stress test. It was due to
broken open mode in swsusp_write(), fixes it in v2.

patch 1 is a renaming of the function parameter, to better reflects the swap
device open mode.
patch 2 is to fix the open mode on the swap device. 

v1: https://lore.kernel.org/lkml/20230823161329.7348-1-yu.c.chen@intel.com/

Chen Yu (2):
  PM: hibernate: Rename the function parameter from snapshot_test to
    exclusive
  PM: hibernate: Fix the exclusive get block device in test_resume mode

 kernel/power/hibernate.c | 12 ++++++------
 kernel/power/power.h     |  4 ++--
 kernel/power/swap.c      | 14 ++++++++------
 3 files changed, 16 insertions(+), 14 deletions(-)

-- 
2.25.1

[PATCH v2 1/2] PM: hibernate: Rename the function parameter from snapshot_test to exclusive

[Chen Yu]

Several functions reply on snapshot_test to decide whether to
open the resume device exclusively. However there is no strict
connection between the snapshot_test and the open mode. Rename
the 'snapshot_test' input parameter to 'exclusive' to better reflect
the use case.

No functional change is expected.

Signed-off-by: Chen Yu <yu.c.chen@intel.com>
---
 kernel/power/power.h |  4 ++--
 kernel/power/swap.c  | 14 ++++++++------
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/kernel/power/power.h b/kernel/power/power.h
index 46eb14dc50c3..a98f95e309a3 100644
--- a/kernel/power/power.h
+++ b/kernel/power/power.h
@@ -168,11 +168,11 @@ extern int swsusp_swap_in_use(void);
 #define SF_HW_SIG		8
 
 /* kernel/power/hibernate.c */
-int swsusp_check(bool snapshot_test);
+int swsusp_check(bool exclusive);
 extern void swsusp_free(void);
 extern int swsusp_read(unsigned int *flags_p);
 extern int swsusp_write(unsigned int flags);
-void swsusp_close(bool snapshot_test);
+void swsusp_close(bool exclusive);
 #ifdef CONFIG_SUSPEND
 extern int swsusp_unmark(void);
 #endif
diff --git a/kernel/power/swap.c b/kernel/power/swap.c
index f6ebcd00c410..74edbce2320b 100644
--- a/kernel/power/swap.c
+++ b/kernel/power/swap.c
@@ -1513,12 +1513,13 @@ int swsusp_read(unsigned int *flags_p)
 static void *swsusp_holder;
 
 /**
- *      swsusp_check - Check for swsusp signature in the resume device
+ * swsusp_check - Check for swsusp signature in the resume device
+ * @exclusive: Open the resume device exclusively.
  */
 
-int swsusp_check(bool snapshot_test)
+int swsusp_check(bool exclusive)
 {
-	void *holder = snapshot_test ? &swsusp_holder : NULL;
+	void *holder = exclusive ? &swsusp_holder : NULL;
 	int error;
 
 	hib_resume_bdev = blkdev_get_by_dev(swsusp_resume_device, BLK_OPEN_READ,
@@ -1563,17 +1564,18 @@ int swsusp_check(bool snapshot_test)
 }
 
 /**
- *	swsusp_close - close swap device.
+ * swsusp_close - close swap device.
+ * @exclusive: Close the resume device which is exclusively opened.
  */
 
-void swsusp_close(bool snapshot_test)
+void swsusp_close(bool exclusive)
 {
 	if (IS_ERR(hib_resume_bdev)) {
 		pr_debug("Image device not initialised\n");
 		return;
 	}
 
-	blkdev_put(hib_resume_bdev, snapshot_test ? &swsusp_holder : NULL);
+	blkdev_put(hib_resume_bdev, exclusive ? &swsusp_holder : NULL);
 }
 
 /**
-- 
2.25.1

[PATCH v2 2/2] PM: hibernate: Fix the exclusive get block device in test_resume mode

[Chen Yu]

commit 5904de0d735b ("PM: hibernate: Do not get block device exclusively
in test_resume mode") fixes a hibernation issue under test_resume mode.
That commit is supposed to open the block device in non-exclusive mode
when in test_resume. However the code did the opposite, which is against
its description.

In summary, the swap device is only opened exclusively by swsusp_check()
with its corresponding *close(), and must be in non test_resume mode.
This is to avoid the race condition that different processes scribble the
device at the same time. All the other cases should use non-exclusive mode.

Fix it by really disabling exclusive mode under test_resume.

Fixes: 5904de0d735b ("PM: hibernate: Do not get block device exclusively in test_resume mode")
Reported-by: syzbot+38d04642cea49f3a3d2e@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/lkml/000000000000761f5f0603324129@google.com/
Reported-by: Pengfei Xu <pengfei.xu@intel.com>
Signed-off-by: Chen Yu <yu.c.chen@intel.com>
---
 kernel/power/hibernate.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
index 2b4a946a6ff5..8d35b9f9aaa3 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -786,9 +786,9 @@ int hibernate(void)
 	unlock_device_hotplug();
 	if (snapshot_test) {
 		pm_pr_dbg("Checking hibernation image\n");
-		error = swsusp_check(snapshot_test);
+		error = swsusp_check(false);
 		if (!error)
-			error = load_image_and_restore(snapshot_test);
+			error = load_image_and_restore(false);
 	}
 	thaw_processes();
 
@@ -945,14 +945,14 @@ static int software_resume(void)
 	pm_pr_dbg("Looking for hibernation image.\n");
 
 	mutex_lock(&system_transition_mutex);
-	error = swsusp_check(false);
+	error = swsusp_check(true);
 	if (error)
 		goto Unlock;
 
 	/* The snapshot device should not be opened while we're running */
 	if (!hibernate_acquire()) {
 		error = -EBUSY;
-		swsusp_close(false);
+		swsusp_close(true);
 		goto Unlock;
 	}
 
@@ -973,7 +973,7 @@ static int software_resume(void)
 		goto Close_Finish;
 	}
 
-	error = load_image_and_restore(false);
+	error = load_image_and_restore(true);
 	thaw_processes();
  Finish:
 	pm_notifier_call_chain(PM_POST_RESTORE);
@@ -987,7 +987,7 @@ static int software_resume(void)
 	pm_pr_dbg("Hibernation image not present or could not be loaded.\n");
 	return error;
  Close_Finish:
-	swsusp_close(false);
+	swsusp_close(true);
 	goto Finish;
 }
 
-- 
2.25.1

回复: [PATCH v2 2/2] PM: hibernate: Fix the exclusive get block device in test_resume mode

[Feng, ChenzhouX]

> -----邮件原件-----
> 发件人: Chen, Yu C <yu.c.chen@intel.com>
> 发送时间: Wednesday, September 6, 2023 12:19
> 收件人: Rafael J. Wysocki <rafael@kernel.org>; Brown, Len
> <len.brown@intel.com>; Pavel Machek <pavel@ucw.cz>
> 抄送: linux-pm@vger.kernel.org; linux-kernel@vger.kernel.org; Feng, ChenzhouX
> <chenzhoux.feng@intel.com>; Xu, Pengfei <pengfei.xu@intel.com>; Chen, Yu C
> <yu.c.chen@intel.com>;
> syzbot+38d04642cea49f3a3d2e@syzkaller.appspotmail.com
> 主题: [PATCH v2 2/2] PM: hibernate: Fix the exclusive get block device in
> test_resume mode
> 
> commit 5904de0d735b ("PM: hibernate: Do not get block device exclusively in
> test_resume mode") fixes a hibernation issue under test_resume mode.
> That commit is supposed to open the block device in non-exclusive mode when in
> test_resume. However the code did the opposite, which is against its description.
> 
> In summary, the swap device is only opened exclusively by swsusp_check() with
> its corresponding *close(), and must be in non test_resume mode.
> This is to avoid the race condition that different processes scribble the device at
> the same time. All the other cases should use non-exclusive mode.
> 
> Fix it by really disabling exclusive mode under test_resume.
> 
> Fixes: 5904de0d735b ("PM: hibernate: Do not get block device exclusively in
> test_resume mode")
> Reported-by: syzbot+38d04642cea49f3a3d2e@syzkaller.appspotmail.com
> Closes:
> https://lore.kernel.org/lkml/000000000000761f5f0603324129@google.com/
> Reported-by: Pengfei Xu <pengfei.xu@intel.com>
> Signed-off-by: Chen Yu <yu.c.chen@intel.com>

It works on my machine, for the whole series,
Tested-by: Chenzhou Feng <chenzhoux.feng@intel.com>

thanks,
Chenzhou