linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/3] init: add support to directly boot to a mapped device
@ 2016-02-20 18:13 Kees Cook
  2016-02-20 18:13 ` [PATCH v5 1/3] dm: export a table+mapped device to the ioctl interface Kees Cook
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Kees Cook @ 2016-02-20 18:13 UTC (permalink / raw)
  To: Alasdair Kergon
  Cc: Kees Cook, Mike Snitzer, dm-devel, Jonathan Corbet, Shaohua Li,
	Dan Ehrenberg, Rafael J. Wysocki, Chen Yu, Vishnu Pratap Singh,
	Andrew Morton, Yaowei Bai, linux-doc, linux-kernel, linux-raid,
	Will Drewry, David Zeuthen

This is a resurrection of a patch series from a few years back, first
brought to the dm maintainers in 2010. It creates a way to define dm
devices on the kernel command line for systems that do not use an
initramfs, or otherwise need a dm running before init starts.

This has been used by Chrome OS for several years, and now by Brillo
(and likely Android soon).

The last version was v4:
https://patchwork.kernel.org/patch/104860/
https://patchwork.kernel.org/patch/104861/

-Kees

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v5 1/3] dm: export a table+mapped device to the ioctl interface
  2016-02-20 18:13 [PATCH v5 0/3] init: add support to directly boot to a mapped device Kees Cook
@ 2016-02-20 18:13 ` Kees Cook
  2016-02-20 18:13 ` [PATCH v5 2/3] dm: make mapped_device locking functions available Kees Cook
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 11+ messages in thread
From: Kees Cook @ 2016-02-20 18:13 UTC (permalink / raw)
  To: Alasdair Kergon
  Cc: Kees Cook, Will Drewry, Mike Snitzer, dm-devel, Jonathan Corbet,
	Shaohua Li, Dan Ehrenberg, Rafael J. Wysocki, Chen Yu,
	Vishnu Pratap Singh, Andrew Morton, Yaowei Bai, linux-doc,
	linux-kernel, linux-raid, David Zeuthen

From: Will Drewry <wad@chromium.org>

If a mapped device and table is configured without traversing the dm-ioctl
interface (dm-fs-style), then it will not be bound to a name or uuid. This
means that it will be inaccessible for userspace management and udev
will be unhappy with the lack of a name or uuid.

The function added in this change performs the required association to
transition to being managed by the ioctl interface.

Signed-off-by: Will Drewry <wad@chromium.org>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
v5: resurrection
v4: https://patchwork.kernel.org/patch/104860/
---
 drivers/md/dm-ioctl.c         | 35 +++++++++++++++++++++++++++++++++++
 include/linux/device-mapper.h |  6 ++++++
 2 files changed, 41 insertions(+)

diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
index 80a439543259..e0efc6844b3a 100644
--- a/drivers/md/dm-ioctl.c
+++ b/drivers/md/dm-ioctl.c
@@ -1923,6 +1923,41 @@ void dm_interface_exit(void)
 	dm_hash_exit();
 }
 
+
+/**
+ * dm_ioctl_export - Permanently export a mapped device via the ioctl interface
+ * @md: Pointer to mapped_device
+ * @name: Buffer (size DM_NAME_LEN) for name
+ * @uuid: Buffer (size DM_UUID_LEN) for uuid or NULL if not desired
+ */
+int dm_ioctl_export(struct mapped_device *md, const char *name,
+		    const char *uuid)
+{
+	int r = 0;
+	struct hash_cell *hc;
+
+	if (!md)
+		return -ENXIO;
+
+	/* The name and uuid can only be set once. */
+	mutex_lock(&dm_hash_cells_mutex);
+	hc = dm_get_mdptr(md);
+	mutex_unlock(&dm_hash_cells_mutex);
+	if (hc) {
+		DMERR("%s: already exported", dm_device_name(md));
+		return -ENXIO;
+	}
+
+	r = dm_hash_insert(name, uuid, md);
+	if (r) {
+		DMERR("%s: could not bind to '%s'", dm_device_name(md), name);
+		return r;
+	}
+
+	/* Let udev know we've changed. */
+	dm_kobject_uevent(md, KOBJ_CHANGE, dm_get_event_nr(md));
+	return r;
+}
 /**
  * dm_copy_name_and_uuid - Copy mapped device name & uuid into supplied buffers
  * @md: Pointer to mapped_device
diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h
index ec1c61c87d89..87afa0552398 100644
--- a/include/linux/device-mapper.h
+++ b/include/linux/device-mapper.h
@@ -381,6 +381,12 @@ void dm_set_mdptr(struct mapped_device *md, void *ptr);
 void *dm_get_mdptr(struct mapped_device *md);
 
 /*
+ * Export the device via the ioctl interface (uses mdptr).
+ */
+int dm_ioctl_export(struct mapped_device *md, const char *name,
+		    const char *uuid);
+
+/*
  * A device can still be used while suspended, but I/O is deferred.
  */
 int dm_suspend(struct mapped_device *md, unsigned suspend_flags);
-- 
2.6.3

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v5 2/3] dm: make mapped_device locking functions available
  2016-02-20 18:13 [PATCH v5 0/3] init: add support to directly boot to a mapped device Kees Cook
  2016-02-20 18:13 ` [PATCH v5 1/3] dm: export a table+mapped device to the ioctl interface Kees Cook
