Skip to contentSkip to conversation

hibernate: add config option for snapshot device

PATCH New Domenico Andreoli 1 participant 3 messages May 1, 2020

Subsystems (2)
Suspend to RAM Hibernation
lore
Share & tools
lore

Domenico Andreoli

author
#0 lore raw

v2 cover hibernate: add config option for snapshot device

Dear maintainers,

These are the improvements over v1 [0]:

- move the mutual exclusion to hibernate.c (it was in user.c)
- switched to refcount_t API (it was atomic_t)
- the option is enabled by default (it was disabled by default)
- remove the premature "DEPRECATED" marking
- drop the redefinition of mutual exclusion helpers (in v2 they are owned by hibernte.c) in case the option is deselected
- add the help message to the config option

Kind regards,
Domenico

[0] https://lore.kernel.org/linux-pm/[email protected]/

Signature · 2 lines
rsa4096: 3B10 0CA1 8674 ACBA B4FE  FCD2 CE5B CF17 9960 DE13
ed25519: FFB4 0CC3 7F2E 091D F7DA  356E CC79 2832 ED38 CB05
author
#1 lore raw

PATCH v2 1/2 hibernate: incorporate concurrency handling

From: Domenico Andreoli <[email protected]>

Hibernation concurrency handling is currently delegated to user.c, where it's also used for regulating the access to the snapshot device.

In the prospective of making user.c a separate configuration option, such mutual exclusion is brought into hibernate.c and made available through accessor helpers hereby introduced.

v2:
- move the mutual exclusion to hibernate.c (it was in user.c)
- switched to refcount_t API (it was atomic_t)

Signed-off-by Domenico Andreoli <[email protected]>
Cc "Rafael J. Wysocki" <[email protected]>
Cc Pavel Machek <[email protected]>
Cc Linux PM <[email protected]>
---
 kernel/power/hibernate.c |   20 ++++++++++++++++----
 kernel/power/power.h     |    4 ++--
 kernel/power/user.c      |   10 ++++------
 3 files changed, 22 insertions(+), 12 deletions(-)
kernel/power/user.c +4 -6
Index: b/kernel/power/user.c
===================================================================
@@ -37,8 +37,6 @@ static struct snapshot_data {
 	bool free_bitmaps;
 } snapshot_state;
 
