Re: [PATCH 5/5] dm verity: add support for version 0 of the on-disk format
On 11/04/17 19:21, Milan Broz wrote:
> On 04/11/2017 05:33 PM, Enric Balletbo i Serra wrote:
>> From: Will Drewry
>>
>> 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
>
>
> 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
>> Signed-off-by: Enric Balletbo i Serra
>> ---
>> 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
>> #include
>> #include
>>
>> @@ -28,6 +29,7 @@
>> #define DM_VERITY_DEFAULT_PREFETCH_SIZE 262144
>>
>> #define DM_VERITY_MAX_CORRUPTED_ERRS100
>> +#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_devi
Re: [PATCH 5/5] dm verity: add support for version 0 of the on-disk format
On 04/11/2017 05:33 PM, Enric Balletbo i Serra wrote:
> From: Will Drewry
>
> 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
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
> Signed-off-by: Enric Balletbo i Serra
> ---
> 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
> #include
> #include
>
> @@ -28,6 +29,7 @@
> #define DM_VERITY_DEFAULT_PREFETCH_SIZE 262144
>
> #define DM_VERITY_MAX_CORRUPTED_ERRS 100
> +#define DM_VERITY_NUM_POSITIONAL_ARGS10
>
> #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 star
[PATCH 5/5] dm verity: add support for version 0 of the on-disk format
From: Will Drewry
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
Signed-off-by: Enric Balletbo i Serra
---
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
#include
#include
@@ -28,6 +29,7 @@
#define DM_VERITY_DEFAULT_PREFETCH_SIZE262144
#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_