@ 2016-02-20 18:13 ` Kees Cook
  2016-02-20 18:13 ` [PATCH v5 3/3] init: add support to directly boot to a mapped device Kees Cook
  2016-02-21 22:08 ` [PATCH v5 0/3] " Alasdair G Kergon
  3 siblings, 0 replies; 11+ messages in thread
From: Kees Cook @ 2016-02-20 18:13 UTC (permalink / raw)
  To: Alasdair Kergon
  Cc: Kees Cook, Mike Snitzer, dm-devel, Jonathan Corbet, Shaohua Li,
	Dan Ehrenberg, Rafael J. Wysocki, Chen Yu, Vishnu Pratap Singh,
	Andrew Morton, Yaowei Bai, linux-doc, linux-kernel, linux-raid,
	Will Drewry, David Zeuthen

For init to build a mapped_device, it must hold the appropriate locks,
so move these to the common header.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
v5: first version of this specific patch in the series
---
 drivers/md/dm.h               | 2 --
 include/linux/device-mapper.h | 6 ++++++
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/md/dm.h b/drivers/md/dm.h
index 7edcf97dfa5a..f21700431f67 100644
--- a/drivers/md/dm.h
+++ b/drivers/md/dm.h
@@ -78,8 +78,6 @@ bool dm_table_mq_request_based(struct dm_table *t);
 void dm_table_free_md_mempools(struct dm_table *t);
 struct dm_md_mempools *dm_table_get_md_mempools(struct dm_table *t);
 
-void dm_lock_md_type(struct mapped_device *md);
-void dm_unlock_md_type(struct mapped_device *md);
 void dm_set_md_type(struct mapped_device *md, unsigned type);
 unsigned dm_get_md_type(struct mapped_device *md);
 struct target_type *dm_get_immutable_target_type(struct mapped_device *md);
diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h
index 87afa0552398..48df518345fd 100644
--- a/include/linux/device-mapper.h
+++ b/include/linux/device-mapper.h
@@ -414,6 +414,12 @@ union map_info *dm_get_rq_mapinfo(struct request *rq);
 struct queue_limits *dm_get_queue_limits(struct mapped_device *md);
 
 /*
+ * Lock functions.
+ */
+void dm_lock_md_type(struct mapped_device *md);
+void dm_unlock_md_type(struct mapped_device *md);
+
+/*
  * Geometry functions.
  */
 int dm_get_geometry(struct mapped_device *md, struct hd_geometry *geo);
-- 
2.6.3

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v5 3/3] init: add support to directly boot to a mapped device
  2016-02-20 18:13 [PATCH v5 0/3] init: add support to directly boot to a mapped device Kees Cook
  2016-02-20 18:13 ` [PATCH v5 1/3] dm: export a table+mapped device to the ioctl interface Kees Cook
  2016-02-20 18:13 ` [PATCH v5 2/3] dm: make mapped_device locking functions available Kees Cook
@ 2016-02-20 18:13 ` Kees Cook
  2016-02-21 22:08 ` [PATCH v5 0/3] " Alasdair G Kergon
  3 siblings, 0 replies; 11+ messages in thread
From: Kees Cook @ 2016-02-20 18:13 UTC (permalink / raw)
  To: Alasdair Kergon
  Cc: Kees Cook, Mike Snitzer, dm-devel, Jonathan Corbet, Shaohua Li,
	Dan Ehrenberg, Rafael J. Wysocki, Chen Yu, Vishnu Pratap Singh,
	Andrew Morton, Yaowei Bai, linux-doc, linux-kernel, linux-raid,
	Will Drewry, David Zeuthen

This change adds a dm= kernel parameter modelled after the md= parameter
from do_mounts_md. It allows for simple device-mapper targets to be
configured at boot time for use early in the boot process (as the root
device or otherwise).

The format is as follows:
dm="count name uuid|none ro|rw num-tables, table line 1, table line 2, ..."

It relies on the ability to create a device mapper device programmatically
and bind that device to the ioctl-style name/uuid.

Based on work by Will Drewry and Paul Taysom.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
v5: resurrection, multiple devices, cleanups, error reporting improvements
v4: https://patchwork.kernel.org/patch/104861/
---
 Documentation/device-mapper/boot.txt |  64 ++++++
 Documentation/kernel-parameters.txt  |   4 +
 init/Makefile                        |   1 +
 init/do_mounts.c                     |   1 +
 init/do_mounts.h                     |  10 +
 init/do_mounts_dm.c                  | 433 +++++++++++++++++++++++++++++++++++
 6 files changed, 513 insertions(+)
 create mode 100644 Documentation/device-mapper/boot.txt
 create mode 100644 init/do_mounts_dm.c

diff --git a/Documentation/device-mapper/boot.txt b/Documentation/device-mapper/boot.txt
new file mode 100644
index 000000000000..9dd04b37b3b1
--- /dev/null
+++ b/Documentation/device-mapper/boot.txt
@@ -0,0 +1,64 @@
+Boot time creation of mapped devices
+====================================
+It is possible to configure a device mapper device to act as the root
+device for your system in two ways.
+
+The first is to build an initial ramdisk which boots to a minimal
+userspace which configures the device, then pivot_root(8) in to it.
+
+The second is to possible when the device-mapper and any targets are
+compiled into the kernel (not a module), one or more device-mappers may
+be created and used as the root device at boot time with the parameters
+given with the boot line dm=...
+
+Multiple device-mappers can be stacked by specifying the number of
+devices. A device can have multiple tables if the the number of tables
+is specified.
+
+	<dm>		::= <num-mappers> <device-mapper>+
+	<device-mapper>	::= <head> "," <table>+
+	<head>		::= <name> <uuid> <mode> [<num-tables>]
+	<table>		::= <start> <length> <type> <options> ","
+	<mode>		::= "ro" | "rw"
+	<uuid>		::= xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx | "none"
+	<type>		::= "verity" | "bootcache" | ...
+
+Each tables line may be as normal when using the dmsetup tool except for
+two variations:
+1. Any use of commas will be interpreted as a newline
+2. Quotation marks cannot be escaped and cannot be used without
+   terminating the dm= argument.
+
+Unless renamed by udev, the device node created will be dm-0 as the
+first minor number for the device-mapper is used during early creation.
+
+The <num-tables> field is optional and assumed to be 1.
+
+Examples
+========
+An example of booting to a linear array made up of user-mode linux block
+devices:
+
+  dm="1 lroot none rw 2, 0 4096 linear 98:16 0, 4096 4096 linear 98:32 0" \
+  root=/dev/dm-0
+
+This will boot to a rw dm-linear target of 8192 sectors split across two
+block devices identified by their major:minor numbers.  After boot, udev
+will rename this target to /dev/mapper/lroot (depending on the rules).
+No uuid was assigned.
+
+An example of multiple device-mappers, with the dm="..." contents shown
+here split on multiple lines for readability:
+
+  3 vboot none ro,
+      0 1768000 bootcache
+        device=aa55b119-2a47-8c45-946a-5ac57765011f+1
+        signature=76e9be054b15884a9fa85973e9cb274c93afadb6
+        cache_start=1768000 max_blocks=100000 size_limit=23 max_trace=20000,
+    vroot none ro,
+      0 1740800 verity payload=254:0 hashtree=254:0 hashstart=1740800 alg=sha1
+        root_hexdigest=76e9be054b15884a9fa85973e9cb274c93afadb6
+        salt=5b3549d54d6c7a3837b9b81ed72e49463a64c03680c47835bef94d768e5646fe,
+    vram none rw 2,
+      0 32768 linear 1:0 0,
+      32768 32768 linear 1:1 0,
diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 9a53c929f017..ba3a49bc1229 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -56,6 +56,7 @@ parameter is applicable:
 	BLACKFIN Blackfin architecture is enabled.
 	CLK	Common clock infrastructure is enabled.
 	CMA	Contiguous Memory Area support is enabled.
+	DM	Device mapper support is enabled.
 	DRM	Direct Rendering Management support is enabled.
 	DYNAMIC_DEBUG Build in debug messages and enable them at runtime
 	EDD	BIOS Enhanced Disk Drive Services (EDD) is enabled
@@ -930,6 +931,9 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 
 	dis_ucode_ldr	[X86] Disable the microcode loader.
 
+	dm=		[DM] Allows early creation of a device-mapper device.
+			See Documentation/device-mapper/boot.txt.
+
 	dma_debug=off	If the kernel is compiled with DMA_API_DEBUG support,
 			this option disables the debugging code at boot.
 
diff --git a/init/Makefile b/init/Makefile
index 7bc47ee31c36..90410fd7b658 100644
--- a/init/Makefile
+++ b/init/Makefile
@@ -18,6 +18,7 @@ mounts-y			:= do_mounts.o
 mounts-$(CONFIG_BLK_DEV_RAM)	+= do_mounts_rd.o
 mounts-$(CONFIG_BLK_DEV_INITRD)	+= do_mounts_initrd.o
 mounts-$(CONFIG_BLK_DEV_MD)	+= do_mounts_md.o
+mounts-$(CONFIG_BLK_DEV_DM)	+= do_mounts_dm.o
 
 # dependencies on generated files need to be listed explicitly
 $(obj)/version.o: include/generated/compile.h
diff --git a/init/do_mounts.c b/init/do_mounts.c
index dea5de95c2dd..1902a1c80831 100644
--- a/init/do_mounts.c
+++ b/init/do_mounts.c
@@ -566,6 +566,7 @@ void __init prepare_namespace(void)
 	wait_for_device_probe();
 
 	md_run_setup();
+	dm_run_setup();
 
 	if (saved_root_name[0]) {
 		root_device_name = saved_root_name;
diff --git a/init/do_mounts.h b/init/do_mounts.h
index 067af1d9e8b6..ecb275782c03 100644
--- a/init/do_mounts.h
+++ b/init/do_mounts.h
@@ -74,3 +74,13 @@ void md_run_setup(void);
 static inline void md_run_setup(void) {}
 
 #endif
+
+#ifdef CONFIG_BLK_DEV_DM
+
+void dm_run_setup(void);
+
+#else
+
+static inline void dm_run_setup(void) {}
+
+#endif
diff --git a/init/do_mounts_dm.c b/init/do_mounts_dm.c
new file mode 100644
index 000000000000..331a0d551a34
--- /dev/null
+++ b/init/do_mounts_dm.c
@@ -0,0 +1,433 @@
+/*
+ * do_mounts_dm.c
+ * Copyright (C) 2010 The Chromium OS Authors <chromium-os-dev@chromium.org>
+ * Based on do_mounts_md.c
+ *
+ * This file is released under the GPLv2.
+ */
+#include <linux/async.h>
+#include <linux/ctype.h>
+#include <linux/device-mapper.h>
+#include <linux/fs.h>
+#include <linux/string.h>
+#include <linux/delay.h>
+
+#include "do_mounts.h"
+
+#define DM_MAX_DEVICES 256
+#define DM_MAX_TARGETS 256
+#define DM_MAX_NAME 32
+#define DM_MAX_UUID 129
+#define DM_NO_UUID "none"
+
+#define DM_MSG_PREFIX "init"
+#define DMERR_PARSE(fmt, args...) \
+	DMERR("failed to parse " fmt " for device %s<%lu>", args)
+
+/* Separators used for parsing the dm= argument. */
+#define DM_FIELD_SEP " "
+#define DM_LINE_SEP ","
+#define DM_ANY_SEP DM_FIELD_SEP DM_LINE_SEP
+
+/* See Documentation/device-mapper/boot.txt for dm="..." format details. */
+
+struct dm_setup_table {
+	sector_t begin;
+	sector_t length;
+	char *type;
+	char *params;
+	/* simple singly linked list */
+	struct dm_setup_table *next;
+};
+
+struct dm_device {
+	int minor;
+	int ro;
+	char name[DM_MAX_NAME];
+	char uuid[DM_MAX_UUID];
+	unsigned long num_tables;
+	struct dm_setup_table *table;
+	int table_count;
+	struct dm_device *next;
+};
+
+struct dm_option {
+	char *start;
+	char *next;
+	size_t len;
+	char delim;
+};
+
+static struct {
+	unsigned long num_devices;
+	char *str;
+} dm_setup_args __initdata;
+
+static int dm_early_setup __initdata;
+
+static int __init get_dm_option(struct dm_option *opt, const char *accept)
+{
+	char *str = opt->next;
+	char *endp;
+
+	if (!str)
+		return 0;
+
+	str = skip_spaces(str);
+	opt->start = str;
+	endp = strpbrk(str, accept);
+	if (!endp) {  /* act like strchrnul */
+		opt->len = strlen(str);
+		endp = str + opt->len;
+	} else {
+		opt->len = endp - str;
+	}
+	opt->delim = *endp;
+	if (*endp == 0) {
+		/* Don't advance past the nul. */
+		opt->next = endp;
+	} else {
+		opt->next = endp + 1;
+	}
+	return opt->len != 0;
+}
+
+static int __init get_dm_option_u64(struct dm_option *opt, const char *sep,
+				    unsigned long long *result)
+{
+	char buf[32];
+
+	if (!get_dm_option(opt, sep))
+		return -EINVAL;
+
+	strlcpy(buf, opt->start, min(sizeof(buf), opt->len + 1));
+	return kstrtoull(buf, 0, result);
+}
+
+static void __init dm_setup_cleanup(struct dm_device *devices)
+{
+	struct dm_device *dev = devices;
+
+	while (dev) {
+		struct dm_device *old_dev = dev;
+		struct dm_setup_table *table = dev->table;
+
+		while (table) {
+			struct dm_setup_table *old_table = table;
+
+			kfree(table->type);
+			kfree(table->params);
+			table = table->next;
+			kfree(old_table);
+			dev->table_count--;
+		}
+		WARN_ON(dev->table_count);
+		dev = dev->next;
+		kfree(old_dev);
+	}
+}
+
+static char * __init dm_parse_device(struct dm_device *dev, char *str,
+				     unsigned long idx)
+{
+	struct dm_option opt;
+	size_t len;
+	unsigned long long num_tables;
+
+	/* Grab the logical name of the device to be exported to udev */
+	opt.next = str;
+	if (!get_dm_option(&opt, DM_FIELD_SEP)) {
+		DMERR_PARSE("name", "", idx);
+		goto parse_fail;
+	}
+	len = min(opt.len + 1, sizeof(dev->name));
+	strlcpy(dev->name, opt.start, len);  /* includes nul */
+
+	/* Grab the UUID value or "none" */
+	if (!get_dm_option(&opt, DM_FIELD_SEP)) {
+		DMERR_PARSE("uuid", dev->name, idx);
+		goto parse_fail;
+	}
+	len = min(opt.len + 1, sizeof(dev->uuid));
+	strlcpy(dev->uuid, opt.start, len);
+
+	/* Determine if the table/device will be read only or read-write */
+	get_dm_option(&opt, DM_ANY_SEP);
+	if (!strncmp("ro", opt.start, opt.len)) {
+		dev->ro = 1;
+	} else if (!strncmp("rw", opt.start, opt.len)) {
+		dev->ro = 0;
+	} else {
+		DMERR_PARSE("table mode", dev->name, idx);
+		goto parse_fail;
+	}
+
+	/* Optional number field */
+	if (opt.delim == DM_FIELD_SEP[0]) {
+		if (get_dm_option_u64(&opt, DM_LINE_SEP, &num_tables)) {
+			DMERR_PARSE("number of tables", dev->name, idx);
+			goto parse_fail;
+		}
+	} else {
+		num_tables = 1;
+	}
+	if (num_tables > DM_MAX_TARGETS) {
+		DMERR_PARSE("too many tables (%llu > %d)", num_tables,
+			    DM_MAX_TARGETS, dev->name, idx);
+	}
+	dev->num_tables = num_tables;
+
+	return opt.next;
+
+parse_fail:
+	return NULL;
+}
+
+static char * __init dm_parse_tables(struct dm_device *dev, char *str,
+				     unsigned long idx)
+{
+	struct dm_option opt;
+	struct dm_setup_table **table = &dev->table;
+	unsigned long num_tables = dev->num_tables;
+	unsigned long i;
+	unsigned long long value;
+
+	/*
+	 * Tables are defined as per the normal table format but with a
+	 * comma as a newline separator.
+	 */
+	opt.next = str;
+	for (i = 0; i < num_tables; i++) {
+		*table = kzalloc(sizeof(struct dm_setup_table), GFP_KERNEL);
+		if (!*table) {
+			DMERR_PARSE("table %lu (out of memory)", i, dev->name,
+				    idx);
+			goto parse_fail;
+		}
+		dev->table_count++;
+
+		if (get_dm_option_u64(&opt, DM_FIELD_SEP, &value)) {
+			DMERR_PARSE("starting sector for table %lu", i,
+				    dev->name, idx);
+			goto parse_fail;
+		}
+		(*table)->begin = value;
+
+		if (get_dm_option_u64(&opt, DM_FIELD_SEP, &value)) {
+			DMERR_PARSE("length for table %lu", i, dev->name, idx);
+			goto parse_fail;
+		}
+		(*table)->length = value;
+
+		if (get_dm_option(&opt, DM_FIELD_SEP))
+			(*table)->type = kstrndup(opt.start, opt.len,
+							GFP_KERNEL);
+		if (!((*table)->type)) {
+			DMERR_PARSE("type for table %lu", i, dev->name, idx);
+			goto parse_fail;
+		}
+		if (get_dm_option(&opt, DM_LINE_SEP))
+			(*table)->params = kstrndup(opt.start, opt.len,
+						    GFP_KERNEL);
+		if (!((*table)->params)) {
+			DMERR_PARSE("params for table %lu", i, dev->name, idx);
+			goto parse_fail;
+		}
+		table = &((*table)->next);
+	}
+	DMDEBUG("tables parsed: %d", dev->table_count);
+
+	return opt.next;
+
+parse_fail:
+	return NULL;
+}
+
+static struct dm_device *dm_parse_args(void)
+{
+	struct dm_device *devices = NULL;
+	struct dm_device **tail = &devices;
+	struct dm_device *dev;
+	char *str = dm_setup_args.str;
+	unsigned long num_devices = dm_setup_args.num_devices;
+	unsigned long i;
+
+	if (!str)
+		return NULL;
+	for (i = 0; i < num_devices; i++) {
+		dev = kzalloc(sizeof(*dev), GFP_KERNEL);
+		if (!dev) {
+			DMERR("failed to allocated memory for device %lu", i);
+			goto error;
+		}
+		*tail = dev;
+		tail = &dev->next;
+		/*
+		 * devices are given minor numbers 0 - n-1
+		 * in the order they are found in the arg
+		 * string.
+		 */
+		dev->minor = i;
+		str = dm_parse_device(dev, str, i);
+		if (!str)	/* NULL indicates error in parsing, bail */
+			goto error;
+
+		str = dm_parse_tables(dev, str, i);
+		if (!str)
+			goto error;
+	}
+	return devices;
+error:
+	dm_setup_cleanup(devices);
+	return NULL;
+}
+
+/*
+ * Parse the command-line parameters given our kernel, but do not
+ * actually try to invoke the DM device now; that is handled by
+ * dm_setup_drives after the low-level disk drivers have initialised.
+ * dm format is described at the top of the file.
+ *
+ * Because dm minor numbers are assigned in assending order starting with 0,
+ * You can assume the first device is /dev/dm-0, the next device is /dev/dm-1,
+ * and so forth.
+ */
+static int __init dm_setup(char *str)
+{
+	struct dm_option opt;
+	unsigned long long num_devices;
+
+	if (!str) {
+		DMERR("setup str is NULL");
+		goto parse_fail;
+	}
+
+	DMDEBUG("Want to parse \"%s\"", str);
+	opt.next = str;
+	if (get_dm_option_u64(&opt, DM_FIELD_SEP, &num_devices))
+		goto parse_fail;
+	str = opt.next;
+	if (num_devices > DM_MAX_DEVICES) {
+		DMERR("too many devices %llu > %d", num_devices,
+		      DM_MAX_DEVICES);
+	}
+	dm_setup_args.num_devices = num_devices;
+	dm_setup_args.str = str;
+
+	DMINFO("will configure %lu device%s", dm_setup_args.num_devices,
+	       dm_setup_args.num_devices == 1 ? "" : "s");
+	dm_early_setup = 1;
+	return 1;
+
+parse_fail:
+	DMWARN("Invalid arguments supplied to dm=.");
+	return 0;
+}
+
+static void __init dm_setup_drives(void)
+{
+	struct mapped_device *md = NULL;
+	struct dm_table *tables = NULL;
+	struct dm_setup_table *table;
+	struct dm_device *dev;
+	char *uuid;
+	fmode_t fmode = FMODE_READ;
+	struct dm_device *devices;
+
+	devices = dm_parse_args();
+
+	for (dev = devices; dev; dev = dev->next) {
+		if (dm_create(dev->minor, &md)) {
+			DMERR("failed to create device %s", dev->name);
+			goto fail;
+		}
+		DMDEBUG("created device '%s'", dm_device_name(md));
+
+		/*
+		 * In addition to flagging the table below, the disk must be
+		 * set explicitly ro/rw.
+		 */
+		set_disk_ro(dm_disk(md), dev->ro);
+
+		if (!dev->ro)
+			fmode |= FMODE_WRITE;
+		if (dm_table_create(&tables, fmode, dev->table_count, md)) {
+			DMERR("failed to create device %s tables", dev->name);
+			goto fail_put;
+		}
+		for (table = dev->table; table; table = table->next) {
+			DMINFO("device %s adding table '%llu %llu %s %s'",
+			       dev->name,
+			       (unsigned long long) table->begin,
+			       (unsigned long long) table->length,
+			       table->type, table->params);
+			if (dm_table_add_target(tables, table->type,
+						table->begin,
+						table->length,
+						table->params)) {
+				DMERR("failed to add table to device %s",
+					dev->name);
+				goto fail_put;
+			}
+		}
+		dm_lock_md_type(md);
+		if (dm_table_complete(tables)) {
+			DMERR("failed to complete device %s tables",
+				dev->name);
+			dm_unlock_md_type(md);
+			goto fail_put;
+		}
+		dm_unlock_md_type(md);
+
+		/* Suspend the device so that we can bind it to the tables. */
+		if (dm_suspend(md, 0)) {
+			DMERR("failed to suspend device %s pre-bind",
+				dev->name);
+			goto fail_put;
+		}
+
+		/*
+		 * Bind the tables to the device. This is the only way
+		 * to associate md->map with the tables and set the disk
+		 * capacity directly.
+		 */
+		if (dm_swap_table(md, tables)) {  /* should return NULL. */
+			DMERR("failed to bind device %s to tables",
+				dev->name);
+			goto fail_put;
+		}
+
+		/* Finally, resume and the device should be ready. */
+		if (dm_resume(md)) {
+			DMERR("failed to resume device %s", dev->name);
+			goto fail_put;
+		}
+
+		/* Export the dm device via the ioctl interface */
+		if (!strcmp(DM_NO_UUID, dev->uuid))
+			uuid = NULL;
+		if (dm_ioctl_export(md, dev->name, uuid)) {
+			DMERR("failed to export device %s", dev->name);
+			goto fail_put;
+		}
+		DMINFO("dm-%d (%s) is ready", dev->minor, dev->name);
+	}
+	dm_setup_cleanup(devices);
+	return;
+
+fail_put:
+	dm_put(md);
+fail:
+	DMERR("starting dm-%d (%s) failed", dev->minor, dev->name);
+	dm_setup_cleanup(devices);
+}
+
+__setup("dm=", dm_setup);
+
+void __init dm_run_setup(void)
+{
+	if (!dm_early_setup)
+		return;
+	DMINFO("attempting early device configuration.");
+	dm_setup_drives();
+}
-- 
2.6.3

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v5 0/3] init: add support to directly boot to a mapped device
  2016-02-20 18:13 [PATCH v5 0/3] init: add support to directly boot to a mapped device Kees Cook
                   ` (2 preceding siblings ...)
  2016-02-20 18:13 ` [PATCH v5 3/3] init: add support to directly boot to a mapped device Kees Cook
@ 2016-02-21 22:08 ` Alasdair G Kergon
  2016-02-22 18:55   ` Kees Cook
  3 siblings, 1 reply; 11+ messages in thread
From: Alasdair G Kergon @ 2016-02-21 22:08 UTC (permalink / raw)
  To: Kees Cook
  Cc: Alasdair Kergon, Mike Snitzer, dm-devel, Jonathan Corbet,
	Shaohua Li, Dan Ehrenberg, Rafael J. Wysocki, Chen Yu,
	Vishnu Pratap Singh, Andrew Morton, Yaowei Bai, linux-doc,
	linux-kernel, linux-raid, Will Drewry, David Zeuthen

On Sat, Feb 20, 2016 at 10:13:49AM -0800, Kees Cook wrote:
> This is a resurrection of a patch series from a few years back, first
> brought to the dm maintainers in 2010. It creates a way to define dm
> devices on the kernel command line for systems that do not use an
> initramfs, or otherwise need a dm running before init starts.
> 
> This has been used by Chrome OS for several years, and now by Brillo
> (and likely Android soon).
> 
> The last version was v4:
> https://patchwork.kernel.org/patch/104860/
> https://patchwork.kernel.org/patch/104861/
 
Inconsistencies in the terminology here can be sorted out during review,
and I see that you've taken on board some of my review comments from
2010, but what are your responses to the rest of them?

Alasdair

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v5 0/3] init: add support to directly boot to a mapped device
  2016-02-21 22:08 ` [PATCH v5 0/3] " Alasdair G Kergon
