diff mbox

[3/3] dm-crypt: Adds support for wiping key when doing suspend/hibernation

Message ID 1428254419-7334-4-git-send-email-pali.rohar@gmail.com (mailing list archive)
State RFC, archived
Headers show

Commit Message

Pali Rohár April 5, 2015, 5:20 p.m. UTC 
This patch adds dm message commands and option strings to optionally wipe key
from dm-crypt device before entering suspend or hibernate state.

Before key is wiped dm device must be suspended. To prevent race conditions with
I/O and userspace processes, wiping action must be called after processes are
freezed. Otherwise userspace processes could start reading/writing to disk after
dm device is suspened and freezing processes before suspend/hibernate action
will fail.

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
---
 drivers/md/dm-crypt.c |  109 +++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 102 insertions(+), 7 deletions(-)

Comments

Alasdair G Kergon April 7, 2015, 1:55 p.m. UTC | #1 
On Sun, Apr 05, 2015 at 07:20:19PM +0200, Pali Rohár wrote:
> This patch adds dm message commands and option strings to optionally wipe key
> from dm-crypt device before entering suspend or hibernate state.
 
Try to avoid 0/1 - use descriptive options instead. 
E.g. key wipe_on_hibernation / key retain_on_hibernation  (message)
     wipe_key_on_hiberation ('dmsetup table' - don't forget the reporting interface!)

Have you tested against every state the driver might be in at the time of
suspend/hibernation?

> +static void crypt_suspend_and_wipe_key(struct crypt_config *cc)
> +{
> +	dm_suspend_md(dm_table_get_md(cc->ti->table));

I'm not particularly keen on this - silently ignoring expected error states
like -EINVAL rather than checking first and not calling the function at all
when it's known not to be needed.

Alasdair

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 713a962..9b02824 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -23,6 +23,7 @@ 
 #include <linux/atomic.h>
 #include <linux/scatterlist.h>
 #include <linux/rbtree.h>
+#include <linux/suspend.h>
 #include <asm/page.h>
 #include <asm/unaligned.h>
 #include <crypto/hash.h>
@@ -31,6 +32,8 @@ 
 
 #include <linux/device-mapper.h>
 
+#include "dm.h"
+
 #define DM_MSG_PREFIX "crypt"
 
 /*
@@ -112,13 +115,18 @@  struct iv_tcw_private {
  * and encrypts / decrypts at the same time.
  */
 enum flags { DM_CRYPT_SUSPENDED, DM_CRYPT_KEY_VALID,
-	     DM_CRYPT_SAME_CPU, DM_CRYPT_NO_OFFLOAD };
+	     DM_CRYPT_SAME_CPU, DM_CRYPT_NO_OFFLOAD,
+	     DM_CRYPT_KEY_WIPE_ON_HIBERNATION,
+	     DM_CRYPT_KEY_WIPE_ON_SUSPEND,
+};
 
 /*
  * The fields in here must be read only after initialization.
  */
 struct crypt_config {
 	struct dm_dev *dev;
+	struct dm_target *ti;
+	struct list_head entry;
 	sector_t start;
 
 	/*
@@ -181,6 +189,9 @@  struct crypt_config {
 
 #define MIN_IOS        16
 
+static LIST_HEAD(crypt_list);
+static DEFINE_MUTEX(crypt_list_mtx);
+
 static void clone_init(struct dm_crypt_io *, struct bio *);
 static void kcryptd_queue_crypt(struct dm_crypt_io *io);
 static u8 *iv_of_dmreq(struct crypt_config *cc, struct dm_crypt_request *dmreq);
@@ -1497,12 +1508,26 @@  out:
 
 static int crypt_wipe_key(struct crypt_config *cc)
 {
+	int ret;
+
+	if (cc->iv_gen_ops && cc->iv_gen_ops->wipe) {
+		ret = cc->iv_gen_ops->wipe(cc);
+		if (ret)
+			return ret;
+	}
+
 	clear_bit(DM_CRYPT_KEY_VALID, &cc->flags);
 	memset(&cc->key, 0, cc->key_size * sizeof(u8));
 
 	return crypt_setkey_allcpus(cc);
 }
 
+static void crypt_suspend_and_wipe_key(struct crypt_config *cc)
+{
+	dm_suspend_md(dm_table_get_md(cc->ti->table));
+	crypt_wipe_key(cc);
+}
+
 static void crypt_dtr(struct dm_target *ti)
 {
 	struct crypt_config *cc = ti->private;
@@ -1512,6 +1537,10 @@  static void crypt_dtr(struct dm_target *ti)
 	if (!cc)
 		return;
 
+	mutex_lock(&crypt_list_mtx);
+	list_del(&cc->entry);
+	mutex_unlock(&crypt_list_mtx);
+
 	if (cc->write_thread)
 		kthread_stop(cc->write_thread);
 
@@ -1738,6 +1767,7 @@  static int crypt_ctr(struct dm_target *ti, unsigned int argc, char **argv)
 	cc->key_size = key_size;
 
 	ti->private = cc;
+	cc->ti = ti;
 	ret = crypt_ctr_cipher(ti, argv[0], argv[1]);
 	if (ret < 0)
 		goto bad;
@@ -1832,7 +1862,14 @@  static int crypt_ctr(struct dm_target *ti, unsigned int argc, char **argv)
 			else if (!strcasecmp(opt_string, "submit_from_crypt_cpus"))
 				set_bit(DM_CRYPT_NO_OFFLOAD, &cc->flags);
 
+			else if (!strcasecmp(opt_string, "key_wipe_on_hibernation"))
+				set_bit(DM_CRYPT_KEY_WIPE_ON_HIBERNATION, &cc->flags);
+
+			else if (!strcasecmp(opt_string, "key_wipe_on_suspend"))
+				set_bit(DM_CRYPT_KEY_WIPE_ON_SUSPEND, &cc->flags);
+
 			else {
+				ret = -EINVAL;
 				ti->error = "Invalid feature arguments";
 				goto bad;
 			}
@@ -1871,6 +1908,10 @@  static int crypt_ctr(struct dm_target *ti, unsigned int argc, char **argv)
 	ti->num_flush_bios = 1;
 	ti->discard_zeroes_data_unsupported = true;
 
+	mutex_lock(&crypt_list_mtx);
+	list_add(&cc->entry, &crypt_list);
+	mutex_unlock(&crypt_list_mtx);
+
 	return 0;
 
 bad:
@@ -1979,6 +2020,8 @@  static void crypt_resume(struct dm_target *ti)
 /* Message interface
  *	key set <key>
  *	key wipe
+ *	key wipe_on_hibernation <0|1>
+ *	key wipe_on_suspend <0|1>
  */
 static int crypt_message(struct dm_target *ti, unsigned argc, char **argv)
 {
@@ -1989,6 +2032,30 @@  static int crypt_message(struct dm_target *ti, unsigned argc, char **argv)
 		goto error;
 
 	if (!strcasecmp(argv[0], "key")) {
+		if (argc == 3 && !strcasecmp(argv[1], "wipe_on_hibernation")) {
+			if (!strcmp(argv[2], "1")) {
+				set_bit(DM_CRYPT_KEY_WIPE_ON_HIBERNATION, &cc->flags);
+				return 0;
+			} else if (!strcmp(argv[2], "0")) {
+				clear_bit(DM_CRYPT_KEY_WIPE_ON_HIBERNATION, &cc->flags);
+				return 0;
+			} else {
+				DMWARN("unrecognised message received.");
+				return -EINVAL;
+			}
+		}
+		if (argc == 3 && !strcasecmp(argv[1], "wipe_on_suspend")) {
+			if (!strcmp(argv[2], "1")) {
+				set_bit(DM_CRYPT_KEY_WIPE_ON_SUSPEND, &cc->flags);
+				return 0;
+			} else if (!strcmp(argv[2], "0")) {
+				clear_bit(DM_CRYPT_KEY_WIPE_ON_SUSPEND, &cc->flags);
+				return 0;
+			} else {
+				DMWARN("unrecognised message received.");
+				return -EINVAL;
+			}
+		}
 		if (!test_bit(DM_CRYPT_SUSPENDED, &cc->flags)) {
 			DMWARN("not suspended during key manipulation.");
 			return -EINVAL;
@@ -2002,11 +2069,6 @@  static int crypt_message(struct dm_target *ti, unsigned argc, char **argv)
 			return ret;
 		}
 		if (argc == 2 && !strcasecmp(argv[1], "wipe")) {
-			if (cc->iv_gen_ops && cc->iv_gen_ops->wipe) {
-				ret = cc->iv_gen_ops->wipe(cc);
-				if (ret)
-					return ret;
-			}
 			return crypt_wipe_key(cc);
 		}
 	}
@@ -2055,19 +2117,52 @@  static struct target_type crypt_target = {
 	.iterate_devices = crypt_iterate_devices,
 };
 
+static int dm_crypt_pm_notifier_call(struct notifier_block *nb,
+				     unsigned long action, void *data)
+{
+	struct crypt_config *cc;
+
+	mutex_lock(&crypt_list_mtx);
+
+	list_for_each_entry(cc, &crypt_list, entry) {
+		if ((action == PM_HIBERNATION_AFTER_FREEZE &&
+		     test_bit(DM_CRYPT_KEY_WIPE_ON_HIBERNATION, &cc->flags)) ||
+		    (action == PM_SUSPEND_AFTER_FREEZE &&
+		     test_bit(DM_CRYPT_KEY_WIPE_ON_SUSPEND, &cc->flags))) {
+			crypt_suspend_and_wipe_key(cc);
+		}
+	}
+
+	mutex_unlock(&crypt_list_mtx);
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block dm_crypt_pm_notifier_block = {
+	.notifier_call = dm_crypt_pm_notifier_call,
+};
+
 static int __init dm_crypt_init(void)
 {
 	int r;
 
 	r = dm_register_target(&crypt_target);
-	if (r < 0)
+	if (r < 0) {
 		DMERR("register failed %d", r);
+		return r;
+	}
+
+	r = register_pm_notifier(&dm_crypt_pm_notifier_block);
+	if (r) {
+		DMWARN("register_pm_notifier failed %d", r);
+	}
 
 	return r;
 }
 
 static void __exit dm_crypt_exit(void)
 {
+	unregister_pm_notifier(&dm_crypt_pm_notifier_block);
 	dm_unregister_target(&crypt_target);
 }