-atomic_t snapshot_device_available = ATOMIC_INIT(1);
-
 static int snapshot_open(struct inode *inode, struct file *filp)
 {
 	struct snapshot_data *data;

@@ -49,13 +47,13 @@ static int snapshot_open(struct inode *i
 
 	lock_system_sleep();
 
-	if (!atomic_add_unless(&snapshot_device_available, -1, 0)) {
+	if (!hibernate_acquire()) {
 		error = -EBUSY;
 		goto Unlock;
 	}
 
 	if ((filp->f_flags & O_ACCMODE) == O_RDWR) {
-		atomic_inc(&snapshot_device_available);
+		hibernate_release();
 		error = -ENOSYS;
 		goto Unlock;
 	}

@@ -92,7 +90,7 @@ static int snapshot_open(struct inode *i
 			__pm_notifier_call_chain(PM_POST_RESTORE, nr_calls, NULL);
 	}
 	if (error)
-		atomic_inc(&snapshot_device_available);
+		hibernate_release();
 
 	data->frozen = false;
 	data->ready = false;

@@ -122,7 +120,7 @@ static int snapshot_release(struct inode
 	}
 	pm_notifier_call_chain(data->mode == O_RDONLY ?
 			PM_POST_HIBERNATION : PM_POST_RESTORE);
-	atomic_inc(&snapshot_device_available);
+	hibernate_release();
 
 	unlock_system_sleep();
kernel/power/power.h +2 -2
Index: b/kernel/power/power.h
===================================================================
@@ -154,8 +154,8 @@ extern int snapshot_write_next(struct sn
 extern void snapshot_write_finalize(struct snapshot_handle *handle);
 extern int snapshot_image_loaded(struct snapshot_handle *handle);
 
-/* If unset, the snapshot device cannot be open. */
-extern atomic_t snapshot_device_available;
+extern bool hibernate_acquire(void);
+extern void hibernate_release(void);
 
 extern sector_t alloc_swapdev_block(int swap);
 extern void free_all_swap_pages(int swap);
kernel/power/hibernate.c +16 -4
Index: b/kernel/power/hibernate.c
===================================================================
@@ -67,6 +67,18 @@ bool freezer_test_done;
 
 static const struct platform_hibernation_ops *hibernation_ops;
 
+static refcount_t hibernate_refcount = REFCOUNT_INIT(1);
+
+bool hibernate_acquire(void)
+{
+	return refcount_add_not_zero(-1, &hibernate_refcount);
+}
+
+void hibernate_release(void)
+{
+	refcount_inc(&hibernate_refcount);
+}
+
 bool hibernation_available(void)
 {
 	return nohibernate == 0 && !security_locked_down(LOCKDOWN_HIBERNATION);

@@ -704,7 +716,7 @@ int hibernate(void)
 
 	lock_system_sleep();
 	/* The snapshot device should not be opened while we're running */
-	if (!atomic_add_unless(&snapshot_device_available, -1, 0)) {
+	if (!hibernate_acquire()) {
 		error = -EBUSY;
 		goto Unlock;
 	}

@@ -775,7 +787,7 @@ int hibernate(void)
  Exit:
 	__pm_notifier_call_chain(PM_POST_HIBERNATION, nr_calls, NULL);
 	pm_restore_console();
-	atomic_inc(&snapshot_device_available);
+	hibernate_release();
  Unlock:
 	unlock_system_sleep();
 	pr_info("hibernation exit\n");

@@ -880,7 +892,7 @@ static int software_resume(void)
 		goto Unlock;
 
 	/* The snapshot device should not be opened while we're running */
-	if (!atomic_add_unless(&snapshot_device_available, -1, 0)) {
+	if (!hibernate_acquire()) {
 		error = -EBUSY;
 		swsusp_close(FMODE_READ);
 		goto Unlock;

@@ -904,7 +916,7 @@ static int software_resume(void)
 	__pm_notifier_call_chain(PM_POST_RESTORE, nr_calls, NULL);
 	pm_restore_console();
 	pr_info("resume failed (%d)\n", error);
-	atomic_inc(&snapshot_device_available);
+	hibernate_release();
 	/* For success case, the suspend path will release the lock */
  Unlock:
 	mutex_unlock(&system_transition_mutex);
author
#2 lore raw

PATCH v2 2/2 hibernate: split snapshot dev option

In reply to Domenico Andreoli

From: Domenico Andreoli <[email protected]>

Make it possible to reduce the attack surface in case the snapshot device is not to be used from userspace.

v2:
- the option is enabled by default (it was disabled by default)
- remove the premature "DEPRECATED" marking
- drop the redefinition of mutual exclusion helpers (in v2 they are owned by hibernte.c) in case the option is deselected
- add the help message to the config option

Signed-off-by Domenico Andreoli <[email protected]>
Cc "Rafael J. Wysocki" <[email protected]>
Cc Pavel Machek <[email protected]>
Cc Linux PM <[email protected]>
---
 kernel/power/Kconfig  |   12 ++++++++++++
 kernel/power/Makefile |    3 ++-
 2 files changed, 14 insertions(+), 1 deletion(-)
kernel/power/Kconfig +12 -0
Index: b/kernel/power/Kconfig
===================================================================
@@ -80,6 +80,18 @@ config HIBERNATION
 
 	  For more information take a look at <file:Documentation/power/swsusp.rst>.
 
+config HIBERNATION_SNAPSHOT_DEV
+	bool "Userspace snapshot device"
+	depends on HIBERNATION
+	default y
+	---help---
+	  Device used by the uswsusp tools.
+
+	  Say N if no snapshotting from userspace is needed, this also
+	  reduces the attack surface of the kernel.
+
+	  If in doubt, say Y.
+
 config PM_STD_PARTITION
 	string "Default resume partition"
 	depends on HIBERNATION
kernel/power/Makefile +2 -1
Index: b/kernel/power/Makefile
===================================================================
@@ -10,7 +10,8 @@ obj-$(CONFIG_VT_CONSOLE_SLEEP)	+= consol
 obj-$(CONFIG_FREEZER)		+= process.o
 obj-$(CONFIG_SUSPEND)		+= suspend.o
 obj-$(CONFIG_PM_TEST_SUSPEND)	+= suspend_test.o
-obj-$(CONFIG_HIBERNATION)	+= hibernate.o snapshot.o swap.o user.o
+obj-$(CONFIG_HIBERNATION)	+= hibernate.o snapshot.o swap.o
+obj-$(CONFIG_HIBERNATION_SNAPSHOT_DEV) += user.o
 obj-$(CONFIG_PM_AUTOSLEEP)	+= autosleep.o
 obj-$(CONFIG_PM_WAKELOCKS)	+= wakelock.o