@ 2016-02-22 18:55   ` Kees Cook
  2016-02-26 16:53     ` Mike Snitzer
  0 siblings, 1 reply; 11+ messages in thread
From: Kees Cook @ 2016-02-22 18:55 UTC (permalink / raw)
  To: Kees Cook, Alasdair Kergon, Mike Snitzer, dm-devel,
	Jonathan Corbet, Shaohua Li, Dan Ehrenberg, Rafael J. Wysocki,
	Chen Yu, Vishnu Pratap Singh, Andrew Morton, Yaowei Bai,
	linux-doc, LKML, linux-raid, Will Drewry, David Zeuthen

On Sun, Feb 21, 2016 at 2:08 PM, Alasdair G Kergon <agk@redhat.com> wrote:
> On Sat, Feb 20, 2016 at 10:13:49AM -0800, Kees Cook wrote:
>> This is a resurrection of a patch series from a few years back, first
>> brought to the dm maintainers in 2010. It creates a way to define dm
>> devices on the kernel command line for systems that do not use an
>> initramfs, or otherwise need a dm running before init starts.
>>
>> This has been used by Chrome OS for several years, and now by Brillo
>> (and likely Android soon).
>>
>> The last version was v4:
>> https://patchwork.kernel.org/patch/104860/
>> https://patchwork.kernel.org/patch/104861/
>
> Inconsistencies in the terminology here can be sorted out during review,
> and I see that you've taken on board some of my review comments from
> 2010, but what are your responses to the rest of them?

