[PATCH 0/5] dm: boot a mapped device without an initramfs

[PATCH 0/5] dm: boot a mapped device without an initramfs

[Enric Balletbo i Serra]

Hello,

Some of these patches were send few years back, I saw that first
version was send to this list in 2010, and after some reviews did not
landi [1] , apparently without any reason.

The patches has been used and working well on the ChromeOS kernel for
long time, this version is basically a rebase on top of mainline based
on latest version found in chromeos 4.4 kernel, the patches has also
been tested on a veyron device.

This a new attempt to try to land these patches again, so I'll wait
for comments/reviews and send new version to the list.

[1] Patchwork links:
    https://patchwork.kernel.org/patch/104857/
    https://patchwork.kernel.org/patch/104856/
    https://patchwork.kernel.org/patch/104858/

Best regards,
 Enric

Brian Norris (1):
  dm: move dm_table_destroy() to same header as dm_table_create()

Enric Balletbo i Serra (1):
  dm: move dm definitions outside the private header to boot dm targets.

Will Drewry (3):
  dm: export a table+mapped device to the ioctl interface
  init: add support to directly boot to a mapped device
  dm verity: add support for version 0 of the on-disk format

 Documentation/admin-guide/kernel-parameters.rst |   1 +
 Documentation/admin-guide/kernel-parameters.txt |   3 +
 Documentation/device-mapper/boot.txt            |  42 +++
 drivers/md/dm-ioctl.c                           |  39 ++
 drivers/md/dm-verity-target.c                   | 279 ++++++++++----
 drivers/md/dm.h                                 |   8 -
 include/linux/device-mapper.h                   |  19 +
 init/Makefile                                   |   1 +
 init/do_mounts.c                                |   1 +
 init/do_mounts.h                                |  10 +
 init/do_mounts_dm.c                             | 472 ++++++++++++++++++++++++
 11 files changed, 803 insertions(+), 72 deletions(-)
 create mode 100644 Documentation/device-mapper/boot.txt
 create mode 100644 init/do_mounts_dm.c

-- 
2.9.3

[PATCH 1/5] dm: move dm_table_destroy() to same header as dm_table_create()

[Enric Balletbo i Serra]

From: Brian Norris <briannorris@chromium.org>

If anyone is going to use dm_table_create(), they probably should be
able to use dm_table_destroy() too. Move the dm_table_destroy()
definition outside the private header, near dm_table_create()

Signed-off-by: Brian Norris <briannorris@chromium.org>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
 drivers/md/dm.h               | 1 -
 include/linux/device-mapper.h | 5 +++++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/md/dm.h b/drivers/md/dm.h
index f298b01..c54d2f1 100644
--- a/drivers/md/dm.h
+++ b/drivers/md/dm.h
@@ -48,7 +48,6 @@ struct dm_md_mempools;
 /*-----------------------------------------------------------------
  * Internal table functions.
  *---------------------------------------------------------------*/
-void dm_table_destroy(struct dm_table *t);
 void dm_table_event_callback(struct dm_table *t,
 			     void (*fn)(void *), void *context);
 struct dm_target *dm_table_get_target(struct dm_table *t, unsigned int index);
diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h
index a7e6903..70cb6af 100644
--- a/include/linux/device-mapper.h
+++ b/include/linux/device-mapper.h
@@ -472,6 +472,11 @@ void dm_table_set_type(struct dm_table *t, unsigned type);
 int dm_table_complete(struct dm_table *t);
 
 /*
+ * Destroy the table when finished.
+ */
+void dm_table_destroy(struct dm_table *t);
+
+/*
  * Target may require that it is never sent I/O larger than len.
  */
 int __must_check dm_set_target_max_io_len(struct dm_target *ti, sector_t len);
-- 
2.9.3

[PATCH 2/5] dm: move dm definitions outside the private header to boot dm targets.

[Enric Balletbo i Serra]

To boot to device-mapper targets without an initr* we should be able to
use some dm functions, move these to the device-mapper include file so
we can call these functions from init/*

The functions moved are:

  dm_table_get_type()
  dm_lock_md_type()
  dm_unlock_md_type()
  dm_set_md_type()
  dm_get_md_type()
  dm_setup_md_queue()

Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
 drivers/md/dm.h               | 7 -------
 include/linux/device-mapper.h | 8 ++++++++
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/md/dm.h b/drivers/md/dm.h
index c54d2f1..6b501b5 100644
--- a/drivers/md/dm.h
+++ b/drivers/md/dm.h
@@ -63,7 +63,6 @@ void dm_table_presuspend_undo_targets(struct dm_table *t);
 void dm_table_postsuspend_targets(struct dm_table *t);
 int dm_table_resume_targets(struct dm_table *t);
 int dm_table_any_congested(struct dm_table *t, int bdi_bits);
-unsigned dm_table_get_type(struct dm_table *t);
 struct target_type *dm_table_get_immutable_target_type(struct dm_table *t);
 struct dm_target *dm_table_get_immutable_target(struct dm_table *t);
 struct dm_target *dm_table_get_wildcard_target(struct dm_table *t);
@@ -73,14 +72,8 @@ bool dm_table_all_blk_mq_devices(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);
 
-int dm_setup_md_queue(struct mapped_device *md, struct dm_table *t);
-
 /*
  * To check the return value from dm_table_find_target().
  */
diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h
index 70cb6af..013ac2e 100644
--- a/include/linux/device-mapper.h
+++ b/include/linux/device-mapper.h
@@ -513,6 +513,14 @@ void dm_table_run_md_queue_async(struct dm_table *t);
 struct dm_table *dm_swap_table(struct mapped_device *md,
 			       struct dm_table *t);
 
+unsigned dm_table_get_type(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);
+int dm_setup_md_queue(struct mapped_device *md, struct dm_table *t);
+
 /*
  * A wrapper around vmalloc.
  */
-- 
2.9.3

[PATCH 3/5] dm: export a table+mapped device to the ioctl interface

[Enric Balletbo i Serra]

From: Will Drewry <wad@chromium.org>

One function is added which allows for a programmatically created
mapped device to be inserted into the dm-ioctl hash table. This binds
the device to a name and, optional, uuid which is needed by udev and
allows for userspace management of the mapped device.

Signed-off-by: Will Drewry <wad@chromium.org>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
 drivers/md/dm-ioctl.c         | 39 +++++++++++++++++++++++++++++++++++++++
 include/linux/device-mapper.h |  6 ++++++
 2 files changed, 45 insertions(+)

diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
index 4da6fc6..cdd7a14 100644
--- a/drivers/md/dm-ioctl.c
+++ b/drivers/md/dm-ioctl.c
@@ -1929,6 +1929,45 @@ void dm_interface_exit(void)
 }
 
 /**
+ * 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) {
+		r = -ENXIO;
+		goto out;
+	}
+
+	/* 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));
+		r = -ENXIO;
+		goto out;
+	}
+
+	r = dm_hash_insert(name, uuid, md);
+	if (r) {
+		DMERR("%s: could not bind to '%s'", dm_device_name(md), name);
+		goto out;
+	}
+
+	/* Let udev know we've changed. */
+	dm_kobject_uevent(md, KOBJ_CHANGE, dm_get_event_nr(md));
+out:
+	return r;
+}
+
+/**
  * dm_copy_name_and_uuid - Copy mapped device name & uuid into supplied buffers
  * @md: Pointer to mapped_device
  * @name: Buffer (size DM_NAME_LEN) for name
diff --git a/include/linux/device-mapper.h b/include/linux/device-mapper.h
index 013ac2e..b60892e 100644
--- a/include/linux/device-mapper.h
+++ b/include/linux/device-mapper.h
@@ -405,6 +405,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.9.3

[PATCH 4/5] init: add support to directly boot to a mapped device

[Enric Balletbo i Serra]

From: Will Drewry <wad@chromium.org>

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

Signed-off-by: Will Drewry <wad@chromium.org>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
 Documentation/admin-guide/kernel-parameters.rst |   1 +
 Documentation/admin-guide/kernel-parameters.txt |   3 +
 Documentation/device-mapper/boot.txt            |  42 +++
 init/Makefile                                   |   1 +
 init/do_mounts.c                                |   1 +
 init/do_mounts.h                                |  10 +
 init/do_mounts_dm.c                             | 478 ++++++++++++++++++++++++
 7 files changed, 536 insertions(+)
 create mode 100644 Documentation/device-mapper/boot.txt
 create mode 100644 init/do_mounts_dm.c

diff --git a/Documentation/admin-guide/kernel-parameters.rst b/Documentation/admin-guide/kernel-parameters.rst
index b516164..ecb58d6 100644
--- a/Documentation/admin-guide/kernel-parameters.rst
+++ b/Documentation/admin-guide/kernel-parameters.rst
@@ -91,6 +91,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
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 2ba45ca..686453d 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -830,6 +830,9 @@
 
 	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/Documentation/device-mapper/boot.txt b/Documentation/device-mapper/boot.txt
new file mode 100644
index 0000000..76ea6ac
--- /dev/null
+++ b/Documentation/device-mapper/boot.txt
@@ -0,0 +1,42 @@
+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.
+
+For simple device mapper configurations, it is possible to boot directly
+using the following kernel command line:
+
+dm="<name> <uuid> <ro>,table line 1,...,table line n"
+
+name = the name to associate with the device
+	after boot, udev, if used, will use that name to label
+	the device node.
+uuid = may be 'none' or the UUID desired for the device.
+ro = may be 'ro' or 'rw'.  If 'ro', the device and device table will be
+	marked read-only.
+
+Each table 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.
+
+Example
+=======
+
+- Booting to a linear array made up of user-mode linux block devices:
+
+  dm="lroot none 0, 0 4096 linear 98:16 0, 4096 4096 linear 98:32 0" \
+  root=/dev/dm-0
+
+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.
diff --git a/init/Makefile b/init/Makefile
index c4fb455..30424d7 100644
--- a/init/Makefile
+++ b/init/Makefile
@@ -20,6 +20,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 c2de510..8b9182b 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 067af1d..ecb2757 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 0000000..54c3299
--- /dev/null
+++ b/init/do_mounts_dm.c
@@ -0,0 +1,478 @@
+/* do_mounts_dm.c
+ * Copyright (C) 2010 The Chromium OS Authors <chromium-os-dev@chromium.org>
+ *                    All Rights Reserved.
+ * Based on do_mounts_md.c
+ *
+ * This file is released under the GPL.
+ */
+#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"
+
+/* 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
+
+/*
+ * 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 specifying the number of
+ * devices. A device can have multiple targets if the the number of
+ * targets is specified.
+ *
+ * In the future, the <num> field will be mandatory.
+ *
+ * <device>        ::= [<num>] <device-mapper>+
+ * <device-mapper> ::= <head> "," <target>+
+ * <head>          ::= <name> <uuid> <mode> [<num>]
+ * <target>        ::= <start> <length> <type> <options> ","
+ * <mode>          ::= "ro" | "rw"
+ * <uuid>          ::= xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx | "none"
+ * <type>          ::= "verity" | "bootcache" | ...
+ *
+ * Example:
+ * 2 vboot none ro 1,
+ *     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 1,
+ *     0 1740800 verity payload=254:0 hashtree=254:0 hashstart=1740800 alg=sha1
+ *       root_hexdigest=76e9be054b15884a9fa85973e9cb274c93afadb6
+ *       salt=5b3549d54d6c7a3837b9b81ed72e49463a64c03680c47835bef94d768e5646fe
+ *
+ * Notes:
+ *  1. uuid is a label for the device and we set it to "none".
+ *  2. The <num> field will be optional initially and assumed to be 1.
+ *     Once all the scripts that set these fields have been set, it will
+ *     be made mandatory.
+ */
+
+struct dm_setup_target {
+	sector_t begin;
+	sector_t length;
+	char *type;
+	char *params;
+	/* simple singly linked list */
+	struct dm_setup_target *next;
+};
+
+struct dm_device {
+	int minor;
+	int ro;
+	char name[DM_MAX_NAME];
+	char uuid[DM_MAX_UUID];
+	unsigned long num_targets;
+	struct dm_setup_target *target;
+	int target_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 __initdata dm_early_setup;
+
+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 dm_setup_cleanup(struct dm_device *devices)
+{
+	struct dm_device *dev = devices;
+
+	while (dev) {
+		struct dm_device *old_dev = dev;
+		struct dm_setup_target *target = dev->target;
+
+		while (target) {
+			struct dm_setup_target *old_target = target;
+
+			kfree(target->type);
+			kfree(target->params);
+			target = target->next;
+			kfree(old_target);
+			dev->target_count--;
+		}
+		BUG_ON(dev->target_count);
+		dev = dev->next;
+		kfree(old_dev);
+	}
+	return 0;
+}
+
+static char * __init dm_parse_device(struct dm_device *dev, char *str)
+{
+	struct dm_option opt;
+	size_t len;
+
+	/* Grab the logical name of the device to be exported to udev */
+	opt.next = str;
+	if (!get_dm_option(&opt, DM_FIELD_SEP)) {
+		DMERR("failed to parse device name");
+		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("failed to parse device uuid");
+		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("failed to parse table mode");
+		goto parse_fail;
+	}
+
+	/* Optional number field */
+	if (opt.delim == DM_FIELD_SEP[0]) {
+		if (!get_dm_option(&opt, DM_LINE_SEP))
+			return NULL;
+		if (kstrtoul(opt.start, 10, &dev->num_targets))
+			dev->num_targets = 1;
+	} else {
+		dev->num_targets = 1;
+	}
+	if (dev->num_targets > DM_MAX_TARGETS) {
+		DMERR("too many targets %lu > %d",
+		      dev->num_targets, DM_MAX_TARGETS);
+	}
+	return opt.next;
+
+parse_fail:
+	return NULL;
+}
+
+static char * __init dm_parse_targets(struct dm_device *dev, char *str)
+{
+	struct dm_option opt;
+	struct dm_setup_target **target = &dev->target;
+	unsigned long num_targets = dev->num_targets;
+	unsigned long i;
+
+	/* Targets are defined as per the table format but with a
+	 * comma as a newline separator.
+	 */
+	opt.next = str;
+	for (i = 0; i < num_targets; i++) {
+		*target = kzalloc(sizeof(*target), GFP_KERNEL);
+		if (!*target) {
+			DMERR("failed to allocate memory for target %s<%ld>",
+			      dev->name, i);
+			goto parse_fail;
+		}
+		dev->target_count++;
+
+		if (!get_dm_option(&opt, DM_FIELD_SEP)) {
+			DMERR("failed to parse starting sector"
+			      " for target %s<%ld>", dev->name, i);
+			goto parse_fail;
+		}
+
+		if (kstrtoull(opt.start, 10, &(*target)->begin))
+			goto parse_fail;
+
+		if (!get_dm_option(&opt, DM_FIELD_SEP)) {
+			DMERR("failed to parse length for target %s<%ld>",
+			      dev->name, i);
+			goto parse_fail;
+		}
+
+		if (kstrtoull(opt.start, 10, &(*target)->length))
+			goto parse_fail;
+
+		if (get_dm_option(&opt, DM_FIELD_SEP))
+			(*target)->type = kstrndup(opt.start, opt.len,
+						   GFP_KERNEL);
+		if (!((*target)->type)) {
+			DMERR("failed to parse type for target %s<%ld>",
+			      dev->name, i);
+			goto parse_fail;
+		}
+		if (get_dm_option(&opt, DM_LINE_SEP))
+			(*target)->params = kstrndup(opt.start, opt.len,
+						     GFP_KERNEL);
+		if (!((*target)->params)) {
+			DMERR("failed to parse params for target %s<%ld>",
+			      dev->name, i);
+			goto parse_fail;
+		}
+		target = &((*target)->next);
+	}
+	DMDEBUG("parsed %d targets", dev->target_count);
+
+	return opt.next;
+
+parse_fail:
+	return NULL;
+}
+
+static struct dm_device * __init 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 dev");
+			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);
+		if (!str)	/* NULL indicates error in parsing, bail */
+			goto error;
+
+		str = dm_parse_targets(dev, str);
+		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 num_devices;
+
+	if (!str) {
+		DMDEBUG("str is NULL");
+		goto parse_fail;
+	}
+	opt.next = str;
+	if (!get_dm_option(&opt, DM_FIELD_SEP))
+		goto parse_fail;
+	if (isdigit(opt.start[0])) {	/* Optional number field */
+		if (kstrtoul(opt.start, 10, &num_devices))
+			num_devices = 1;
+		str = opt.next;
+	} else {
+		num_devices = 1;
+		/* Don't advance str */
+	}
+	if (num_devices > DM_MAX_DEVICES) {
+		DMDEBUG("too many devices %lu > %d",
+			num_devices, DM_MAX_DEVICES);
+	}
+	dm_setup_args.str = str;
+	dm_setup_args.num_devices = num_devices;
+	DMINFO("will configure %lu devices", num_devices);
+	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 *table = NULL;
+	struct dm_setup_target *target;
+	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)) {
+			DMDEBUG("failed to create the device");
+			goto dm_create_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(&table, fmode, dev->target_count, md)) {
+			DMDEBUG("failed to create the table");
+			goto dm_table_create_fail;
+		}
+
+		dm_lock_md_type(md);
+
+		for (target = dev->target; target; target = target->next) {
+			DMINFO("adding target '%llu %llu %s %s'",
+			       (unsigned long long) target->begin,
+			       (unsigned long long) target->length,
+			       target->type, target->params);
+			if (dm_table_add_target(table, target->type,
+						target->begin,
+						target->length,
+						target->params)) {
+				DMDEBUG("failed to add the target"
+					" to the table");
+				goto add_target_fail;
+			}
+		}
+		if (dm_table_complete(table)) {
+			DMDEBUG("failed to complete the table");
+			goto table_complete_fail;
+		}
+
+		/* Suspend the device so that we can bind it to the table. */
+		if (dm_suspend(md, 0)) {
+			DMDEBUG("failed to suspend the device pre-bind");
+			goto suspend_fail;
+		}
+
+		/* Initial table load: acquire type of table. */
+		dm_set_md_type(md, dm_table_get_type(table));
+
+		/* Setup md->queue to reflect md's type. */
+		if (dm_setup_md_queue(md, table)) {
+			DMWARN("unable to set up device queue for new table.");
+			goto setup_md_queue_fail;
+		}
+
+		/*
+		 * Bind the table to the device. This is the only way
+		 * to associate md->map with the table and set the disk
+		 * capacity directly.
+		 */
+		if (dm_swap_table(md, table)) {  /* should return NULL. */
+			DMDEBUG("failed to bind the device to the table");
+			goto table_bind_fail;
+		}
+
+		/* Finally, resume and the device should be ready. */
+		if (dm_resume(md)) {
+			DMDEBUG("failed to resume the device");
+			goto resume_fail;
+		}
+
+		/* 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)) {
+			DMDEBUG("failed to export device with given"
+				" name and uuid");
+			goto export_fail;
+		}
+
+		dm_unlock_md_type(md);
+
+		DMINFO("dm-%d is ready", dev->minor);
+	}
+	dm_setup_cleanup(devices);
+	return;
+
+export_fail:
+resume_fail:
+table_bind_fail:
+setup_md_queue_fail:
+suspend_fail:
+table_complete_fail:
+add_target_fail:
+	dm_unlock_md_type(md);
+	dm_table_destroy(table);
+dm_table_create_fail:
+	dm_put(md);
+dm_create_fail:
+	DMWARN("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.9.3

[PATCH 5/5] dm verity: add support for version 0 of the on-disk format

[Enric Balletbo i Serra]

From: Will Drewry <wad@chromium.org>

Version 0 of the on-disk format is compatible with the format used in the
Chromium OS. This patch adds support for this version.

Format type 0 is the original Chrome OS version, whereas the format type 1
is current version, but 0, the original format used in the Chromium OS is
still used in most devices. This patch adds support for the original
on-disk format so you can decide which want you want to use.

Signed-off-by: Will Drewry <wad@chromium.org>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---
 drivers/md/dm-verity-target.c | 279 ++++++++++++++++++++++++++++++++----------
 init/do_mounts_dm.c           |  16 +--
 2 files changed, 220 insertions(+), 75 deletions(-)

diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c
index 7335d8a..c098d22 100644
--- a/drivers/md/dm-verity-target.c
+++ b/drivers/md/dm-verity-target.c
@@ -17,6 +17,7 @@
 #include "dm-verity.h"
 #include "dm-verity-fec.h"
 
+#include <linux/delay.h>
 #include <linux/module.h>
 #include <linux/reboot.h>
 
@@ -28,6 +29,7 @@
 #define DM_VERITY_DEFAULT_PREFETCH_SIZE	262144
 
 #define DM_VERITY_MAX_CORRUPTED_ERRS	100
+#define DM_VERITY_NUM_POSITIONAL_ARGS	10
 
 #define DM_VERITY_OPT_LOGGING		"ignore_corruption"
 #define DM_VERITY_OPT_RESTART		"restart_on_corruption"
@@ -46,6 +48,11 @@ struct dm_verity_prefetch_work {
 	unsigned n_blocks;
 };
 
+/* Controls whether verity_get_device will wait forever for a device. */
+static int dev_wait;
+module_param(dev_wait, int, 0444);
+MODULE_PARM_DESC(dev_wait, "Wait forever for a backing device");
+
 /*
  * Auxiliary structure appended to each dm-bufio buffer. If the value
  * hash_verified is nonzero, hash of the block has been verified.
@@ -803,6 +810,183 @@ static int verity_parse_opt_args(struct dm_arg_set *as, struct dm_verity *v)
 	return r;
 }
 
+static int verity_get_device(struct dm_target *ti, const char *devname,
+			     struct dm_dev **dm_dev)
+{
+	do {
+		/* Try the normal path first since if everything is ready, it
+		 * will be the fastest.
+		 */
+		if (!dm_get_device(ti, devname, /*FMODE_READ*/
+				   dm_table_get_mode(ti->table), dm_dev))
+			return 0;
+
+		if (!dev_wait)
+			break;
+
+		/* No need to be too aggressive since this is a slow path. */
+		msleep(500);
+	} while (driver_probe_done() != 0 || !(*dm_dev));
+	return -1;
+}
+
+struct verity_args {
+	int version;
+	char *data_device;
+	char *hash_device;
+	int data_block_size_bits;
+	int hash_block_size_bits;
+	u64 num_data_blocks;
+	u64 hash_start_block;
+	char *algorithm;
+	char *digest;
+	char *salt;
+};
+
+static void pr_args(struct verity_args *args)
+{
+	printk(KERN_INFO "VERITY args: version=%d data_device=%s hash_device=%s"
+		" data_block_size_bits=%d hash_block_size_bits=%d"
+		" num_data_blocks=%lld hash_start_block=%lld"
+		" algorithm=%s digest=%s salt=%s\n",
+		args->version,
+		args->data_device,
+		args->hash_device,
+		args->data_block_size_bits,
+		args->hash_block_size_bits,
+		args->num_data_blocks,
+		args->hash_start_block,
+		args->algorithm,
+		args->digest,
+		args->salt);
+}
+
+/*
+ * positional_args - collects the argments using the positional
+ * parameters.
+ * arg# - parameter
+ *    0 - version
+ *    1 - data device
+ *    2 - hash device - may be same as data device
+ *    3 - data block size log2
+ *    4 - hash block size log2
+ *    5 - number of data blocks
+ *    6 - hash start block
+ *    7 - algorithm
+ *    8 - digest
+ *    9 - salt
+ */
+static char *positional_args(unsigned argc, char **argv,
+			     struct verity_args *args)
+{
+	unsigned int num;
+	unsigned long long num_ll;
+	char dummy;
+
+	if (argc < DM_VERITY_NUM_POSITIONAL_ARGS)
+		return "Invalid argument count: at least 10 arguments required";
+
+	if (sscanf(argv[0], "%d%c", &num, &dummy) != 1 ||
+	    num < 0 || num > 1)
+		return "Invalid version";
+	args->version = num;
+
+	args->data_device = argv[1];
+	args->hash_device = argv[2];
+
+	if (sscanf(argv[3], "%u%c", &num, &dummy) != 1 ||
+	    !num || (num & (num - 1)) ||
+	    num > PAGE_SIZE)
+		return "Invalid data device block size";
+	args->data_block_size_bits = ffs(num) - 1;
+
+	if (sscanf(argv[4], "%u%c", &num, &dummy) != 1 ||
+	    !num || (num & (num - 1)) ||
+	    num > INT_MAX)
+		return "Invalid hash device block size";
+	args->hash_block_size_bits = ffs(num) - 1;
+
+	if (sscanf(argv[5], "%llu%c", &num_ll, &dummy) != 1 ||
+	    (sector_t)(num_ll << (args->data_block_size_bits - SECTOR_SHIFT))
+	    >> (args->data_block_size_bits - SECTOR_SHIFT) != num_ll)
+		return "Invalid data blocks";
+	args->num_data_blocks = num_ll;
+
+	if (sscanf(argv[6], "%llu%c", &num_ll, &dummy) != 1 ||
+	    (sector_t)(num_ll << (args->hash_block_size_bits - SECTOR_SHIFT))
+	    >> (args->hash_block_size_bits - SECTOR_SHIFT) != num_ll)
+		return "Invalid hash start";
+	args->hash_start_block = num_ll;
+
+	args->algorithm = argv[7];
+	args->digest = argv[8];
+	args->salt = argv[9];
+
+	return NULL;
+}
+
+static void splitarg(char *arg, char **key, char **val)
+{
+	*key = strsep(&arg, "=");
+	*val = strsep(&arg, "");
+}
+
+static char *chromeos_args(unsigned int argc, char **argv,
+			   struct verity_args *args)
+{
+	char *key, *val;
+	unsigned long num;
+	int i;
+
+	args->version = 0;
+	args->data_block_size_bits = 12;
+	args->hash_block_size_bits = 12;
+	for (i = 0; i < argc; ++i) {
+		DMWARN("Argument %d: '%s'", i, argv[i]);
+		splitarg(argv[i], &key, &val);
+		if (!key) {
+			DMWARN("Bad argument %d: missing key?", i);
+			return "Bad argument: missing key";
+		}
+		if (!val) {
+			DMWARN("Bad argument %d='%s': missing value", i, key);
+			return "Bad argument: missing value";
+		}
+		if (!strcmp(key, "alg")) {
+			args->algorithm = val;
+		} else if (!strcmp(key, "payload")) {
+			args->data_device = val;
+		} else if (!strcmp(key, "hashtree")) {
+			args->hash_device = val;
+		} else if (!strcmp(key, "root_hexdigest")) {
+			args->digest = val;
+		} else if (!strcmp(key, "hashstart")) {
+			if (kstrtoul(val, 10, &num))
+				return "Invalid hashstart";
+			args->hash_start_block =
+			   num >> (args->hash_block_size_bits - SECTOR_SHIFT);
+			args->num_data_blocks = args->hash_start_block;
+		} else if (!strcmp(key, "salt")) {
+			args->salt = val;
+		}
+	}
+	if (!args->salt)
+		args->salt = "";
+
+#define NEEDARG(n) do { \
+	if (!(n)) { \
+		return "Missing argument: " #n; \
+	} } while (0)
+
+	NEEDARG(args->algorithm);
+	NEEDARG(args->data_device);
+	NEEDARG(args->hash_device);
+	NEEDARG(args->digest);
+
+#undef NEEDARG
+	return NULL;
+}
+
 /*
  * Target parameters:
  *	<version>	The current format is version 1.
@@ -819,14 +1003,21 @@ static int verity_parse_opt_args(struct dm_arg_set *as, struct dm_verity *v)
  */
 static int verity_ctr(struct dm_target *ti, unsigned argc, char **argv)
 {
+	struct verity_args args = { 0 };
 	struct dm_verity *v;
 	struct dm_arg_set as;
-	unsigned int num;
-	unsigned long long num_ll;
 	int r;
 	int i;
 	sector_t hash_position;
-	char dummy;
+
+	if (argc >= DM_VERITY_NUM_POSITIONAL_ARGS)
+		ti->error = positional_args(argc, argv, &args);
+	else
+		ti->error = chromeos_args(argc, argv, &args);
+	if (ti->error)
+		return -EINVAL;
+	if (0)
+		pr_args(&args);
 
 	v = kzalloc(sizeof(struct dm_verity), GFP_KERNEL);
 	if (!v) {
@@ -840,83 +1031,46 @@ static int verity_ctr(struct dm_target *ti, unsigned argc, char **argv)
 	if (r)
 		goto bad;
 
-	if ((dm_table_get_mode(ti->table) & ~FMODE_READ)) {
-		ti->error = "Device must be readonly";
-		r = -EINVAL;
-		goto bad;
-	}
+	v->version = args.version;
 
-	if (argc < 10) {
-		ti->error = "Not enough arguments";
-		r = -EINVAL;
-		goto bad;
-	}
-
-	if (sscanf(argv[0], "%u%c", &num, &dummy) != 1 ||
-	    num > 1) {
-		ti->error = "Invalid version";
-		r = -EINVAL;
-		goto bad;
-	}
-	v->version = num;
-
-	r = dm_get_device(ti, argv[1], FMODE_READ, &v->data_dev);
+	r = verity_get_device(ti, args.data_device, &v->data_dev);
 	if (r) {
 		ti->error = "Data device lookup failed";
 		goto bad;
 	}
 
-	r = dm_get_device(ti, argv[2], FMODE_READ, &v->hash_dev);
+	r = verity_get_device(ti, args.hash_device, &v->hash_dev);
 	if (r) {
 		ti->error = "Hash device lookup failed";
 		goto bad;
 	}
 
-	if (sscanf(argv[3], "%u%c", &num, &dummy) != 1 ||
-	    !num || (num & (num - 1)) ||
-	    num < bdev_logical_block_size(v->data_dev->bdev) ||
-	    num > PAGE_SIZE) {
+	v->data_dev_block_bits = args.data_block_size_bits;
+	if ((1 << v->data_dev_block_bits) <
+	    bdev_logical_block_size(v->data_dev->bdev)) {
 		ti->error = "Invalid data device block size";
 		r = -EINVAL;
 		goto bad;
 	}
-	v->data_dev_block_bits = __ffs(num);
 
-	if (sscanf(argv[4], "%u%c", &num, &dummy) != 1 ||
-	    !num || (num & (num - 1)) ||
-	    num < bdev_logical_block_size(v->hash_dev->bdev) ||
-	    num > INT_MAX) {
+	v->hash_dev_block_bits = args.hash_block_size_bits;
+	if ((1 << v->data_dev_block_bits) <
+	    bdev_logical_block_size(v->hash_dev->bdev)) {
 		ti->error = "Invalid hash device block size";
 		r = -EINVAL;
 		goto bad;
 	}
-	v->hash_dev_block_bits = __ffs(num);
-
-	if (sscanf(argv[5], "%llu%c", &num_ll, &dummy) != 1 ||
-	    (sector_t)(num_ll << (v->data_dev_block_bits - SECTOR_SHIFT))
-	    >> (v->data_dev_block_bits - SECTOR_SHIFT) != num_ll) {
-		ti->error = "Invalid data blocks";
-		r = -EINVAL;
-		goto bad;
-	}
-	v->data_blocks = num_ll;
 
+	v->data_blocks = args.num_data_blocks;
 	if (ti->len > (v->data_blocks << (v->data_dev_block_bits - SECTOR_SHIFT))) {
 		ti->error = "Data device is too small";
 		r = -EINVAL;
 		goto bad;
 	}
 
-	if (sscanf(argv[6], "%llu%c", &num_ll, &dummy) != 1 ||
-	    (sector_t)(num_ll << (v->hash_dev_block_bits - SECTOR_SHIFT))
-	    >> (v->hash_dev_block_bits - SECTOR_SHIFT) != num_ll) {
-		ti->error = "Invalid hash start";
-		r = -EINVAL;
-		goto bad;
-	}
-	v->hash_start = num_ll;
+	v->hash_start = args.hash_start_block;
 
-	v->alg_name = kstrdup(argv[7], GFP_KERNEL);
+	v->alg_name = kstrdup(args.algorithm, GFP_KERNEL);
 	if (!v->alg_name) {
 		ti->error = "Cannot allocate algorithm name";
 		r = -ENOMEM;
@@ -945,36 +1099,33 @@ static int verity_ctr(struct dm_target *ti, unsigned argc, char **argv)
 		r = -ENOMEM;
 		goto bad;
 	}
-	if (strlen(argv[8]) != v->digest_size * 2 ||
-	    hex2bin(v->root_digest, argv[8], v->digest_size)) {
+	if (strlen(args.digest) != v->digest_size * 2 ||
+	    hex2bin(v->root_digest, args.digest, v->digest_size)) {
 		ti->error = "Invalid root digest";
 		r = -EINVAL;
 		goto bad;
 	}
 
-	if (strcmp(argv[9], "-")) {
-		v->salt_size = strlen(argv[9]) / 2;
+	if (strcmp(args.salt, "-")) {
+		v->salt_size = strlen(args.salt) / 2;
 		v->salt = kmalloc(v->salt_size, GFP_KERNEL);
 		if (!v->salt) {
 			ti->error = "Cannot allocate salt";
 			r = -ENOMEM;
 			goto bad;
 		}
-		if (strlen(argv[9]) != v->salt_size * 2 ||
-		    hex2bin(v->salt, argv[9], v->salt_size)) {
+		if (strlen(args.salt) != v->salt_size * 2 ||
+		    hex2bin(v->salt, args.salt, v->salt_size)) {
 			ti->error = "Invalid salt";
 			r = -EINVAL;
 			goto bad;
 		}
 	}
 
-	argv += 10;
-	argc -= 10;
-
 	/* Optional parameters */
-	if (argc) {
-		as.argc = argc;
-		as.argv = argv;
+	if (argc > DM_VERITY_NUM_POSITIONAL_ARGS) {
+		as.argc = argc - DM_VERITY_NUM_POSITIONAL_ARGS;
+		as.argv = argv + DM_VERITY_NUM_POSITIONAL_ARGS;
 
 		r = verity_parse_opt_args(&as, v);
 		if (r < 0)
diff --git a/init/do_mounts_dm.c b/init/do_mounts_dm.c
index 54c3299..25a040e 100644
--- a/init/do_mounts_dm.c
+++ b/init/do_mounts_dm.c
@@ -187,8 +187,7 @@ static char * __init dm_parse_device(struct dm_device *dev, char *str)
 	if (opt.delim == DM_FIELD_SEP[0]) {
 		if (!get_dm_option(&opt, DM_LINE_SEP))
 			return NULL;
-		if (kstrtoul(opt.start, 10, &dev->num_targets))
-			dev->num_targets = 1;
+		dev->num_targets = simple_strtoul(opt.start, NULL, 10);
 	} else {
 		dev->num_targets = 1;
 	}
@@ -214,7 +213,7 @@ static char * __init dm_parse_targets(struct dm_device *dev, char *str)
 	 */
 	opt.next = str;
 	for (i = 0; i < num_targets; i++) {
-		*target = kzalloc(sizeof(*target), GFP_KERNEL);
+		*target = kzalloc(sizeof(struct dm_setup_target), GFP_KERNEL);
 		if (!*target) {
 			DMERR("failed to allocate memory for target %s<%ld>",
 			      dev->name, i);
@@ -227,18 +226,14 @@ static char * __init dm_parse_targets(struct dm_device *dev, char *str)
 			      " for target %s<%ld>", dev->name, i);
 			goto parse_fail;
 		}
-
-		if (kstrtoull(opt.start, 10, &(*target)->begin))
-			goto parse_fail;
+		(*target)->begin = simple_strtoull(opt.start, NULL, 10);
 
 		if (!get_dm_option(&opt, DM_FIELD_SEP)) {
 			DMERR("failed to parse length for target %s<%ld>",
 			      dev->name, i);
 			goto parse_fail;
 		}
-
-		if (kstrtoull(opt.start, 10, &(*target)->length))
-			goto parse_fail;
+		(*target)->length = simple_strtoull(opt.start, NULL, 10);
 
 		if (get_dm_option(&opt, DM_FIELD_SEP))
 			(*target)->type = kstrndup(opt.start, opt.len,
@@ -328,8 +323,7 @@ static int __init dm_setup(char *str)
 	if (!get_dm_option(&opt, DM_FIELD_SEP))
 		goto parse_fail;
 	if (isdigit(opt.start[0])) {	/* Optional number field */
-		if (kstrtoul(opt.start, 10, &num_devices))
-			num_devices = 1;
+		num_devices = simple_strtoul(opt.start, NULL, 10);
 		str = opt.next;
 	} else {
 		num_devices = 1;
-- 
2.9.3

Re: [PATCH 5/5] dm verity: add support for version 0 of the on-disk format

[Milan Broz]

On 04/11/2017 05:33 PM, Enric Balletbo i Serra wrote:
> From: Will Drewry <wad@chromium.org>
> 
> Version 0 of the on-disk format is compatible with the format used in the
> Chromium OS. This patch adds support for this version.
> 
> Format type 0 is the original Chrome OS version, whereas the format type 1
> is current version, but 0, the original format used in the Chromium OS is
> still used in most devices. This patch adds support for the original
> on-disk format so you can decide which want you want to use.

NACK!

What the ...  is this patch doing?

Isn't the format 0 already there? According to
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/device-mapper/verity.txt

<version>
    This is the type of the on-disk hash format.

    0 is the original format used in the Chromium OS.
      The salt is appended when hashing, digests are stored continuously and
      the rest of the block is padded with zeroes.

Reading this patch, it does something completely different than it describes
in header.

How is superblock format related to:
+MODULE_PARM_DESC(dev_wait, "Wait forever for a backing device");
...
not mentioning complete rewrite of verity table parsing... why?

Also please note that there is userspace (veritysetup) that have to work with
the old format as well (I think it does already).
I think we solved the compatibility years ago.

If not, please describe properly what is missing.
I do not see any on-disk format changes here...

Milan

> 
> Signed-off-by: Will Drewry <wad@chromium.org>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
>  drivers/md/dm-verity-target.c | 279 ++++++++++++++++++++++++++++++++----------
>  init/do_mounts_dm.c           |  16 +--
>  2 files changed, 220 insertions(+), 75 deletions(-)
> 
> diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c
> index 7335d8a..c098d22 100644
> --- a/drivers/md/dm-verity-target.c
> +++ b/drivers/md/dm-verity-target.c
> @@ -17,6 +17,7 @@
>  #include "dm-verity.h"
>  #include "dm-verity-fec.h"
>  
> +#include <linux/delay.h>
>  #include <linux/module.h>
>  #include <linux/reboot.h>
>  
> @@ -28,6 +29,7 @@
>  #define DM_VERITY_DEFAULT_PREFETCH_SIZE	262144
>  
>  #define DM_VERITY_MAX_CORRUPTED_ERRS	100
> +#define DM_VERITY_NUM_POSITIONAL_ARGS	10
>  
>  #define DM_VERITY_OPT_LOGGING		"ignore_corruption"
>  #define DM_VERITY_OPT_RESTART		"restart_on_corruption"
> @@ -46,6 +48,11 @@ struct dm_verity_prefetch_work {
>  	unsigned n_blocks;
>  };
>  
> +/* Controls whether verity_get_device will wait forever for a device. */
> +static int dev_wait;
> +module_param(dev_wait, int, 0444);
> +MODULE_PARM_DESC(dev_wait, "Wait forever for a backing device");
> +
>  /*
>   * Auxiliary structure appended to each dm-bufio buffer. If the value
>   * hash_verified is nonzero, hash of the block has been verified.
> @@ -803,6 +810,183 @@ static int verity_parse_opt_args(struct dm_arg_set *as, struct dm_verity *v)
>  	return r;
>  }
>  
> +static int verity_get_device(struct dm_target *ti, const char *devname,
> +			     struct dm_dev **dm_dev)
> +{
> +	do {
> +		/* Try the normal path first since if everything is ready, it
> +		 * will be the fastest.
> +		 */
> +		if (!dm_get_device(ti, devname, /*FMODE_READ*/
> +				   dm_table_get_mode(ti->table), dm_dev))
> +			return 0;
> +
> +		if (!dev_wait)
> +			break;
> +
> +		/* No need to be too aggressive since this is a slow path. */
> +		msleep(500);
> +	} while (driver_probe_done() != 0 || !(*dm_dev));
> +	return -1;
> +}
> +
> +struct verity_args {
> +	int version;
> +	char *data_device;
> +	char *hash_device;
> +	int data_block_size_bits;
> +	int hash_block_size_bits;
> +	u64 num_data_blocks;
> +	u64 hash_start_block;
> +	char *algorithm;
> +	char *digest;
> +	char *salt;
> +};
> +
> +static void pr_args(struct verity_args *args)
> +{
> +	printk(KERN_INFO "VERITY args: version=%d data_device=%s hash_device=%s"
> +		" data_block_size_bits=%d hash_block_size_bits=%d"
> +		" num_data_blocks=%lld hash_start_block=%lld"
> +		" algorithm=%s digest=%s salt=%s\n",
> +		args->version,
> +		args->data_device,
> +		args->hash_device,
> +		args->data_block_size_bits,
> +		args->hash_block_size_bits,
> +		args->num_data_blocks,
> +		args->hash_start_block,
> +		args->algorithm,
> +		args->digest,
> +		args->salt);
> +}
> +
> +/*
> + * positional_args - collects the argments using the positional
> + * parameters.
> + * arg# - parameter
> + *    0 - version
> + *    1 - data device
> + *    2 - hash device - may be same as data device
> + *    3 - data block size log2
> + *    4 - hash block size log2
> + *    5 - number of data blocks
> + *    6 - hash start block
> + *    7 - algorithm
> + *    8 - digest
> + *    9 - salt
> + */
> +static char *positional_args(unsigned argc, char **argv,
> +			     struct verity_args *args)
> +{
> +	unsigned int num;
> +	unsigned long long num_ll;
> +	char dummy;
> +
> +	if (argc < DM_VERITY_NUM_POSITIONAL_ARGS)
> +		return "Invalid argument count: at least 10 arguments required";
> +
> +	if (sscanf(argv[0], "%d%c", &num, &dummy) != 1 ||
> +	    num < 0 || num > 1)
> +		return "Invalid version";
> +	args->version = num;
> +
> +	args->data_device = argv[1];
> +	args->hash_device = argv[2];
> +
> +	if (sscanf(argv[3], "%u%c", &num, &dummy) != 1 ||
> +	    !num || (num & (num - 1)) ||
> +	    num > PAGE_SIZE)
> +		return "Invalid data device block size";
> +	args->data_block_size_bits = ffs(num) - 1;
> +
> +	if (sscanf(argv[4], "%u%c", &num, &dummy) != 1 ||
> +	    !num || (num & (num - 1)) ||
> +	    num > INT_MAX)
> +		return "Invalid hash device block size";
> +	args->hash_block_size_bits = ffs(num) - 1;
> +
> +	if (sscanf(argv[5], "%llu%c", &num_ll, &dummy) != 1 ||
> +	    (sector_t)(num_ll << (args->data_block_size_bits - SECTOR_SHIFT))
> +	    >> (args->data_block_size_bits - SECTOR_SHIFT) != num_ll)
> +		return "Invalid data blocks";
> +	args->num_data_blocks = num_ll;
> +
> +	if (sscanf(argv[6], "%llu%c", &num_ll, &dummy) != 1 ||
> +	    (sector_t)(num_ll << (args->hash_block_size_bits - SECTOR_SHIFT))
> +	    >> (args->hash_block_size_bits - SECTOR_SHIFT) != num_ll)
> +		return "Invalid hash start";
> +	args->hash_start_block = num_ll;
> +
> +	args->algorithm = argv[7];
> +	args->digest = argv[8];
> +	args->salt = argv[9];
> +
> +	return NULL;
> +}
> +
> +static void splitarg(char *arg, char **key, char **val)
> +{
> +	*key = strsep(&arg, "=");
> +	*val = strsep(&arg, "");
> +}
> +
> +static char *chromeos_args(unsigned int argc, char **argv,
> +			   struct verity_args *args)
> +{
> +	char *key, *val;
> +	unsigned long num;
> +	int i;
> +
> +	args->version = 0;
> +	args->data_block_size_bits = 12;
> +	args->hash_block_size_bits = 12;
> +	for (i = 0; i < argc; ++i) {
> +		DMWARN("Argument %d: '%s'", i, argv[i]);
> +		splitarg(argv[i], &key, &val);
> +		if (!key) {
> +			DMWARN("Bad argument %d: missing key?", i);
> +			return "Bad argument: missing key";
> +		}
> +		if (!val) {
> +			DMWARN("Bad argument %d='%s': missing value", i, key);
> +			return "Bad argument: missing value";
> +		}
> +		if (!strcmp(key, "alg")) {
> +			args->algorithm = val;
> +		} else if (!strcmp(key, "payload")) {
> +			args->data_device = val;
> +		} else if (!strcmp(key, "hashtree")) {
> +			args->hash_device = val;
> +		} else if (!strcmp(key, "root_hexdigest")) {
> +			args->digest = val;
> +		} else if (!strcmp(key, "hashstart")) {
> +			if (kstrtoul(val, 10, &num))
> +				return "Invalid hashstart";
> +			args->hash_start_block =
> +			   num >> (args->hash_block_size_bits - SECTOR_SHIFT);
> +			args->num_data_blocks = args->hash_start_block;
> +		} else if (!strcmp(key, "salt")) {
> +			args->salt = val;
> +		}
> +	}
> +	if (!args->salt)
> +		args->salt = "";
> +
> +#define NEEDARG(n) do { \
> +	if (!(n)) { \
> +		return "Missing argument: " #n; \
> +	} } while (0)
> +
> +	NEEDARG(args->algorithm);
> +	NEEDARG(args->data_device);
> +	NEEDARG(args->hash_device);
> +	NEEDARG(args->digest);
> +
> +#undef NEEDARG
> +	return NULL;
> +}
> +
>  /*
>   * Target parameters:
>   *	<version>	The current format is version 1.
> @@ -819,14 +1003,21 @@ static int verity_parse_opt_args(struct dm_arg_set *as, struct dm_verity *v)
>   */
>  static int verity_ctr(struct dm_target *ti, unsigned argc, char **argv)
>  {
> +	struct verity_args args = { 0 };
>  	struct dm_verity *v;
>  	struct dm_arg_set as;
> -	unsigned int num;
> -	unsigned long long num_ll;
>  	int r;
>  	int i;
>  	sector_t hash_position;
> -	char dummy;
> +
> +	if (argc >= DM_VERITY_NUM_POSITIONAL_ARGS)
> +		ti->error = positional_args(argc, argv, &args);
> +	else
> +		ti->error = chromeos_args(argc, argv, &args);
> +	if (ti->error)
> +		return -EINVAL;
> +	if (0)
> +		pr_args(&args);
>  
>  	v = kzalloc(sizeof(struct dm_verity), GFP_KERNEL);
>  	if (!v) {
> @@ -840,83 +1031,46 @@ static int verity_ctr(struct dm_target *ti, unsigned argc, char **argv)
>  	if (r)
>  		goto bad;
>  
> -	if ((dm_table_get_mode(ti->table) & ~FMODE_READ)) {
> -		ti->error = "Device must be readonly";
> -		r = -EINVAL;
> -		goto bad;
> -	}
> +	v->version = args.version;
>  
> -	if (argc < 10) {
> -		ti->error = "Not enough arguments";
> -		r = -EINVAL;
> -		goto bad;
> -	}
> -
> -	if (sscanf(argv[0], "%u%c", &num, &dummy) != 1 ||
> -	    num > 1) {
> -		ti->error = "Invalid version";
> -		r = -EINVAL;
> -		goto bad;
> -	}
> -	v->version = num;
> -
> -	r = dm_get_device(ti, argv[1], FMODE_READ, &v->data_dev);
> +	r = verity_get_device(ti, args.data_device, &v->data_dev);
>  	if (r) {
>  		ti->error = "Data device lookup failed";
>  		goto bad;
>  	}
>  
> -	r = dm_get_device(ti, argv[2], FMODE_READ, &v->hash_dev);
> +	r = verity_get_device(ti, args.hash_device, &v->hash_dev);
>  	if (r) {
>  		ti->error = "Hash device lookup failed";
>  		goto bad;
>  	}
>  
> -	if (sscanf(argv[3], "%u%c", &num, &dummy) != 1 ||
> -	    !num || (num & (num - 1)) ||
> -	    num < bdev_logical_block_size(v->data_dev->bdev) ||
> -	    num > PAGE_SIZE) {
> +	v->data_dev_block_bits = args.data_block_size_bits;
> +	if ((1 << v->data_dev_block_bits) <
> +	    bdev_logical_block_size(v->data_dev->bdev)) {
>  		ti->error = "Invalid data device block size";
>  		r = -EINVAL;
>  		goto bad;
>  	}
> -	v->data_dev_block_bits = __ffs(num);
>  
> -	if (sscanf(argv[4], "%u%c", &num, &dummy) != 1 ||
> -	    !num || (num & (num - 1)) ||
> -	    num < bdev_logical_block_size(v->hash_dev->bdev) ||
> -	    num > INT_MAX) {
> +	v->hash_dev_block_bits = args.hash_block_size_bits;
> +	if ((1 << v->data_dev_block_bits) <
> +	    bdev_logical_block_size(v->hash_dev->bdev)) {
>  		ti->error = "Invalid hash device block size";
>  		r = -EINVAL;
>  		goto bad;
>  	}
> -	v->hash_dev_block_bits = __ffs(num);
> -
> -	if (sscanf(argv[5], "%llu%c", &num_ll, &dummy) != 1 ||
> -	    (sector_t)(num_ll << (v->data_dev_block_bits - SECTOR_SHIFT))
> -	    >> (v->data_dev_block_bits - SECTOR_SHIFT) != num_ll) {
> -		ti->error = "Invalid data blocks";
> -		r = -EINVAL;
> -		goto bad;
> -	}
> -	v->data_blocks = num_ll;
>  
> +	v->data_blocks = args.num_data_blocks;
>  	if (ti->len > (v->data_blocks << (v->data_dev_block_bits - SECTOR_SHIFT))) {
>  		ti->error = "Data device is too small";
>  		r = -EINVAL;
>  		goto bad;
>  	}
>  
> -	if (sscanf(argv[6], "%llu%c", &num_ll, &dummy) != 1 ||
> -	    (sector_t)(num_ll << (v->hash_dev_block_bits - SECTOR_SHIFT))
> -	    >> (v->hash_dev_block_bits - SECTOR_SHIFT) != num_ll) {
> -		ti->error = "Invalid hash start";
> -		r = -EINVAL;
> -		goto bad;
> -	}
> -	v->hash_start = num_ll;
> +	v->hash_start = args.hash_start_block;
>  
> -	v->alg_name = kstrdup(argv[7], GFP_KERNEL);
> +	v->alg_name = kstrdup(args.algorithm, GFP_KERNEL);
>  	if (!v->alg_name) {
>  		ti->error = "Cannot allocate algorithm name";
>  		r = -ENOMEM;
> @@ -945,36 +1099,33 @@ static int verity_ctr(struct dm_target *ti, unsigned argc, char **argv)
>  		r = -ENOMEM;
>  		goto bad;
>  	}
> -	if (strlen(argv[8]) != v->digest_size * 2 ||
> -	    hex2bin(v->root_digest, argv[8], v->digest_size)) {
> +	if (strlen(args.digest) != v->digest_size * 2 ||
> +	    hex2bin(v->root_digest, args.digest, v->digest_size)) {
>  		ti->error = "Invalid root digest";
>  		r = -EINVAL;
>  		goto bad;
>  	}
>  
> -	if (strcmp(argv[9], "-")) {
> -		v->salt_size = strlen(argv[9]) / 2;
> +	if (strcmp(args.salt, "-")) {
> +		v->salt_size = strlen(args.salt) / 2;
>  		v->salt = kmalloc(v->salt_size, GFP_KERNEL);
>  		if (!v->salt) {
>  			ti->error = "Cannot allocate salt";
>  			r = -ENOMEM;
>  			goto bad;
>  		}
> -		if (strlen(argv[9]) != v->salt_size * 2 ||
> -		    hex2bin(v->salt, argv[9], v->salt_size)) {
> +		if (strlen(args.salt) != v->salt_size * 2 ||
> +		    hex2bin(v->salt, args.salt, v->salt_size)) {
>  			ti->error = "Invalid salt";
>  			r = -EINVAL;
>  			goto bad;
>  		}
>  	}
>  
> -	argv += 10;
> -	argc -= 10;
> -
>  	/* Optional parameters */
> -	if (argc) {
> -		as.argc = argc;
> -		as.argv = argv;
> +	if (argc > DM_VERITY_NUM_POSITIONAL_ARGS) {
> +		as.argc = argc - DM_VERITY_NUM_POSITIONAL_ARGS;
> +		as.argv = argv + DM_VERITY_NUM_POSITIONAL_ARGS;
>  
>  		r = verity_parse_opt_args(&as, v);
>  		if (r < 0)
> diff --git a/init/do_mounts_dm.c b/init/do_mounts_dm.c
> index 54c3299..25a040e 100644
> --- a/init/do_mounts_dm.c
> +++ b/init/do_mounts_dm.c
> @@ -187,8 +187,7 @@ static char * __init dm_parse_device(struct dm_device *dev, char *str)
>  	if (opt.delim == DM_FIELD_SEP[0]) {
>  		if (!get_dm_option(&opt, DM_LINE_SEP))
>  			return NULL;
> -		if (kstrtoul(opt.start, 10, &dev->num_targets))
> -			dev->num_targets = 1;
> +		dev->num_targets = simple_strtoul(opt.start, NULL, 10);
>  	} else {
>  		dev->num_targets = 1;
>  	}
> @@ -214,7 +213,7 @@ static char * __init dm_parse_targets(struct dm_device *dev, char *str)
>  	 */
>  	opt.next = str;
>  	for (i = 0; i < num_targets; i++) {
> -		*target = kzalloc(sizeof(*target), GFP_KERNEL);
> +		*target = kzalloc(sizeof(struct dm_setup_target), GFP_KERNEL);
>  		if (!*target) {
>  			DMERR("failed to allocate memory for target %s<%ld>",
>  			      dev->name, i);
> @@ -227,18 +226,14 @@ static char * __init dm_parse_targets(struct dm_device *dev, char *str)
>  			      " for target %s<%ld>", dev->name, i);
>  			goto parse_fail;
>  		}
> -
> -		if (kstrtoull(opt.start, 10, &(*target)->begin))
> -			goto parse_fail;
> +		(*target)->begin = simple_strtoull(opt.start, NULL, 10);
>  
>  		if (!get_dm_option(&opt, DM_FIELD_SEP)) {
>  			DMERR("failed to parse length for target %s<%ld>",
>  			      dev->name, i);
>  			goto parse_fail;
>  		}
> -
> -		if (kstrtoull(opt.start, 10, &(*target)->length))
> -			goto parse_fail;
> +		(*target)->length = simple_strtoull(opt.start, NULL, 10);
>  
>  		if (get_dm_option(&opt, DM_FIELD_SEP))
>  			(*target)->type = kstrndup(opt.start, opt.len,
> @@ -328,8 +323,7 @@ static int __init dm_setup(char *str)
>  	if (!get_dm_option(&opt, DM_FIELD_SEP))
>  		goto parse_fail;
>  	if (isdigit(opt.start[0])) {	/* Optional number field */
> -		if (kstrtoul(opt.start, 10, &num_devices))
> -			num_devices = 1;
> +		num_devices = simple_strtoul(opt.start, NULL, 10);
>  		str = opt.next;
>  	} else {
>  		num_devices = 1;
> 

Re: [PATCH 5/5] dm verity: add support for version 0 of the on-disk format

[Milan Broz]

On 04/11/2017 05:33 PM, Enric Balletbo i Serra wrote:
> From: Will Drewry <wad@chromium.org>
> 
> Version 0 of the on-disk format is compatible with the format used in the
> Chromium OS. This patch adds support for this version.
> 
> Format type 0 is the original Chrome OS version, whereas the format type 1
> is current version, but 0, the original format used in the Chromium OS is
> still used in most devices. This patch adds support for the original
> on-disk format so you can decide which want you want to use.

NACK!

What the ...  is this patch doing?

Isn't the format 0 already there? According to
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/device-mapper/verity.txt

<version>
    This is the type of the on-disk hash format.

    0 is the original format used in the Chromium OS.
      The salt is appended when hashing, digests are stored continuously and
      the rest of the block is padded with zeroes.

Reading this patch, it does something completely different than it describes
in header.

How is superblock format related to:
+MODULE_PARM_DESC(dev_wait, "Wait forever for a backing device");
...
not mentioning complete rewrite of verity table parsing... why?

Also please note that there is userspace (veritysetup) that have to work with
the old format as well (I think it does already).
I think we solved the compatibility years ago.

If not, please describe properly what is missing.
I do not see any on-disk format changes here...

Milan

> 
> Signed-off-by: Will Drewry <wad@chromium.org>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
>  drivers/md/dm-verity-target.c | 279 ++++++++++++++++++++++++++++++++----------
>  init/do_mounts_dm.c           |  16 +--
>  2 files changed, 220 insertions(+), 75 deletions(-)
> 
> diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c
> index 7335d8a..c098d22 100644
> --- a/drivers/md/dm-verity-target.c
> +++ b/drivers/md/dm-verity-target.c
> @@ -17,6 +17,7 @@
>  #include "dm-verity.h"
>  #include "dm-verity-fec.h"
>  
> +#include <linux/delay.h>
>  #include <linux/module.h>
>  #include <linux/reboot.h>
>  
> @@ -28,6 +29,7 @@
>  #define DM_VERITY_DEFAULT_PREFETCH_SIZE	262144
>  
>  #define DM_VERITY_MAX_CORRUPTED_ERRS	100
> +#define DM_VERITY_NUM_POSITIONAL_ARGS	10
>  
>  #define DM_VERITY_OPT_LOGGING		"ignore_corruption"
>  #define DM_VERITY_OPT_RESTART		"restart_on_corruption"
> @@ -46,6 +48,11 @@ struct dm_verity_prefetch_work {
>  	unsigned n_blocks;
>  };
>  
> +/* Controls whether verity_get_device will wait forever for a device. */
> +static int dev_wait;
> +module_param(dev_wait, int, 0444);
> +MODULE_PARM_DESC(dev_wait, "Wait forever for a backing device");
> +
>  /*
>   * Auxiliary structure appended to each dm-bufio buffer. If the value
>   * hash_verified is nonzero, hash of the block has been verified.
> @@ -803,6 +810,183 @@ static int verity_parse_opt_args(struct dm_arg_set *as, struct dm_verity *v)
>  	return r;
>  }
>  
> +static int verity_get_device(struct dm_target *ti, const char *devname,
> +			     struct dm_dev **dm_dev)
> +{
> +	do {
> +		/* Try the normal path first since if everything is ready, it
> +		 * will be the fastest.
> +		 */
> +		if (!dm_get_device(ti, devname, /*FMODE_READ*/
> +				   dm_table_get_mode(ti->table), dm_dev))
> +			return 0;
> +
> +		if (!dev_wait)
> +			break;
> +
> +		/* No need to be too aggressive since this is a slow path. */
> +		msleep(500);
> +	} while (driver_probe_done() != 0 || !(*dm_dev));
> +	return -1;
> +}
> +
> +struct verity_args {
> +	int version;
> +	char *data_device;
> +	char *hash_device;
> +	int data_block_size_bits;
> +	int hash_block_size_bits;
> +	u64 num_data_blocks;
> +	u64 hash_start_block;
> +	char *algorithm;
> +	char *digest;
> +	char *salt;
> +};
> +
> +static void pr_args(struct verity_args *args)
> +{
> +	printk(KERN_INFO "VERITY args: version=%d data_device=%s hash_device=%s"
> +		" data_block_size_bits=%d hash_block_size_bits=%d"
> +		" num_data_blocks=%lld hash_start_block=%lld"
> +		" algorithm=%s digest=%s salt=%s\n",
> +		args->version,
> +		args->data_device,
> +		args->hash_device,
> +		args->data_block_size_bits,
> +		args->hash_block_size_bits,
> +		args->num_data_blocks,
> +		args->hash_start_block,
> +		args->algorithm,
> +		args->digest,
> +		args->salt);
> +}
> +
> +/*
> + * positional_args - collects the argments using the positional
> + * parameters.
> + * arg# - parameter
> + *    0 - version
> + *    1 - data device
> + *    2 - hash device - may be same as data device
> + *    3 - data block size log2
> + *    4 - hash block size log2
> + *    5 - number of data blocks
> + *    6 - hash start block
> + *    7 - algorithm
> + *    8 - digest
> + *    9 - salt
> + */
> +static char *positional_args(unsigned argc, char **argv,
> +			     struct verity_args *args)
> +{
> +	unsigned int num;
> +	unsigned long long num_ll;
> +	char dummy;
> +
> +	if (argc < DM_VERITY_NUM_POSITIONAL_ARGS)
> +		return "Invalid argument count: at least 10 arguments required";
> +
> +	if (sscanf(argv[0], "%d%c", &num, &dummy) != 1 ||
> +	    num < 0 || num > 1)
> +		return "Invalid version";
> +	args->version = num;
> +
> +	args->data_device = argv[1];
> +	args->hash_device = argv[2];
> +
> +	if (sscanf(argv[3], "%u%c", &num, &dummy) != 1 ||
> +	    !num || (num & (num - 1)) ||
> +	    num > PAGE_SIZE)
> +		return "Invalid data device block size";
> +	args->data_block_size_bits = ffs(num) - 1;
> +
> +	if (sscanf(argv[4], "%u%c", &num, &dummy) != 1 ||
> +	    !num || (num & (num - 1)) ||
> +	    num > INT_MAX)
> +		return "Invalid hash device block size";
> +	args->hash_block_size_bits = ffs(num) - 1;
> +
> +	if (sscanf(argv[5], "%llu%c", &num_ll, &dummy) != 1 ||
> +	    (sector_t)(num_ll << (args->data_block_size_bits - SECTOR_SHIFT))
> +	    >> (args->data_block_size_bits - SECTOR_SHIFT) != num_ll)
> +		return "Invalid data blocks";
> +	args->num_data_blocks = num_ll;
> +
> +	if (sscanf(argv[6], "%llu%c", &num_ll, &dummy) != 1 ||
> +	    (sector_t)(num_ll << (args->hash_block_size_bits - SECTOR_SHIFT))
> +	    >> (args->hash_block_size_bits - SECTOR_SHIFT) != num_ll)
> +		return "Invalid hash start";
> +	args->hash_start_block = num_ll;
> +
> +	args->algorithm = argv[7];
> +	args->digest = argv[8];
> +	args->salt = argv[9];
> +
> +	return NULL;
> +}
> +
> +static void splitarg(char *arg, char **key, char **val)
> +{
> +	*key = strsep(&arg, "=");
> +	*val = strsep(&arg, "");
> +}
> +
> +static char *chromeos_args(unsigned int argc, char **argv,
> +			   struct verity_args *args)
> +{
> +	char *key, *val;
> +	unsigned long num;
> +	int i;
> +
> +	args->version = 0;
> +	args->data_block_size_bits = 12;
> +	args->hash_block_size_bits = 12;
> +	for (i = 0; i < argc; ++i) {
> +		DMWARN("Argument %d: '%s'", i, argv[i]);
> +		splitarg(argv[i], &key, &val);
> +		if (!key) {
> +			DMWARN("Bad argument %d: missing key?", i);
> +			return "Bad argument: missing key";
> +		}
> +		if (!val) {
> +			DMWARN("Bad argument %d='%s': missing value", i, key);
> +			return "Bad argument: missing value";
> +		}
> +		if (!strcmp(key, "alg")) {
> +			args->algorithm = val;
> +		} else if (!strcmp(key, "payload")) {
> +			args->data_device = val;
> +		} else if (!strcmp(key, "hashtree")) {
> +			args->hash_device = val;
> +		} else if (!strcmp(key, "root_hexdigest")) {
> +			args->digest = val;
> +		} else if (!strcmp(key, "hashstart")) {
> +			if (kstrtoul(val, 10, &num))
> +				return "Invalid hashstart";
> +			args->hash_start_block =
> +			   num >> (args->hash_block_size_bits - SECTOR_SHIFT);
> +			args->num_data_blocks = args->hash_start_block;
> +		} else if (!strcmp(key, "salt")) {
> +			args->salt = val;
> +		}
> +	}
> +	if (!args->salt)
> +		args->salt = "";
> +
> +#define NEEDARG(n) do { \
> +	if (!(n)) { \
> +		return "Missing argument: " #n; \
> +	} } while (0)
> +
> +	NEEDARG(args->algorithm);
> +	NEEDARG(args->data_device);
> +	NEEDARG(args->hash_device);
> +	NEEDARG(args->digest);
> +
> +#undef NEEDARG
> +	return NULL;
> +}
> +
>  /*
>   * Target parameters:
>   *	<version>	The current format is version 1.
> @@ -819,14 +1003,21 @@ static int verity_parse_opt_args(struct dm_arg_set *as, struct dm_verity *v)
>   */
>  static int verity_ctr(struct dm_target *ti, unsigned argc, char **argv)
>  {
> +	struct verity_args args = { 0 };
>  	struct dm_verity *v;
>  	struct dm_arg_set as;
> -	unsigned int num;
> -	unsigned long long num_ll;
>  	int r;
>  	int i;
>  	sector_t hash_position;
> -	char dummy;
> +
> +	if (argc >= DM_VERITY_NUM_POSITIONAL_ARGS)
> +		ti->error = positional_args(argc, argv, &args);
> +	else
> +		ti->error = chromeos_args(argc, argv, &args);
> +	if (ti->error)
> +		return -EINVAL;
> +	if (0)
> +		pr_args(&args);
>  
>  	v = kzalloc(sizeof(struct dm_verity), GFP_KERNEL);
>  	if (!v) {
> @@ -840,83 +1031,46 @@ static int verity_ctr(struct dm_target *ti, unsigned argc, char **argv)
>  	if (r)
>  		goto bad;
>  
> -	if ((dm_table_get_mode(ti->table) & ~FMODE_READ)) {
> -		ti->error = "Device must be readonly";
> -		r = -EINVAL;
> -		goto bad;
> -	}
> +	v->version = args.version;
>  
> -	if (argc < 10) {
> -		ti->error = "Not enough arguments";
> -		r = -EINVAL;
> -		goto bad;
> -	}
> -
> -	if (sscanf(argv[0], "%u%c", &num, &dummy) != 1 ||
> -	    num > 1) {
> -		ti->error = "Invalid version";
> -		r = -EINVAL;
> -		goto bad;
> -	}
> -	v->version = num;
> -
> -	r = dm_get_device(ti, argv[1], FMODE_READ, &v->data_dev);
> +	r = verity_get_device(ti, args.data_device, &v->data_dev);
>  	if (r) {
>  		ti->error = "Data device lookup failed";
>  		goto bad;
>  	}
>  
> -	r = dm_get_device(ti, argv[2], FMODE_READ, &v->hash_dev);
> +	r = verity_get_device(ti, args.hash_device, &v->hash_dev);
>  	if (r) {
>  		ti->error = "Hash device lookup failed";
>  		goto bad;
>  	}
>  
> -	if (sscanf(argv[3], "%u%c", &num, &dummy) != 1 ||
> -	    !num || (num & (num - 1)) ||
> -	    num < bdev_logical_block_size(v->data_dev->bdev) ||
> -	    num > PAGE_SIZE) {
> +	v->data_dev_block_bits = args.data_block_size_bits;
> +	if ((1 << v->data_dev_block_bits) <
> +	    bdev_logical_block_size(v->data_dev->bdev)) {
>  		ti->error = "Invalid data device block size";
>  		r = -EINVAL;
>  		goto bad;
>  	}
> -	v->data_dev_block_bits = __ffs(num);
>  
> -	if (sscanf(argv[4], "%u%c", &num, &dummy) != 1 ||
> -	    !num || (num & (num - 1)) ||
> -	    num < bdev_logical_block_size(v->hash_dev->bdev) ||
> -	    num > INT_MAX) {
> +	v->hash_dev_block_bits = args.hash_block_size_bits;
> +	if ((1 << v->data_dev_block_bits) <
> +	    bdev_logical_block_size(v->hash_dev->bdev)) {
>  		ti->error = "Invalid hash device block size";
>  		r = -EINVAL;
>  		goto bad;
>  	}
> -	v->hash_dev_block_bits = __ffs(num);
> -
> -	if (sscanf(argv[5], "%llu%c", &num_ll, &dummy) != 1 ||
> -	    (sector_t)(num_ll << (v->data_dev_block_bits - SECTOR_SHIFT))
> -	    >> (v->data_dev_block_bits - SECTOR_SHIFT) != num_ll) {
> -		ti->error = "Invalid data blocks";
> -		r = -EINVAL;
> -		goto bad;
> -	}
> -	v->data_blocks = num_ll;
>  
> +	v->data_blocks = args.num_data_blocks;
>  	if (ti->len > (v->data_blocks << (v->data_dev_block_bits - SECTOR_SHIFT))) {
>  		ti->error = "Data device is too small";
>  		r = -EINVAL;
>  		goto bad;
>  	}
>  
> -	if (sscanf(argv[6], "%llu%c", &num_ll, &dummy) != 1 ||
> -	    (sector_t)(num_ll << (v->hash_dev_block_bits - SECTOR_SHIFT))
> -	    >> (v->hash_dev_block_bits - SECTOR_SHIFT) != num_ll) {
> -		ti->error = "Invalid hash start";
> -		r = -EINVAL;
> -		goto bad;
> -	}
> -	v->hash_start = num_ll;
> +	v->hash_start = args.hash_start_block;
>  
> -	v->alg_name = kstrdup(argv[7], GFP_KERNEL);
> +	v->alg_name = kstrdup(args.algorithm, GFP_KERNEL);
>  	if (!v->alg_name) {
>  		ti->error = "Cannot allocate algorithm name";
>  		r = -ENOMEM;
> @@ -945,36 +1099,33 @@ static int verity_ctr(struct dm_target *ti, unsigned argc, char **argv)
>  		r = -ENOMEM;
>  		goto bad;
>  	}
> -	if (strlen(argv[8]) != v->digest_size * 2 ||
> -	    hex2bin(v->root_digest, argv[8], v->digest_size)) {
> +	if (strlen(args.digest) != v->digest_size * 2 ||
> +	    hex2bin(v->root_digest, args.digest, v->digest_size)) {
>  		ti->error = "Invalid root digest";
>  		r = -EINVAL;
>  		goto bad;
>  	}
>  
> -	if (strcmp(argv[9], "-")) {
> -		v->salt_size = strlen(argv[9]) / 2;
> +	if (strcmp(args.salt, "-")) {
> +		v->salt_size = strlen(args.salt) / 2;
>  		v->salt = kmalloc(v->salt_size, GFP_KERNEL);
>  		if (!v->salt) {
>  			ti->error = "Cannot allocate salt";
>  			r = -ENOMEM;
>  			goto bad;
>  		}
> -		if (strlen(argv[9]) != v->salt_size * 2 ||
> -		    hex2bin(v->salt, argv[9], v->salt_size)) {
> +		if (strlen(args.salt) != v->salt_size * 2 ||
> +		    hex2bin(v->salt, args.salt, v->salt_size)) {
>  			ti->error = "Invalid salt";
>  			r = -EINVAL;
>  			goto bad;
>  		}
>  	}
>  
> -	argv += 10;
> -	argc -= 10;
> -
>  	/* Optional parameters */
> -	if (argc) {
> -		as.argc = argc;
> -		as.argv = argv;
> +	if (argc > DM_VERITY_NUM_POSITIONAL_ARGS) {
> +		as.argc = argc - DM_VERITY_NUM_POSITIONAL_ARGS;
> +		as.argv = argv + DM_VERITY_NUM_POSITIONAL_ARGS;
>  
>  		r = verity_parse_opt_args(&as, v);
>  		if (r < 0)
> diff --git a/init/do_mounts_dm.c b/init/do_mounts_dm.c
> index 54c3299..25a040e 100644
> --- a/init/do_mounts_dm.c
> +++ b/init/do_mounts_dm.c
> @@ -187,8 +187,7 @@ static char * __init dm_parse_device(struct dm_device *dev, char *str)
>  	if (opt.delim == DM_FIELD_SEP[0]) {
>  		if (!get_dm_option(&opt, DM_LINE_SEP))
>  			return NULL;
> -		if (kstrtoul(opt.start, 10, &dev->num_targets))
> -			dev->num_targets = 1;
> +		dev->num_targets = simple_strtoul(opt.start, NULL, 10);
>  	} else {
>  		dev->num_targets = 1;
>  	}
> @@ -214,7 +213,7 @@ static char * __init dm_parse_targets(struct dm_device *dev, char *str)
>  	 */
>  	opt.next = str;
>  	for (i = 0; i < num_targets; i++) {
> -		*target = kzalloc(sizeof(*target), GFP_KERNEL);
> +		*target = kzalloc(sizeof(struct dm_setup_target), GFP_KERNEL);
>  		if (!*target) {
>  			DMERR("failed to allocate memory for target %s<%ld>",
>  			      dev->name, i);
> @@ -227,18 +226,14 @@ static char * __init dm_parse_targets(struct dm_device *dev, char *str)
>  			      " for target %s<%ld>", dev->name, i);
>  			goto parse_fail;
>  		}
> -
> -		if (kstrtoull(opt.start, 10, &(*target)->begin))
> -			goto parse_fail;
> +		(*target)->begin = simple_strtoull(opt.start, NULL, 10);
>  
>  		if (!get_dm_option(&opt, DM_FIELD_SEP)) {
>  			DMERR("failed to parse length for target %s<%ld>",
>  			      dev->name, i);
>  			goto parse_fail;
>  		}
> -
> -		if (kstrtoull(opt.start, 10, &(*target)->length))
> -			goto parse_fail;
> +		(*target)->length = simple_strtoull(opt.start, NULL, 10);
>  
>  		if (get_dm_option(&opt, DM_FIELD_SEP))
>  			(*target)->type = kstrndup(opt.start, opt.len,
> @@ -328,8 +323,7 @@ static int __init dm_setup(char *str)
>  	if (!get_dm_option(&opt, DM_FIELD_SEP))
>  		goto parse_fail;
>  	if (isdigit(opt.start[0])) {	/* Optional number field */
> -		if (kstrtoul(opt.start, 10, &num_devices))
> -			num_devices = 1;
> +		num_devices = simple_strtoul(opt.start, NULL, 10);
>  		str = opt.next;
>  	} else {
>  		num_devices = 1;
> 

Re: [PATCH 5/5] dm verity: add support for version 0 of the on-disk format

[Enric Balletbo i Serra]

On 11/04/17 19:21, Milan Broz wrote:
> On 04/11/2017 05:33 PM, Enric Balletbo i Serra wrote:
>> From: Will Drewry <wad@chromium.org>
>>
>> Version 0 of the on-disk format is compatible with the format used in the
>> Chromium OS. This patch adds support for this version.
>>
>> Format type 0 is the original Chrome OS version, whereas the format type 1
>> is current version, but 0, the original format used in the Chromium OS is
>> still used in most devices. This patch adds support for the original
>> on-disk format so you can decide which want you want to use.
> 
> NACK!
> 
> What the ...  is this patch doing?
> 

Argh! Very sorry about that was *not* my intention add this patch in this
series, I was only trying to submit 1 to 4 which contains the patches already
send long time ago and the patches to move to the public header some functions
needed for init/do_mount_dm.c

I'm using this specific patch to test 1 to 4 and to boot against a chromeos
rootfs as it parses the current chromeos args, this patch also has the use of
some deprecated functions like simple_strtoull, so definitely this *shouldn't*
be here, my bad.

Besides that, with the below comments, you have made me see that I have some
lacks and I need to rethink some things that I wanted to send after this series,
so many thanks and sorry again if you spend some time looking at this.

> Isn't the format 0 already there? According to
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/device-mapper/verity.txt
> 
> <version>
>     This is the type of the on-disk hash format.
> 
>     0 is the original format used in the Chromium OS.
>       The salt is appended when hashing, digests are stored continuously and
>       the rest of the block is padded with zeroes.
> 
> Reading this patch, it does something completely different than it describes
> in header.
> 
> How is superblock format related to:
> +MODULE_PARM_DESC(dev_wait, "Wait forever for a backing device");
> ...
> not mentioning complete rewrite of verity table parsing... why?
> 
> Also please note that there is userspace (veritysetup) that have to work with
> the old format as well (I think it does already).
> I think we solved the compatibility years ago.
> 
> If not, please describe properly what is missing.
> I do not see any on-disk format changes here...
> 
> Milan
> 
>>
>> Signed-off-by: Will Drewry <wad@chromium.org>
>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>> ---
>>  drivers/md/dm-verity-target.c | 279 ++++++++++++++++++++++++++++++++----------
>>  init/do_mounts_dm.c           |  16 +--
>>  2 files changed, 220 insertions(+), 75 deletions(-)
>>
>> diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c
>> index 7335d8a..c098d22 100644
>> --- a/drivers/md/dm-verity-target.c
>> +++ b/drivers/md/dm-verity-target.c
>> @@ -17,6 +17,7 @@
>>  #include "dm-verity.h"
>>  #include "dm-verity-fec.h"
>>  
>> +#include <linux/delay.h>
>>  #include <linux/module.h>
>>  #include <linux/reboot.h>
>>  
>> @@ -28,6 +29,7 @@
>>  #define DM_VERITY_DEFAULT_PREFETCH_SIZE	262144
>>  
>>  #define DM_VERITY_MAX_CORRUPTED_ERRS	100
>> +#define DM_VERITY_NUM_POSITIONAL_ARGS	10
>>  
>>  #define DM_VERITY_OPT_LOGGING		"ignore_corruption"
>>  #define DM_VERITY_OPT_RESTART		"restart_on_corruption"
>> @@ -46,6 +48,11 @@ struct dm_verity_prefetch_work {
>>  	unsigned n_blocks;
>>  };
>>  
>> +/* Controls whether verity_get_device will wait forever for a device. */
>> +static int dev_wait;
>> +module_param(dev_wait, int, 0444);
>> +MODULE_PARM_DESC(dev_wait, "Wait forever for a backing device");
>> +
>>  /*
>>   * Auxiliary structure appended to each dm-bufio buffer. If the value
>>   * hash_verified is nonzero, hash of the block has been verified.
>> @@ -803,6 +810,183 @@ static int verity_parse_opt_args(struct dm_arg_set *as, struct dm_verity *v)
>>  	return r;
>>  }
>>  
>> +static int verity_get_device(struct dm_target *ti, const char *devname,
>> +			     struct dm_dev **dm_dev)
>> +{
>> +	do {
>> +		/* Try the normal path first since if everything is ready, it
>> +		 * will be the fastest.
>> +		 */
>> +		if (!dm_get_device(ti, devname, /*FMODE_READ*/
>> +				   dm_table_get_mode(ti->table), dm_dev))
>> +			return 0;
>> +
>> +		if (!dev_wait)
>> +			break;
>> +
>> +		/* No need to be too aggressive since this is a slow path. */
>> +		msleep(500);
>> +	} while (driver_probe_done() != 0 || !(*dm_dev));
>> +	return -1;
>> +}
>> +
>> +struct verity_args {
>> +	int version;
>> +	char *data_device;
>> +	char *hash_device;
>> +	int data_block_size_bits;
>> +	int hash_block_size_bits;
>> +	u64 num_data_blocks;
>> +	u64 hash_start_block;
>> +	char *algorithm;
>> +	char *digest;
>> +	char *salt;
>> +};
>> +
>> +static void pr_args(struct verity_args *args)
>> +{
>> +	printk(KERN_INFO "VERITY args: version=%d data_device=%s hash_device=%s"
>> +		" data_block_size_bits=%d hash_block_size_bits=%d"
>> +		" num_data_blocks=%lld hash_start_block=%lld"
>> +		" algorithm=%s digest=%s salt=%s\n",
>> +		args->version,
>> +		args->data_device,
>> +		args->hash_device,
>> +		args->data_block_size_bits,
>> +		args->hash_block_size_bits,
>> +		args->num_data_blocks,
>> +		args->hash_start_block,
>> +		args->algorithm,
>> +		args->digest,
>> +		args->salt);
>> +}
>> +
>> +/*
>> + * positional_args - collects the argments using the positional
>> + * parameters.
>> + * arg# - parameter
>> + *    0 - version
>> + *    1 - data device
>> + *    2 - hash device - may be same as data device
>> + *    3 - data block size log2
>> + *    4 - hash block size log2
>> + *    5 - number of data blocks
>> + *    6 - hash start block
>> + *    7 - algorithm
>> + *    8 - digest
>> + *    9 - salt
>> + */
>> +static char *positional_args(unsigned argc, char **argv,
>> +			     struct verity_args *args)
>> +{
>> +	unsigned int num;
>> +	unsigned long long num_ll;
>> +	char dummy;
>> +
>> +	if (argc < DM_VERITY_NUM_POSITIONAL_ARGS)
>> +		return "Invalid argument count: at least 10 arguments required";
>> +
>> +	if (sscanf(argv[0], "%d%c", &num, &dummy) != 1 ||
>> +	    num < 0 || num > 1)
>> +		return "Invalid version";
>> +	args->version = num;
>> +
>> +	args->data_device = argv[1];
>> +	args->hash_device = argv[2];
>> +
>> +	if (sscanf(argv[3], "%u%c", &num, &dummy) != 1 ||
>> +	    !num || (num & (num - 1)) ||
>> +	    num > PAGE_SIZE)
>> +		return "Invalid data device block size";
>> +	args->data_block_size_bits = ffs(num) - 1;
>> +
>> +	if (sscanf(argv[4], "%u%c", &num, &dummy) != 1 ||
>> +	    !num || (num & (num - 1)) ||
>> +	    num > INT_MAX)
>> +		return "Invalid hash device block size";
>> +	args->hash_block_size_bits = ffs(num) - 1;
>> +
>> +	if (sscanf(argv[5], "%llu%c", &num_ll, &dummy) != 1 ||
>> +	    (sector_t)(num_ll << (args->data_block_size_bits - SECTOR_SHIFT))
>> +	    >> (args->data_block_size_bits - SECTOR_SHIFT) != num_ll)
>> +		return "Invalid data blocks";
>> +	args->num_data_blocks = num_ll;
>> +
>> +	if (sscanf(argv[6], "%llu%c", &num_ll, &dummy) != 1 ||
>> +	    (sector_t)(num_ll << (args->hash_block_size_bits - SECTOR_SHIFT))
>> +	    >> (args->hash_block_size_bits - SECTOR_SHIFT) != num_ll)
>> +		return "Invalid hash start";
>> +	args->hash_start_block = num_ll;
>> +
>> +	args->algorithm = argv[7];
>> +	args->digest = argv[8];
>> +	args->salt = argv[9];
>> +
>> +	return NULL;
>> +}
>> +
>> +static void splitarg(char *arg, char **key, char **val)
>> +{
>> +	*key = strsep(&arg, "=");
>> +	*val = strsep(&arg, "");
>> +}
>> +
>> +static char *chromeos_args(unsigned int argc, char **argv,
>> +			   struct verity_args *args)
>> +{
>> +	char *key, *val;
>> +	unsigned long num;
>> +	int i;
>> +
>> +	args->version = 0;
>> +	args->data_block_size_bits = 12;
>> +	args->hash_block_size_bits = 12;
>> +	for (i = 0; i < argc; ++i) {
>> +		DMWARN("Argument %d: '%s'", i, argv[i]);
>> +		splitarg(argv[i], &key, &val);
>> +		if (!key) {
>> +			DMWARN("Bad argument %d: missing key?", i);
>> +			return "Bad argument: missing key";
>> +		}
>> +		if (!val) {
>> +			DMWARN("Bad argument %d='%s': missing value", i, key);
>> +			return "Bad argument: missing value";
>> +		}
>> +		if (!strcmp(key, "alg")) {
>> +			args->algorithm = val;
>> +		} else if (!strcmp(key, "payload")) {
>> +			args->data_device = val;
>> +		} else if (!strcmp(key, "hashtree")) {
>> +			args->hash_device = val;
>> +		} else if (!strcmp(key, "root_hexdigest")) {
>> +			args->digest = val;
>> +		} else if (!strcmp(key, "hashstart")) {
>> +			if (kstrtoul(val, 10, &num))
>> +				return "Invalid hashstart";
>> +			args->hash_start_block =
>> +			   num >> (args->hash_block_size_bits - SECTOR_SHIFT);
>> +			args->num_data_blocks = args->hash_start_block;
>> +		} else if (!strcmp(key, "salt")) {
>> +			args->salt = val;
>> +		}
>> +	}
>> +	if (!args->salt)
>> +		args->salt = "";
>> +
>> +#define NEEDARG(n) do { \
>> +	if (!(n)) { \
>> +		return "Missing argument: " #n; \
>> +	} } while (0)
>> +
>> +	NEEDARG(args->algorithm);
>> +	NEEDARG(args->data_device);
>> +	NEEDARG(args->hash_device);
>> +	NEEDARG(args->digest);
>> +
>> +#undef NEEDARG
>> +	return NULL;
>> +}
>> +
>>  /*
>>   * Target parameters:
>>   *	<version>	The current format is version 1.
>> @@ -819,14 +1003,21 @@ static int verity_parse_opt_args(struct dm_arg_set *as, struct dm_verity *v)
>>   */
>>  static int verity_ctr(struct dm_target *ti, unsigned argc, char **argv)
>>  {
>> +	struct verity_args args = { 0 };
>>  	struct dm_verity *v;
>>  	struct dm_arg_set as;
>> -	unsigned int num;
>> -	unsigned long long num_ll;
>>  	int r;
>>  	int i;
>>  	sector_t hash_position;
>> -	char dummy;
>> +
>> +	if (argc >= DM_VERITY_NUM_POSITIONAL_ARGS)
>> +		ti->error = positional_args(argc, argv, &args);
>> +	else
>> +		ti->error = chromeos_args(argc, argv, &args);
>> +	if (ti->error)
>> +		return -EINVAL;
>> +	if (0)
>> +		pr_args(&args);
>>  
>>  	v = kzalloc(sizeof(struct dm_verity), GFP_KERNEL);
>>  	if (!v) {
>> @@ -840,83 +1031,46 @@ static int verity_ctr(struct dm_target *ti, unsigned argc, char **argv)
>>  	if (r)
>>  		goto bad;
>>  
>> -	if ((dm_table_get_mode(ti->table) & ~FMODE_READ)) {
>> -		ti->error = "Device must be readonly";
>> -		r = -EINVAL;
>> -		goto bad;
>> -	}
>> +	v->version = args.version;
>>  
>> -	if (argc < 10) {
>> -		ti->error = "Not enough arguments";
>> -		r = -EINVAL;
>> -		goto bad;
>> -	}
>> -
>> -	if (sscanf(argv[0], "%u%c", &num, &dummy) != 1 ||
>> -	    num > 1) {
>> -		ti->error = "Invalid version";
>> -		r = -EINVAL;
>> -		goto bad;
>> -	}
>> -	v->version = num;
>> -
>> -	r = dm_get_device(ti, argv[1], FMODE_READ, &v->data_dev);
>> +	r = verity_get_device(ti, args.data_device, &v->data_dev);
>>  	if (r) {
>>  		ti->error = "Data device lookup failed";
>>  		goto bad;
>>  	}
>>  
>> -	r = dm_get_device(ti, argv[2], FMODE_READ, &v->hash_dev);
>> +	r = verity_get_device(ti, args.hash_device, &v->hash_dev);
>>  	if (r) {
>>  		ti->error = "Hash device lookup failed";
>>  		goto bad;
>>  	}
>>  
>> -	if (sscanf(argv[3], "%u%c", &num, &dummy) != 1 ||
>> -	    !num || (num & (num - 1)) ||
>> -	    num < bdev_logical_block_size(v->data_dev->bdev) ||
>> -	    num > PAGE_SIZE) {
>> +	v->data_dev_block_bits = args.data_block_size_bits;
>> +	if ((1 << v->data_dev_block_bits) <
>> +	    bdev_logical_block_size(v->data_dev->bdev)) {
>>  		ti->error = "Invalid data device block size";
>>  		r = -EINVAL;
>>  		goto bad;
>>  	}
>> -	v->data_dev_block_bits = __ffs(num);
>>  
>> -	if (sscanf(argv[4], "%u%c", &num, &dummy) != 1 ||
>> -	    !num || (num & (num - 1)) ||
>> -	    num < bdev_logical_block_size(v->hash_dev->bdev) ||
>> -	    num > INT_MAX) {
>> +	v->hash_dev_block_bits = args.hash_block_size_bits;
>> +	if ((1 << v->data_dev_block_bits) <
>> +	    bdev_logical_block_size(v->hash_dev->bdev)) {
>>  		ti->error = "Invalid hash device block size";
>>  		r = -EINVAL;
>>  		goto bad;
>>  	}
>> -	v->hash_dev_block_bits = __ffs(num);
>> -
>> -	if (sscanf(argv[5], "%llu%c", &num_ll, &dummy) != 1 ||
>> -	    (sector_t)(num_ll << (v->data_dev_block_bits - SECTOR_SHIFT))
>> -	    >> (v->data_dev_block_bits - SECTOR_SHIFT) != num_ll) {
>> -		ti->error = "Invalid data blocks";
>> -		r = -EINVAL;
>> -		goto bad;
>> -	}
>> -	v->data_blocks = num_ll;
>>  
>> +	v->data_blocks = args.num_data_blocks;
>>  	if (ti->len > (v->data_blocks << (v->data_dev_block_bits - SECTOR_SHIFT))) {
>>  		ti->error = "Data device is too small";
>>  		r = -EINVAL;
>>  		goto bad;
>>  	}
>>  
>> -	if (sscanf(argv[6], "%llu%c", &num_ll, &dummy) != 1 ||
>> -	    (sector_t)(num_ll << (v->hash_dev_block_bits - SECTOR_SHIFT))
>> -	    >> (v->hash_dev_block_bits - SECTOR_SHIFT) != num_ll) {
>> -		ti->error = "Invalid hash start";
>> -		r = -EINVAL;
>> -		goto bad;
>> -	}
>> -	v->hash_start = num_ll;
>> +	v->hash_start = args.hash_start_block;
>>  
>> -	v->alg_name = kstrdup(argv[7], GFP_KERNEL);
>> +	v->alg_name = kstrdup(args.algorithm, GFP_KERNEL);
>>  	if (!v->alg_name) {
>>  		ti->error = "Cannot allocate algorithm name";
>>  		r = -ENOMEM;
>> @@ -945,36 +1099,33 @@ static int verity_ctr(struct dm_target *ti, unsigned argc, char **argv)
>>  		r = -ENOMEM;
>>  		goto bad;
>>  	}
>> -	if (strlen(argv[8]) != v->digest_size * 2 ||
>> -	    hex2bin(v->root_digest, argv[8], v->digest_size)) {
>> +	if (strlen(args.digest) != v->digest_size * 2 ||
>> +	    hex2bin(v->root_digest, args.digest, v->digest_size)) {
>>  		ti->error = "Invalid root digest";
>>  		r = -EINVAL;
>>  		goto bad;
>>  	}
>>  
>> -	if (strcmp(argv[9], "-")) {
>> -		v->salt_size = strlen(argv[9]) / 2;
>> +	if (strcmp(args.salt, "-")) {
>> +		v->salt_size = strlen(args.salt) / 2;
>>  		v->salt = kmalloc(v->salt_size, GFP_KERNEL);
>>  		if (!v->salt) {
>>  			ti->error = "Cannot allocate salt";
>>  			r = -ENOMEM;
>>  			goto bad;
>>  		}
>> -		if (strlen(argv[9]) != v->salt_size * 2 ||
>> -		    hex2bin(v->salt, argv[9], v->salt_size)) {
>> +		if (strlen(args.salt) != v->salt_size * 2 ||
>> +		    hex2bin(v->salt, args.salt, v->salt_size)) {
>>  			ti->error = "Invalid salt";
>>  			r = -EINVAL;
>>  			goto bad;
>>  		}
>>  	}
>>  
>> -	argv += 10;
>> -	argc -= 10;
>> -
>>  	/* Optional parameters */
>> -	if (argc) {
>> -		as.argc = argc;
>> -		as.argv = argv;
>> +	if (argc > DM_VERITY_NUM_POSITIONAL_ARGS) {
>> +		as.argc = argc - DM_VERITY_NUM_POSITIONAL_ARGS;
>> +		as.argv = argv + DM_VERITY_NUM_POSITIONAL_ARGS;
>>  
>>  		r = verity_parse_opt_args(&as, v);
>>  		if (r < 0)
>> diff --git a/init/do_mounts_dm.c b/init/do_mounts_dm.c
>> index 54c3299..25a040e 100644
>> --- a/init/do_mounts_dm.c
>> +++ b/init/do_mounts_dm.c
>> @@ -187,8 +187,7 @@ static char * __init dm_parse_device(struct dm_device *dev, char *str)
>>  	if (opt.delim == DM_FIELD_SEP[0]) {
>>  		if (!get_dm_option(&opt, DM_LINE_SEP))
>>  			return NULL;
>> -		if (kstrtoul(opt.start, 10, &dev->num_targets))
>> -			dev->num_targets = 1;
>> +		dev->num_targets = simple_strtoul(opt.start, NULL, 10);
>>  	} else {
>>  		dev->num_targets = 1;
>>  	}
>> @@ -214,7 +213,7 @@ static char * __init dm_parse_targets(struct dm_device *dev, char *str)
>>  	 */
>>  	opt.next = str;
>>  	for (i = 0; i < num_targets; i++) {
>> -		*target = kzalloc(sizeof(*target), GFP_KERNEL);
>> +		*target = kzalloc(sizeof(struct dm_setup_target), GFP_KERNEL);
>>  		if (!*target) {
>>  			DMERR("failed to allocate memory for target %s<%ld>",
>>  			      dev->name, i);
>> @@ -227,18 +226,14 @@ static char * __init dm_parse_targets(struct dm_device *dev, char *str)
>>  			      " for target %s<%ld>", dev->name, i);
>>  			goto parse_fail;
>>  		}
>> -
>> -		if (kstrtoull(opt.start, 10, &(*target)->begin))
>> -			goto parse_fail;
>> +		(*target)->begin = simple_strtoull(opt.start, NULL, 10);
>>  
>>  		if (!get_dm_option(&opt, DM_FIELD_SEP)) {
>>  			DMERR("failed to parse length for target %s<%ld>",
>>  			      dev->name, i);
>>  			goto parse_fail;
>>  		}
>> -
>> -		if (kstrtoull(opt.start, 10, &(*target)->length))
>> -			goto parse_fail;
>> +		(*target)->length = simple_strtoull(opt.start, NULL, 10);
>>  
>>  		if (get_dm_option(&opt, DM_FIELD_SEP))
>>  			(*target)->type = kstrndup(opt.start, opt.len,
>> @@ -328,8 +323,7 @@ static int __init dm_setup(char *str)
>>  	if (!get_dm_option(&opt, DM_FIELD_SEP))
>>  		goto parse_fail;
>>  	if (isdigit(opt.start[0])) {	/* Optional number field */
>> -		if (kstrtoul(opt.start, 10, &num_devices))
>> -			num_devices = 1;
>> +		num_devices = simple_strtoul(opt.start, NULL, 10);
>>  		str = opt.next;
>>  	} else {
>>  		num_devices = 1;
>>
> 

Re: [PATCH 4/5] init: add support to directly boot to a mapped device

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2806 bytes --]

Hi Will,

[auto build test ERROR on dm/for-next]
[also build test ERROR on v4.11-rc6 next-20170411]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Enric-Balletbo-i-Serra/dm-boot-a-mapped-device-without-an-initramfs/20170412-071136
base:   https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git for-next
config: x86_64-randconfig-x010-201715 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

Note: the linux-review/Enric-Balletbo-i-Serra/dm-boot-a-mapped-device-without-an-initramfs/20170412-071136 HEAD 76c4f3dfe69e92fc617c7d30e966931aec694c75 builds fine.
      It only hurts bisectibility.

All errors (new ones prefixed by >>):

   init/do_mounts_dm.c: In function 'dm_parse_targets':
>> init/do_mounts_dm.c:231:32: error: passing argument 3 of 'kstrtoull' from incompatible pointer type [-Werror=incompatible-pointer-types]
      if (kstrtoull(opt.start, 10, &(*target)->begin))
                                   ^
   In file included from include/linux/list.h:8:0,
                    from include/linux/async.h:16,
                    from init/do_mounts_dm.c:8:
   include/linux/kernel.h:282:18: note: expected 'long long unsigned int *' but argument is of type 'sector_t * {aka long unsigned int *}'
    int __must_check kstrtoull(const char *s, unsigned int base, unsigned long long *res);
                     ^~~~~~~~~
   init/do_mounts_dm.c:240:32: error: passing argument 3 of 'kstrtoull' from incompatible pointer type [-Werror=incompatible-pointer-types]
      if (kstrtoull(opt.start, 10, &(*target)->length))
                                   ^
   In file included from include/linux/list.h:8:0,
                    from include/linux/async.h:16,
                    from init/do_mounts_dm.c:8:
   include/linux/kernel.h:282:18: note: expected 'long long unsigned int *' but argument is of type 'sector_t * {aka long unsigned int *}'
    int __must_check kstrtoull(const char *s, unsigned int base, unsigned long long *res);
                     ^~~~~~~~~
   cc1: some warnings being treated as errors

vim +/kstrtoull +231 init/do_mounts_dm.c

   225			if (!get_dm_option(&opt, DM_FIELD_SEP)) {
   226				DMERR("failed to parse starting sector"
   227				      " for target %s<%ld>", dev->name, i);
   228				goto parse_fail;
   229			}
   230	
 > 231			if (kstrtoull(opt.start, 10, &(*target)->begin))
   232				goto parse_fail;
   233	
   234			if (!get_dm_option(&opt, DM_FIELD_SEP)) {

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 25676 bytes --]