Ah, sorry, the threads I could find were incomplete, so I wasn't able
to find those comments that were made to Will's 2010 submission. In
some of the cleanups I did I was very confused about "target" vs
"table", and tried to fix that. Regardless, I'm open to fixing
whatever is needed. :)

Thanks for looking at this again!

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v5 0/3] init: add support to directly boot to a mapped device
  2016-02-22 18:55   ` Kees Cook
@ 2016-02-26 16:53     ` Mike Snitzer
  2016-02-26 18:52       ` Kees Cook
  0 siblings, 1 reply; 11+ messages in thread
From: Mike Snitzer @ 2016-02-26 16:53 UTC (permalink / raw)
  To: Kees Cook
  Cc: Alasdair Kergon, dm-devel, Jonathan Corbet, Shaohua Li,
	Dan Ehrenberg, Rafael J. Wysocki, Chen Yu, Vishnu Pratap Singh,
	Andrew Morton, Yaowei Bai, linux-doc, LKML, linux-raid,
	Will Drewry, David Zeuthen

On Mon, Feb 22 2016 at  1:55pm -0500,
Kees Cook <keescook@chromium.org> wrote:

> On Sun, Feb 21, 2016 at 2:08 PM, Alasdair G Kergon <agk@redhat.com> wrote:
> > On Sat, Feb 20, 2016 at 10:13:49AM -0800, Kees Cook wrote:
> >> This is a resurrection of a patch series from a few years back, first
> >> brought to the dm maintainers in 2010. It creates a way to define dm
> >> devices on the kernel command line for systems that do not use an
> >> initramfs, or otherwise need a dm running before init starts.
> >>
> >> This has been used by Chrome OS for several years, and now by Brillo
> >> (and likely Android soon).
> >>
> >> The last version was v4:
> >> https://patchwork.kernel.org/patch/104860/
> >> https://patchwork.kernel.org/patch/104861/
> >
> > Inconsistencies in the terminology here can be sorted out during review,
> > and I see that you've taken on board some of my review comments from
> > 2010, but what are your responses to the rest of them?
> 
> Ah, sorry, the threads I could find were incomplete, so I wasn't able
> to find those comments that were made to Will's 2010 submission. In
> some of the cleanups I did I was very confused about "target" vs
> "table", and tried to fix that. Regardless, I'm open to fixing
> whatever is needed. :)
> 
> Thanks for looking at this again!

This work isn't going to fly as is.  I appreciate the effort and the
goal (without understanding _why_) but: you're open-coding, duplicating
and/or reinventing way too much in do_mounts_dm.c

1) You first need to answer: _why_ is using a proper initramfs not
viable?  A very simple initramfs that issues dmsetup commands, etc,
isn't so daunting is it?  Why is it so important for the kernel to
natively provide a dmsetup interface?  Chrome, Android, etc cannot use
initramfs?

2) If you are able to adequately justify the need for dm=:
I'd much rather the dm= kernel commandline be a simple series of
comma-delimited dmsetup-like commands.

You'd handle each command with extremely basic parsing:
 <dm_ioctl_cmd> <args> [, <dm_ioctl_cmd> <args>]
(inventing a special token to denote <newline>, to support tables with
multiple entries, rather than relying on commas and counts, etc)

and you'd then have do_mounts_dm.c open /dev/mapper/control directly and
issue proper DM ioctls rather than adding all your shim code.  This last
bit of opening /dev/mapper/control from init needs more research -- not
sure if doing such a thing from kernel is viable/safe/acceptable.

Mike

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v5 0/3] init: add support to directly boot to a mapped device
  2016-02-26 16:53     ` Mike Snitzer
@ 2016-02-26 18:52       ` Kees Cook
  2016-02-26 19:21         ` Mike Snitzer
  0 siblings, 1 reply; 11+ messages in thread
From: Kees Cook @ 2016-02-26 18:52 UTC (permalink / raw)
  To: Mike Snitzer
  Cc: Alasdair Kergon, dm-devel, Jonathan Corbet, Shaohua Li,
	Dan Ehrenberg, Rafael J. Wysocki, Chen Yu, Vishnu Pratap Singh,
	Andrew Morton, Yaowei Bai, linux-doc, LKML, linux-raid,
	Will Drewry, David Zeuthen

On Fri, Feb 26, 2016 at 8:53 AM, Mike Snitzer <snitzer@redhat.com> wrote:
> On Mon, Feb 22 2016 at  1:55pm -0500,
> Kees Cook <keescook@chromium.org> wrote:
>
>> On Sun, Feb 21, 2016 at 2:08 PM, Alasdair G Kergon <agk@redhat.com> wrote:
>> > On Sat, Feb 20, 2016 at 10:13:49AM -0800, Kees Cook wrote:
>> >> This is a resurrection of a patch series from a few years back, first
>> >> brought to the dm maintainers in 2010. It creates a way to define dm
>> >> devices on the kernel command line for systems that do not use an
>> >> initramfs, or otherwise need a dm running before init starts.
>> >>
>> >> This has been used by Chrome OS for several years, and now by Brillo
>> >> (and likely Android soon).
>> >>
>> >> The last version was v4:
>> >> https://patchwork.kernel.org/patch/104860/
>> >> https://patchwork.kernel.org/patch/104861/
>> >
>> > Inconsistencies in the terminology here can be sorted out during review,
>> > and I see that you've taken on board some of my review comments from
>> > 2010, but what are your responses to the rest of them?
>>
>> Ah, sorry, the threads I could find were incomplete, so I wasn't able
>> to find those comments that were made to Will's 2010 submission. In
>> some of the cleanups I did I was very confused about "target" vs
>> "table", and tried to fix that. Regardless, I'm open to fixing
>> whatever is needed. :)
>>
>> Thanks for looking at this again!
>
> This work isn't going to fly as is.  I appreciate the effort and the
> goal (without understanding _why_) but: you're open-coding, duplicating
> and/or reinventing way too much in do_mounts_dm.c
>
> 1) You first need to answer: _why_ is using a proper initramfs not
> viable?  A very simple initramfs that issues dmsetup commands, etc,
> isn't so daunting is it?  Why is it so important for the kernel to
> natively provide a dmsetup interface?  Chrome, Android, etc cannot use
> initramfs?

That is correct: Chrome OS does not (and won't) use an initramfs. This
is mainly for reasons of boot speed, verified boot block size, and
maybe some other things I don't remember.

> 2) If you are able to adequately justify the need for dm=:
> I'd much rather the dm= kernel commandline be a simple series of
> comma-delimited dmsetup-like commands.
>
> You'd handle each command with extremely basic parsing:
>  <dm_ioctl_cmd> <args> [, <dm_ioctl_cmd> <args>]
> (inventing a special token to denote <newline>, to support tables with
> multiple entries, rather than relying on commas and counts, etc)

Sure, changing the syntax is fine by me. We'd need to plumb access to
the ioctl interface, though.

> and you'd then have do_mounts_dm.c open /dev/mapper/control directly and
> issue proper DM ioctls rather than adding all your shim code.  This last
> bit of opening /dev/mapper/control from init needs more research -- not
> sure if doing such a thing from kernel is viable/safe/acceptable.

Well, there's no /dev and no init since our dm is the root device
(dm-verity). We need everything up and running before we mount the
root filesystem, very similar to do_mount_md.c's purpose.

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v5 0/3] init: add support to directly boot to a mapped device
  2016-02-26 18:52       ` Kees Cook
@ 2016-02-26 19:21         ` Mike Snitzer
  2016-02-26 19:59           ` Kees Cook
  0 siblings, 1 reply; 11+ messages in thread
From: Mike Snitzer @ 2016-02-26 19:21 UTC (permalink / raw)
  To: Kees Cook
  Cc: Will Drewry, Chen Yu, Jonathan Corbet, David Zeuthen, linux-doc,
	Rafael J. Wysocki, LKML, Dan Ehrenberg, linux-raid, dm-devel,
	Vishnu Pratap Singh, Yaowei Bai, Andrew Morton, Shaohua Li,
	Alasdair Kergon

On Fri, Feb 26 2016 at  1:52pm -0500,
Kees Cook <keescook@chromium.org> wrote:

> On Fri, Feb 26, 2016 at 8:53 AM, Mike Snitzer <snitzer@redhat.com> wrote:
> > On Mon, Feb 22 2016 at  1:55pm -0500,
> > Kees Cook <keescook@chromium.org> wrote:
> >
> >> On Sun, Feb 21, 2016 at 2:08 PM, Alasdair G Kergon <agk@redhat.com> wrote:
> >> > On Sat, Feb 20, 2016 at 10:13:49AM -0800, Kees Cook wrote:
> >> >> This is a resurrection of a patch series from a few years back, first
> >> >> brought to the dm maintainers in 2010. It creates a way to define dm
> >> >> devices on the kernel command line for systems that do not use an
> >> >> initramfs, or otherwise need a dm running before init starts.
> >> >>
> >> >> This has been used by Chrome OS for several years, and now by Brillo
> >> >> (and likely Android soon).
> >> >>
> >> >> The last version was v4:
> >> >> https://patchwork.kernel.org/patch/104860/
> >> >> https://patchwork.kernel.org/patch/104861/
> >> >
> >> > Inconsistencies in the terminology here can be sorted out during review,
> >> > and I see that you've taken on board some of my review comments from
> >> > 2010, but what are your responses to the rest of them?
> >>
> >> Ah, sorry, the threads I could find were incomplete, so I wasn't able
> >> to find those comments that were made to Will's 2010 submission. In
> >> some of the cleanups I did I was very confused about "target" vs
> >> "table", and tried to fix that. Regardless, I'm open to fixing
> >> whatever is needed. :)
> >>
> >> Thanks for looking at this again!
> >
> > This work isn't going to fly as is.  I appreciate the effort and the
> > goal (without understanding _why_) but: you're open-coding, duplicating
> > and/or reinventing way too much in do_mounts_dm.c
> >
> > 1) You first need to answer: _why_ is using a proper initramfs not
> > viable?  A very simple initramfs that issues dmsetup commands, etc,
> > isn't so daunting is it?  Why is it so important for the kernel to
> > natively provide a dmsetup interface?  Chrome, Android, etc cannot use
> > initramfs?
> 
> That is correct: Chrome OS does not (and won't) use an initramfs. This
> is mainly for reasons of boot speed, verified boot block size, and
> maybe some other things I don't remember.

Not sure what "verified boot block size" means but...

Sorry I really don't buy that using a custom initramfs would be the
source of slow boot.  initramfs is _not_ this hugely inefficient
mechanism you'd have us believe.

And if that is the justification for this early boot dm= support then
the Chrome OS project/team will have to continue to carry the hack
locally.  It has no place upstream.  But I'm open to revisiting this if
it can be implemented in a very cheap way.

> > 2) If you are able to adequately justify the need for dm=:
> > I'd much rather the dm= kernel commandline be a simple series of
> > comma-delimited dmsetup-like commands.
> >
> > You'd handle each command with extremely basic parsing:
> >  <dm_ioctl_cmd> <args> [, <dm_ioctl_cmd> <args>]
> > (inventing a special token to denote <newline>, to support tables with
> > multiple entries, rather than relying on commas and counts, etc)
> 
> Sure, changing the syntax is fine by me. We'd need to plumb access to
> the ioctl interface, though.

I was hoping to avoid any extra hacks but yes... seems you'd need a new
API to issue the equivalent of a DM ioctl programatically.  Hopefully
it'd be quite a small wrapper.

> > and you'd then have do_mounts_dm.c open /dev/mapper/control directly and
> > issue proper DM ioctls rather than adding all your shim code.  This last
> > bit of opening /dev/mapper/control from init needs more research -- not
> > sure if doing such a thing from kernel is viable/safe/acceptable.
> 
> Well, there's no /dev and no init since our dm is the root device
> (dm-verity). We need everything up and running before we mount the
> root filesystem, very similar to do_mount_md.c's purpose.

Ah yes, microoptimization associated with no udev or normal Linux boot
comes full circle and limits the use of existing standard interfaces.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v5 0/3] init: add support to directly boot to a mapped device
  2016-02-26 19:21         ` Mike Snitzer
@ 2016-02-26 19:59           ` Kees Cook
  2016-02-26 20:47             ` Mike Snitzer
  0 siblings, 1 reply; 11+ messages in thread
From: Kees Cook @ 2016-02-26 19:59 UTC (permalink / raw)
  To: Mike Snitzer
  Cc: Will Drewry, Chen Yu, Jonathan Corbet, David Zeuthen, linux-doc,
	Rafael J. Wysocki, LKML, Dan Ehrenberg, linux-raid, dm-devel,
	Vishnu Pratap Singh, Yaowei Bai, Andrew Morton, Shaohua Li,
	Alasdair Kergon

On Fri, Feb 26, 2016 at 11:21 AM, Mike Snitzer <snitzer@redhat.com> wrote:
> On Fri, Feb 26 2016 at  1:52pm -0500,
> Kees Cook <keescook@chromium.org> wrote:
>
>> On Fri, Feb 26, 2016 at 8:53 AM, Mike Snitzer <snitzer@redhat.com> wrote:
>> > On Mon, Feb 22 2016 at  1:55pm -0500,
>> > Kees Cook <keescook@chromium.org> wrote:
>> >
>> >> On Sun, Feb 21, 2016 at 2:08 PM, Alasdair G Kergon <agk@redhat.com> wrote:
>> >> > On Sat, Feb 20, 2016 at 10:13:49AM -0800, Kees Cook wrote:
>> >> >> This is a resurrection of a patch series from a few years back, first
>> >> >> brought to the dm maintainers in 2010. It creates a way to define dm
>> >> >> devices on the kernel command line for systems that do not use an
>> >> >> initramfs, or otherwise need a dm running before init starts.
>> >> >>
>> >> >> This has been used by Chrome OS for several years, and now by Brillo
>> >> >> (and likely Android soon).
>> >> >>
>> >> >> The last version was v4:
>> >> >> https://patchwork.kernel.org/patch/104860/
>> >> >> https://patchwork.kernel.org/patch/104861/
>> >> >
>> >> > Inconsistencies in the terminology here can be sorted out during review,
>> >> > and I see that you've taken on board some of my review comments from
>> >> > 2010, but what are your responses to the rest of them?
>> >>
>> >> Ah, sorry, the threads I could find were incomplete, so I wasn't able
>> >> to find those comments that were made to Will's 2010 submission. In
>> >> some of the cleanups I did I was very confused about "target" vs
>> >> "table", and tried to fix that. Regardless, I'm open to fixing
>> >> whatever is needed. :)
>> >>
>> >> Thanks for looking at this again!
>> >
>> > This work isn't going to fly as is.  I appreciate the effort and the
>> > goal (without understanding _why_) but: you're open-coding, duplicating
>> > and/or reinventing way too much in do_mounts_dm.c
>> >
>> > 1) You first need to answer: _why_ is using a proper initramfs not
>> > viable?  A very simple initramfs that issues dmsetup commands, etc,
>> > isn't so daunting is it?  Why is it so important for the kernel to
>> > natively provide a dmsetup interface?  Chrome, Android, etc cannot use
>> > initramfs?
>>
>> That is correct: Chrome OS does not (and won't) use an initramfs. This
>> is mainly for reasons of boot speed, verified boot block size, and
>> maybe some other things I don't remember.
>
> Not sure what "verified boot block size" means but...

Chrome OS uses coreboot as its boot firmware and a coreboot module
known as "depthcharge" for doing the crypto-verification and booting
of the Chrome OS system. This is the static root of trust Chrome OS
extends from its read-only boot firmware through to the kernel it
loads and the dm-verity partition it mounts as the read-only root
filesystem. To keep the boot speed fast and the kernel size small,
there is no initramfs.

> Sorry I really don't buy that using a custom initramfs would be the
> source of slow boot.  initramfs is _not_ this hugely inefficient
> mechanism you'd have us believe.

I didn't say it was hugely inefficient, but for Chrome OS it's not
needed, and was a measurable source of boot time. We just got rid of
it since it was redundant. I can't change that design decision; I'm
just here to help bring the dm= boot support upstream. :)

> And if that is the justification for this early boot dm= support then
> the Chrome OS project/team will have to continue to carry the hack
> locally.  It has no place upstream.  But I'm open to revisiting this if
> it can be implemented in a very cheap way.

Yeah, I'm open to whatever suggestions you have.

>> > 2) If you are able to adequately justify the need for dm=:
>> > I'd much rather the dm= kernel commandline be a simple series of
>> > comma-delimited dmsetup-like commands.
>> >
>> > You'd handle each command with extremely basic parsing:
>> >  <dm_ioctl_cmd> <args> [, <dm_ioctl_cmd> <args>]
>> > (inventing a special token to denote <newline>, to support tables with
>> > multiple entries, rather than relying on commas and counts, etc)
>>
>> Sure, changing the syntax is fine by me. We'd need to plumb access to
>> the ioctl interface, though.
>
> I was hoping to avoid any extra hacks but yes... seems you'd need a new
> API to issue the equivalent of a DM ioctl programatically.  Hopefully
> it'd be quite a small wrapper.

Seems like it shouldn't be too bad.

>> > and you'd then have do_mounts_dm.c open /dev/mapper/control directly and
>> > issue proper DM ioctls rather than adding all your shim code.  This last
>> > bit of opening /dev/mapper/control from init needs more research -- not
>> > sure if doing such a thing from kernel is viable/safe/acceptable.
>>
>> Well, there's no /dev and no init since our dm is the root device
>> (dm-verity). We need everything up and running before we mount the
>> root filesystem, very similar to do_mount_md.c's purpose.
>
> Ah yes, microoptimization associated with no udev or normal Linux boot
> comes full circle and limits the use of existing standard interfaces.

Chrome OS is hardly the only Linux device without an initramfs, but
yeah, it does cause us headaches from time to time.

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v5 0/3] init: add support to directly boot to a mapped device
  2016-02-26 19:59           ` Kees Cook
@ 2016-02-26 20:47             ` Mike Snitzer
  0 siblings, 0 replies; 11+ messages in thread
From: Mike Snitzer @ 2016-02-26 20:47 UTC (permalink / raw)
  To: Kees Cook
  Cc: Will Drewry, Chen Yu, Jonathan Corbet, David Zeuthen, linux-doc,
	Rafael J. Wysocki, LKML, Dan Ehrenberg, linux-raid, dm-devel,
	Vishnu Pratap Singh, Yaowei Bai, Andrew Morton, Shaohua Li,
	Alasdair Kergon

On Fri, Feb 26 2016 at  2:59pm -0500,
Kees Cook <keescook@chromium.org> wrote:

> On Fri, Feb 26, 2016 at 11:21 AM, Mike Snitzer <snitzer@redhat.com> wrote:
> > On Fri, Feb 26 2016 at  1:52pm -0500,
> > Kees Cook <keescook@chromium.org> wrote:
> >
> >> On Fri, Feb 26, 2016 at 8:53 AM, Mike Snitzer <snitzer@redhat.com> wrote:
> >> > On Mon, Feb 22 2016 at  1:55pm -0500,
> >> > Kees Cook <keescook@chromium.org> wrote:
> >> >
> >> >> On Sun, Feb 21, 2016 at 2:08 PM, Alasdair G Kergon <agk@redhat.com> wrote:
> >> >> > On Sat, Feb 20, 2016 at 10:13:49AM -0800, Kees Cook wrote:
> >> >> >> This is a resurrection of a patch series from a few years back, first
> >> >> >> brought to the dm maintainers in 2010. It creates a way to define dm
> >> >> >> devices on the kernel command line for systems that do not use an
> >> >> >> initramfs, or otherwise need a dm running before init starts.
> >> >> >>
> >> >> >> This has been used by Chrome OS for several years, and now by Brillo
> >> >> >> (and likely Android soon).
> >> >> >>
> >> >> >> The last version was v4:
> >> >> >> https://patchwork.kernel.org/patch/104860/
> >> >> >> https://patchwork.kernel.org/patch/104861/
> >> >> >
> >> >> > Inconsistencies in the terminology here can be sorted out during review,
> >> >> > and I see that you've taken on board some of my review comments from
> >> >> > 2010, but what are your responses to the rest of them?
> >> >>
> >> >> Ah, sorry, the threads I could find were incomplete, so I wasn't able
> >> >> to find those comments that were made to Will's 2010 submission. In
> >> >> some of the cleanups I did I was very confused about "target" vs
> >> >> "table", and tried to fix that. Regardless, I'm open to fixing
> >> >> whatever is needed. :)
> >> >>
> >> >> Thanks for looking at this again!
> >> >
> >> > This work isn't going to fly as is.  I appreciate the effort and the
> >> > goal (without understanding _why_) but: you're open-coding, duplicating
> >> > and/or reinventing way too much in do_mounts_dm.c
> >> >
> >> > 1) You first need to answer: _why_ is using a proper initramfs not
> >> > viable?  A very simple initramfs that issues dmsetup commands, etc,
> >> > isn't so daunting is it?  Why is it so important for the kernel to
> >> > natively provide a dmsetup interface?  Chrome, Android, etc cannot use
> >> > initramfs?
> >>
> >> That is correct: Chrome OS does not (and won't) use an initramfs. This
> >> is mainly for reasons of boot speed, verified boot block size, and
> >> maybe some other things I don't remember.
> >
> > Not sure what "verified boot block size" means but...
> 
> Chrome OS uses coreboot as its boot firmware and a coreboot module
> known as "depthcharge" for doing the crypto-verification and booting
> of the Chrome OS system. This is the static root of trust Chrome OS
> extends from its read-only boot firmware through to the kernel it
> loads and the dm-verity partition it mounts as the read-only root
> filesystem. To keep the boot speed fast and the kernel size small,
> there is no initramfs.
> 
> > Sorry I really don't buy that using a custom initramfs would be the
> > source of slow boot.  initramfs is _not_ this hugely inefficient
> > mechanism you'd have us believe.
> 
> I didn't say it was hugely inefficient, but for Chrome OS it's not
> needed, and was a measurable source of boot time. We just got rid of
> it since it was redundant. I can't change that design decision; I'm
> just here to help bring the dm= boot support upstream. :)
> 
> > And if that is the justification for this early boot dm= support then
> > the Chrome OS project/team will have to continue to carry the hack
> > locally.  It has no place upstream.  But I'm open to revisiting this if
> > it can be implemented in a very cheap way.
> 
> Yeah, I'm open to whatever suggestions you have.
> 
> >> > 2) If you are able to adequately justify the need for dm=:
> >> > I'd much rather the dm= kernel commandline be a simple series of
> >> > comma-delimited dmsetup-like commands.
> >> >
> >> > You'd handle each command with extremely basic parsing:
> >> >  <dm_ioctl_cmd> <args> [, <dm_ioctl_cmd> <args>]
> >> > (inventing a special token to denote <newline>, to support tables with
> >> > multiple entries, rather than relying on commas and counts, etc)
> >>
> >> Sure, changing the syntax is fine by me. We'd need to plumb access to
> >> the ioctl interface, though.
> >
> > I was hoping to avoid any extra hacks but yes... seems you'd need a new
> > API to issue the equivalent of a DM ioctl programatically.  Hopefully
> > it'd be quite a small wrapper.
> 
> Seems like it shouldn't be too bad.

OK, I'm waiting on you to give it a shot.  I'll do my best to help.

Thanks,
Mike

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2016-02-26 20:48 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-20 18:13 [PATCH v5 0/3] init: add support to directly boot to a mapped device Kees Cook
2016-02-20 18:13 ` [PATCH v5 1/3] dm: export a table+mapped device to the ioctl interface Kees Cook
2016-02-20 18:13 ` [PATCH v5 2/3] dm: make mapped_device locking functions available Kees Cook
2016-02-20 18:13 ` [PATCH v5 3/3] init: add support to directly boot to a mapped device Kees Cook
2016-02-21 22:08 ` [PATCH v5 0/3] " Alasdair G Kergon
2016-02-22 18:55   ` Kees Cook
2016-02-26 16:53     ` Mike Snitzer
2016-02-26 18:52       ` Kees Cook
2016-02-26 19:21         ` Mike Snitzer
2016-02-26 19:59           ` Kees Cook
2016-02-26 20:47             ` Mike Snitzer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).