Re: [dm-devel] [PATCH v2 1/2] dm: verity support data device offset (Linux 3.4.7)

2012-08-09 Thread Wesley Miaw 
On Aug 8, 2012, at 7:04 PM, Alasdair G Kergon wrote:
> 
> However, first you need to address the second part of Mikulas's email,
> namely to make the case for these changes rather than making no kernel
> changes and instead stacking the verity target over a linear target.
> 
> To put this another way, your patch offers an alternative way to do
> something we think the existing kernel can already do, so you need to
> advance some reasons why you believe the new alternative method is worth
> adding to the kernel and explain this in the patch header.

I'm afraid for some reason I didn't get Mikulas's first email, only the reply 
from Milan which must have been an incomplete quote of Mikulas's text.

For my part, I approached this as porting my existing code to the new dm-verity 
implementation included in Linux 3.4. The previous code used a data offset as 
this was convenient and directly supported the block device image format I put 
together back then.

However I can indeed accomplish what I need using linear, it's just a bit more 
code. I am not able to measure any runtime performance difference. The primary 
benefit I can see for is the reduced kernel footprint if the linear target does 
not need to be included (and my corresponding setup code is about 1/3 smaller). 
With my cross-compiled kernel the savings is ~1KB; this is obviously a very 
small benefit.

So I would defer to others on this. While supporting the data offset would be 
convenient and match well with my specific use case I can live without it and I 
don't think the size cost is significant enough to matter. I expect to get some 
feedback from other developers in the coming months regarding my Linux 3.4 
integration but I doubt it would change my current opinion on the matter.

Thanks,
--
Wesley Miaw

signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: [dm-devel] [PATCH v2 2/2] dm: verity support data device offset (Linux 3.4.7)

2012-08-09 Thread Wesley Miaw 
On Aug 8, 2012, at 11:35 PM, Milan Broz wrote:

> On 08/09/2012 02:40 AM, Wesley Miaw wrote:
>> 
>> 
>> This isn't as polished because I pretty much just added support to do
>> what I needed. I'm not sure if the LKML is the right place to post,
>> so let me know if I should send this somewhere else.
> 
> This is libcryptsetup userspace so better list for this is dmcrypt
> mailing list (and/or cc me, I will handle these changes anyway).

I will continue this thread on the dm-crypt mailing list.

Thanks,
--
Wesley Miaw

signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: [dm-devel] [PATCH v2 2/2] dm: verity support data device offset (Linux 3.4.7)

2012-08-09 Thread Wesley Miaw 
On Aug 8, 2012, at 11:35 PM, Milan Broz wrote:

 On 08/09/2012 02:40 AM, Wesley Miaw wrote:
 
 
 This isn't as polished because I pretty much just added support to do
 what I needed. I'm not sure if the LKML is the right place to post,
 so let me know if I should send this somewhere else.
 
 This is libcryptsetup userspace so better list for this is dmcrypt
 mailing list (and/or cc me, I will handle these changes anyway).

I will continue this thread on the dm-crypt mailing list.

Thanks,
--
Wesley Miaw

signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: [dm-devel] [PATCH v2 1/2] dm: verity support data device offset (Linux 3.4.7)

2012-08-09 Thread Wesley Miaw 
On Aug 8, 2012, at 7:04 PM, Alasdair G Kergon wrote:
 
 However, first you need to address the second part of Mikulas's email,
 namely to make the case for these changes rather than making no kernel
 changes and instead stacking the verity target over a linear target.
 
 To put this another way, your patch offers an alternative way to do
 something we think the existing kernel can already do, so you need to
 advance some reasons why you believe the new alternative method is worth
 adding to the kernel and explain this in the patch header.

I'm afraid for some reason I didn't get Mikulas's first email, only the reply 
from Milan which must have been an incomplete quote of Mikulas's text.

For my part, I approached this as porting my existing code to the new dm-verity 
implementation included in Linux 3.4. The previous code used a data offset as 
this was convenient and directly supported the block device image format I put 
together back then.

However I can indeed accomplish what I need using linear, it's just a bit more 
code. I am not able to measure any runtime performance difference. The primary 
benefit I can see for is the reduced kernel footprint if the linear target does 
not need to be included (and my corresponding setup code is about 1/3 smaller). 
With my cross-compiled kernel the savings is ~1KB; this is obviously a very 
small benefit.

So I would defer to others on this. While supporting the data offset would be 
convenient and match well with my specific use case I can live without it and I 
don't think the size cost is significant enough to matter. I expect to get some 
feedback from other developers in the coming months regarding my Linux 3.4 
integration but I doubt it would change my current opinion on the matter.

Thanks,
--
Wesley Miaw

signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: [dm-devel] [PATCH v2 2/2] dm: verity support data device offset (Linux 3.4.7)

2012-08-08 Thread Wesley Miaw 
On Aug 8, 2012, at 1:56 PM, Milan Broz wrote:

> On 08/08/2012 10:46 PM, Wesley Miaw wrote:
> 
>> I did modify veritysetup on my own so the format and verify commands will 
>> work with regular files on disk instead of having to mount through loop 
>> devices.
> 
> Which veritysetup? In upstream (cryptsetup repository) it allocates loop 
> automatically.
> (And for userspace verification it doesn't need loop at all.)
> 
> Anyway, please send a patch for userspace as well then ;-)

This isn't as polished because I pretty much just added support to do what I 
needed. I'm not sure if the LKML is the right place to post, so let me know if 
I should send this somewhere else.

Your previous email implied that veritysetup would need a way to determine if 
the data offset option is supported; I did not modify veritysetup to support 
this idea as I didn't need it.

Thanks.

From: Wesley Miaw 

Allow veritysetup format and verify commands to directly operate on regular
files instead of requiring mounts through loop devices.

Signed-off-by: Wesley Miaw 
---
 cryptsetup/lib/internal.h|1 
 cryptsetup/lib/libcryptsetup.h   |   22 
 cryptsetup/lib/libcryptsetup.sym |2 
 cryptsetup/lib/setup.c   |  133 -
 cryptsetup/lib/utils.c   |   12 ++
 cryptsetup/src/veritysetup.c |   23 +++--
 6 files changed, 183 insertions(+), 10 deletions(-)
--- a/cryptsetup/lib/internal.h 2012-08-08 17:11:20.366392301 -0700
+++ b/cryptsetup/lib/internal.h 2012-08-06 16:17:35.154719491 -0700
@@ -76,6 +76,7 @@ ssize_t read_blockwise(int fd, void *_bu
 ssize_t write_lseek_blockwise(int fd, char *buf, size_t count, off_t offset);
 int device_ready(struct crypt_device *cd, const char *device, int mode);
 int device_size(const char *device, uint64_t *size);
+int file_size(const char *filename, uint64_t *size);
 
 unsigned crypt_getpagesize(void);
 
--- a/cryptsetup/lib/libcryptsetup.h2012-08-08 17:11:20.375392929 -0700
+++ b/cryptsetup/lib/libcryptsetup.h2012-08-06 16:17:35.159720699 -0700
@@ -56,6 +57,19 @@ struct crypt_device; /* crypt device han
 int crypt_init(struct crypt_device **cd, const char *device);
 
 /**
+ * Initial crypt device handle from a file and check if provided file exists.
+ *
+ * @param cd Returns pointer to crypt device handle.
+ * @param filename Path to the backing file.
+ *
+ * @return @e 0 on success or negative errno value otherwise.
+ *
+ * @note Note that logging is not initialized here, possible messages uses
+ *   default log function.
+ */
+int crypt_initfile(struct crypt_device **cd, const char *filename);
+
+/**
  * Initialize crypt device handle from provided active device name,
  * and, optionally, from separate metadata (header) device
  * and check if provided device exists.
@@ -237,6 +251,15 @@ void crypt_set_password_verify(struct cr
 int crypt_set_data_device(struct crypt_device *cd, const char *device);
 
 /**
+ * Set data file
+ * For VERITY it is data file when hash device is separated.
+ *
+ * @param cd crypt device handle
+ * @param filename path to data file
+ */
+int crypt_set_data_file(struct crypt_device *cd, const char *device);
+
+/**
  * @defgroup rng "Cryptsetup RNG"
  *
  * @addtogroup rng
--- a/cryptsetup/lib/libcryptsetup.sym  2012-08-08 17:11:20.375392930 -0700
+++ b/cryptsetup/lib/libcryptsetup.sym  2012-08-06 16:17:35.160720941 -0700
@@ -1,6 +1,7 @@
 CRYPTSETUP_1.0 {
global:
crypt_init;
+   crypt_initfile;
crypt_init_by_name;
crypt_init_by_name_and_header;
crypt_set_log_callback;
@@ -13,6 +14,7 @@ CRYPTSETUP_1.0 {
crypt_set_password_verify;
crypt_set_uuid;
crypt_set_data_device;
+   crypt_set_data_file;
 
crypt_memory_lock;
crypt_format;
--- a/cryptsetup/lib/setup.c2012-08-08 17:11:20.428396640 -0700
+++ b/cryptsetup/lib/setup.c2012-08-06 16:17:35.192728669 -0700
@@ -25,6 +25,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 
 #include "libcryptsetup.h"
 #include "luks.h"
@@ -585,6 +587,56 @@ bad:
return r;
 }
 
+int crypt_initfile(struct crypt_device **cd, const char *filename)
+{
+   struct crypt_device *h = NULL;
+   int fd;
+   struct stat st;
+   int r;
+
+   if (!cd)
+   return -EINVAL;
+
+   if (stat(filename, ) < 0) {
+   log_err(NULL, _("File %s doesn't exist or access denied.\n"), 
filename);
+   return -EINVAL;
+   }
+
+   log_dbg("Trying to open and write file %s.", filename);
+   fd = open(filename, O_RDWR);
+   if (fd < 0) {
+   log_err(NULL, _("Cannot open file %s for writeable access.\n"), 
filename);
+   return -EINVAL;
+   }
+   close(fd);
+
+   log_dbg("Allocating crypt

Re: [dm-devel] [PATCH v2 1/2] dm: verity support data device offset (Linux 3.4.7)

2012-08-08 Thread Wesley Miaw 
On Aug 8, 2012, at 1:31 PM, Milan Broz wrote:

> On 08/08/2012 08:46 PM, Mikulas Patocka wrote:
> 
>> The problem with the patch is that it changes interface to the userspace 
>> tool. The userspace tool veritysetup already exists in recent cryptsetup 
>> package, so we can't change the interface - you should change the patch so 
>> that the starting data block is the last argument and the argument is 
>> optional - so that it is compatible with the existing userspace too.
> 
> yes. Please never change interface without at least increasing target version.
> 
> I have to add userspace support as well to veritysetup and we need a way
> how to detect that option is supported by running kernel.

Apologies if the version increment is incorrect; I was not sure if the minor or 
patch number should be incremented. I assume the different version number is 
what would be used to detect if the data offset option is supported. Thanks.

From: Wesley Miaw 

Add data device start block index as optional dm-verity target parameters to
support verity targets where the data does not begin at sector 0 of the block
device.

Also fix the hash block index computations so they take into account any data
offset.

Signed-off-by: Wesley Miaw 
---
 Documentation/device-mapper/verity.txt |8 ++-
 drivers/md/dm-verity.c |   24 ++-
 2 files changed, 26 insertions(+), 6 deletions(-)
--- a/drivers/md/dm-verity.c2012-08-07 16:03:03.778759000 -0700
+++ b/drivers/md/dm-verity.c2012-08-08 17:04:16.344682266 -0700
@@ -477,7 +477,7 @@ static int verity_map(struct dm_target *
return -EIO;
}
 
-   if ((bio->bi_sector + bio_sectors(bio)) >>
+   if ((bio->bi_sector - v->data_start + bio_sectors(bio)) >>
(v->data_dev_block_bits - SECTOR_SHIFT) > v->data_blocks) {
DMERR_LIMIT("io out of range");
return -EIO;
@@ -491,7 +491,7 @@ static int verity_map(struct dm_target *
io->bio = bio;
io->orig_bi_end_io = bio->bi_end_io;
io->orig_bi_private = bio->bi_private;
-   io->block = bio->bi_sector >> (v->data_dev_block_bits - SECTOR_SHIFT);
+   io->block = (bio->bi_sector - v->data_start) >> (v->data_dev_block_bits 
- SECTOR_SHIFT);
io->n_blocks = bio->bi_size >> v->data_dev_block_bits;
 
bio->bi_end_io = verity_end_io;
@@ -646,6 +646,7 @@ static void verity_dtr(struct dm_target 
  * 
  * 
  *   Hex string or "-" if no salt.
+ *   Optional. The default is zero.
  */
 static int verity_ctr(struct dm_target *ti, unsigned argc, char **argv)
 {
@@ -671,8 +672,8 @@ static int verity_ctr(struct dm_target *
goto bad;
}
 
-   if (argc != 10) {
-   ti->error = "Invalid argument count: exactly 10 arguments 
required";
+   if (argc != 10 && argc != 11) {
+   ti->error = "Invalid argument count: 10 or 11 arguments 
required";
r = -EINVAL;
goto bad;
}
@@ -793,6 +794,19 @@ static int verity_ctr(struct dm_target *
}
}
 
+   if (argc == 11) {
+   if (sscanf(argv[10], "%llu%c", _ll, ) != 1 ||
+   num_ll << (v->data_dev_block_bits - SECTOR_SHIFT) !=
+   (sector_t)num_ll << (v->data_dev_block_bits - 
SECTOR_SHIFT)) {
+   ti->error = "Invalid data start";
+   r = -EINVAL;
+   goto bad;
+   }
+   v->data_start = num_ll << (v->data_dev_block_bits - 
SECTOR_SHIFT);
+   } else {
+   v->data_start = 0;
+   }
+
v->hash_per_block_bits =
fls((1 << v->hash_dev_block_bits) / v->digest_size) - 1;
 
@@ -875,7 +889,7 @@ bad:
 
 static struct target_type verity_target = {
.name   = "verity",
-   .version= {1, 0, 0},
+   .version= {1, 1, 0},
.module = THIS_MODULE,
.ctr= verity_ctr,
.dtr= verity_dtr,
--- a/Documentation/device-mapper/verity.txt2012-08-08 11:02:48.558883756 
-0700
+++ b/Documentation/device-mapper/verity.txt2012-08-08 16:50:04.114864090 
-0700
@@ -11,6 +11,7 @@ Construction Parameters
  
  
   
+[]
 
 
 This is the type of the on-disk hash format.
@@ -62,6 +63,10 @@ Construction Parameters
 
 The hexadecimal encoding of the salt value.
 
+
+This is the offset, in -blocks, from the start of data_dev
+to the first block of the data.
+
 Theory of operation
 ===
 
@@ -138,7 +143,8 @@

Re: [dm-devel] [PATCH] dm: verity support data device offset (Linux 3.4.7)

2012-08-08 Thread Wesley Miaw 
On Aug 8, 2012, at 1:56 PM, Milan Broz wrote:

> On 08/08/2012 10:46 PM, Wesley Miaw wrote:
> 
>> I did modify veritysetup on my own so the format and verify commands will 
>> work with regular files on disk instead of having to mount through loop 
>> devices.
> 
> Which veritysetup? In upstream (cryptsetup repository) it allocates loop 
> automatically.
> (And for userspace verification it doesn't need loop at all.)
> 
> Anyway, please send a patch for userspace as well then ;-)

I grabbed cryptsetup from http://code.google.com/p/cryptsetup as what I read 
said that is the most recent. And then modified the code in there because the 
final block device images need to combine the file system, hash data, and some 
metadata into a single image and I don't want my users to need root privileges.

I can send a separate patch of those changes, but I'm not sure to where? Also 
to the LKML?

Thanks,
--
Wesley Miaw

signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: [dm-devel] [PATCH] dm: verity support data device offset (Linux 3.4.7)

2012-08-08 Thread Wesley Miaw 
On Aug 8, 2012, at 1:31 PM, Milan Broz wrote:

> On 08/08/2012 08:46 PM, Mikulas Patocka wrote:
> 
>> The problem with the patch is that it changes interface to the userspace 
>> tool. The userspace tool veritysetup already exists in recent cryptsetup 
>> package, so we can't change the interface - you should change the patch so 
>> that the starting data block is the last argument and the argument is 
>> optional - so that it is compatible with the existing userspace too.
> 
> yes. Please never change interface without at least increasing target version.
> 
> I have to add userspace support as well to veritysetup and we need a way
> how to detect that option is supported by running kernel.


Understood. Thank you for the feedback. I will attempt a new patch version 
which addresses these issues. I also found that I did not correct the 
last-block boundary check so I will re-submit my patch with that as well.

I did modify veritysetup on my own so the format and verify commands will work 
with regular files on disk instead of having to mount through loop devices.
--
Wesley Miaw

signature.asc
Description: Message signed with OpenPGP using GPGMail

[PATCH] dm: verity support data device offset (Linux 3.4.7)

2012-08-08 Thread Wesley Miaw 
From: Wesley Miaw 

Add data device start block index to dm-verity target parameters to support
verity targets where the data does not begin at sector 0 of the block device.
Also fix the hash block index computation so it takes into account data offsets.

Signed-off-by: Wesley Miaw 
---
 Documentation/device-mapper/verity.txt |8 -
 drivers/md/dm-verity.c |   32 +++
 2 files changed, 27 insertions(+), 13 deletions(-)
--- a/drivers/md/dm-verity.c2012-08-07 16:03:03.778759000 -0700
+++ b/drivers/md/dm-verity.c2012-08-07 17:32:02.130176956 -0700
@@ -491,7 +491,7 @@ static int verity_map(struct dm_target *
io->bio = bio;
io->orig_bi_end_io = bio->bi_end_io;
io->orig_bi_private = bio->bi_private;
-   io->block = bio->bi_sector >> (v->data_dev_block_bits - SECTOR_SHIFT);
+   io->block = (bio->bi_sector - v->data_start) >> (v->data_dev_block_bits 
- SECTOR_SHIFT);
io->n_blocks = bio->bi_size >> v->data_dev_block_bits;
 
bio->bi_end_io = verity_end_io;
@@ -641,6 +641,7 @@ static void verity_dtr(struct dm_target 
  * 
  * 
  * 
+ * 
  * 
  * 
  * 
@@ -671,8 +672,8 @@ static int verity_ctr(struct dm_target *
goto bad;
}
 
-   if (argc != 10) {
-   ti->error = "Invalid argument count: exactly 10 arguments 
required";
+   if (argc != 11) {
+   ti->error = "Invalid argument count: exactly 11 arguments 
required";
r = -EINVAL;
goto bad;
}
@@ -718,6 +719,15 @@ static int verity_ctr(struct dm_target *
v->hash_dev_block_bits = ffs(num) - 1;
 
if (sscanf(argv[5], "%llu%c", _ll, ) != 1 ||
+   num_ll << (v->data_dev_block_bits - SECTOR_SHIFT) !=
+   (sector_t)num_ll << (v->data_dev_block_bits - SECTOR_SHIFT)) {
+   ti->error = "Invalid data start";
+   r = -EINVAL;
+   goto bad;
+   }
+   v->data_start = num_ll << (v->data_dev_block_bits - SECTOR_SHIFT);
+
+   if (sscanf(argv[6], "%llu%c", _ll, ) != 1 ||
num_ll << (v->data_dev_block_bits - SECTOR_SHIFT) !=
(sector_t)num_ll << (v->data_dev_block_bits - SECTOR_SHIFT)) {
ti->error = "Invalid data blocks";
@@ -732,7 +742,7 @@ static int verity_ctr(struct dm_target *
goto bad;
}
 
-   if (sscanf(argv[6], "%llu%c", _ll, ) != 1 ||
+   if (sscanf(argv[7], "%llu%c", _ll, ) != 1 ||
num_ll << (v->hash_dev_block_bits - SECTOR_SHIFT) !=
(sector_t)num_ll << (v->hash_dev_block_bits - SECTOR_SHIFT)) {
ti->error = "Invalid hash start";
@@ -741,7 +751,7 @@ static int verity_ctr(struct dm_target *
}
v->hash_start = num_ll;
 
-   v->alg_name = kstrdup(argv[7], GFP_KERNEL);
+   v->alg_name = kstrdup(argv[8], GFP_KERNEL);
if (!v->alg_name) {
ti->error = "Cannot allocate algorithm name";
r = -ENOMEM;
@@ -770,23 +780,23 @@ static int verity_ctr(struct dm_target *
r = -ENOMEM;
goto bad;
}
-   if (strlen(argv[8]) != v->digest_size * 2 ||
-   hex2bin(v->root_digest, argv[8], v->digest_size)) {
+   if (strlen(argv[9]) != v->digest_size * 2 ||
+   hex2bin(v->root_digest, argv[9], 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(argv[10], "-")) {
+   v->salt_size = strlen(argv[10]) / 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(argv[10]) != v->salt_size * 2 ||
+   hex2bin(v->salt, argv[10], v->salt_size)) {
ti->error = "Invalid salt";
r = -EINVAL;
goto bad;
--- a/Documentation/device-mapper/verity.txt2012-08-08 11:02:48.558883756 
-0700
+++ b/Documentation/device-mapper/verity.txt2012-08-08 11:13:01.259982498 
-0700
@@ -9,7 +9,7 @@ Construction Parameters
 ===
   
  
- 
+  
   
 
 
@@ -41,6 +41,10 @@ Constru

[PATCH] dm: verity support data device offset (Linux 3.4.7)

2012-08-08 Thread Wesley Miaw 
From: Wesley Miaw wm...@netflix.com

Add data device start block index to dm-verity target parameters to support
verity targets where the data does not begin at sector 0 of the block device.
Also fix the hash block index computation so it takes into account data offsets.

Signed-off-by: Wesley Miaw wm...@netflix.com
---
 Documentation/device-mapper/verity.txt |8 -
 drivers/md/dm-verity.c |   32 +++
 2 files changed, 27 insertions(+), 13 deletions(-)
--- a/drivers/md/dm-verity.c2012-08-07 16:03:03.778759000 -0700
+++ b/drivers/md/dm-verity.c2012-08-07 17:32:02.130176956 -0700
@@ -491,7 +491,7 @@ static int verity_map(struct dm_target *
io-bio = bio;
io-orig_bi_end_io = bio-bi_end_io;
io-orig_bi_private = bio-bi_private;
-   io-block = bio-bi_sector  (v-data_dev_block_bits - SECTOR_SHIFT);
+   io-block = (bio-bi_sector - v-data_start)  (v-data_dev_block_bits 
- SECTOR_SHIFT);
io-n_blocks = bio-bi_size  v-data_dev_block_bits;
 
bio-bi_end_io = verity_end_io;
@@ -641,6 +641,7 @@ static void verity_dtr(struct dm_target 
  * hash device
  * data block size
  * hash block size
+ * data start block
  * the number of data blocks
  * hash start block
  * algorithm
@@ -671,8 +672,8 @@ static int verity_ctr(struct dm_target *
goto bad;
}
 
-   if (argc != 10) {
-   ti-error = Invalid argument count: exactly 10 arguments 
required;
+   if (argc != 11) {
+   ti-error = Invalid argument count: exactly 11 arguments 
required;
r = -EINVAL;
goto bad;
}
@@ -718,6 +719,15 @@ static int verity_ctr(struct dm_target *
v-hash_dev_block_bits = ffs(num) - 1;
 
if (sscanf(argv[5], %llu%c, num_ll, dummy) != 1 ||
+   num_ll  (v-data_dev_block_bits - SECTOR_SHIFT) !=
+   (sector_t)num_ll  (v-data_dev_block_bits - SECTOR_SHIFT)) {
+   ti-error = Invalid data start;
+   r = -EINVAL;
+   goto bad;
+   }
+   v-data_start = num_ll  (v-data_dev_block_bits - SECTOR_SHIFT);
+
+   if (sscanf(argv[6], %llu%c, num_ll, dummy) != 1 ||
num_ll  (v-data_dev_block_bits - SECTOR_SHIFT) !=
(sector_t)num_ll  (v-data_dev_block_bits - SECTOR_SHIFT)) {
ti-error = Invalid data blocks;
@@ -732,7 +742,7 @@ static int verity_ctr(struct dm_target *
goto bad;
}
 
-   if (sscanf(argv[6], %llu%c, num_ll, dummy) != 1 ||
+   if (sscanf(argv[7], %llu%c, num_ll, dummy) != 1 ||
num_ll  (v-hash_dev_block_bits - SECTOR_SHIFT) !=
(sector_t)num_ll  (v-hash_dev_block_bits - SECTOR_SHIFT)) {
ti-error = Invalid hash start;
@@ -741,7 +751,7 @@ static int verity_ctr(struct dm_target *
}
v-hash_start = num_ll;
 
-   v-alg_name = kstrdup(argv[7], GFP_KERNEL);
+   v-alg_name = kstrdup(argv[8], GFP_KERNEL);
if (!v-alg_name) {
ti-error = Cannot allocate algorithm name;
r = -ENOMEM;
@@ -770,23 +780,23 @@ static int verity_ctr(struct dm_target *
r = -ENOMEM;
goto bad;
}
-   if (strlen(argv[8]) != v-digest_size * 2 ||
-   hex2bin(v-root_digest, argv[8], v-digest_size)) {
+   if (strlen(argv[9]) != v-digest_size * 2 ||
+   hex2bin(v-root_digest, argv[9], 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(argv[10], -)) {
+   v-salt_size = strlen(argv[10]) / 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(argv[10]) != v-salt_size * 2 ||
+   hex2bin(v-salt, argv[10], v-salt_size)) {
ti-error = Invalid salt;
r = -EINVAL;
goto bad;
--- a/Documentation/device-mapper/verity.txt2012-08-08 11:02:48.558883756 
-0700
+++ b/Documentation/device-mapper/verity.txt2012-08-08 11:13:01.259982498 
-0700
@@ -9,7 +9,7 @@ Construction Parameters
 ===
 version dev hash_dev
 data_block_size hash_block_size
-num_data_blocks hash_start_block
+data_start_block num_data_blocks hash_start_block
 algorithm digest salt
 
 version
@@ -41,6 +41,10 @@ Construction Parameters
 hash_block_size
 The size of a hash block in bytes.
 
+data_start_block
+This is the offset, in data_block_size-blocks, from the start

Re: [dm-devel] [PATCH] dm: verity support data device offset (Linux 3.4.7)

2012-08-08 Thread Wesley Miaw 
On Aug 8, 2012, at 1:31 PM, Milan Broz wrote:

 On 08/08/2012 08:46 PM, Mikulas Patocka wrote:
 
 The problem with the patch is that it changes interface to the userspace 
 tool. The userspace tool veritysetup already exists in recent cryptsetup 
 package, so we can't change the interface - you should change the patch so 
 that the starting data block is the last argument and the argument is 
 optional - so that it is compatible with the existing userspace too.
 
 yes. Please never change interface without at least increasing target version.
 
 I have to add userspace support as well to veritysetup and we need a way
 how to detect that option is supported by running kernel.


Understood. Thank you for the feedback. I will attempt a new patch version 
which addresses these issues. I also found that I did not correct the 
last-block boundary check so I will re-submit my patch with that as well.

I did modify veritysetup on my own so the format and verify commands will work 
with regular files on disk instead of having to mount through loop devices.
--
Wesley Miaw

signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: [dm-devel] [PATCH] dm: verity support data device offset (Linux 3.4.7)

2012-08-08 Thread Wesley Miaw 
On Aug 8, 2012, at 1:56 PM, Milan Broz wrote:

 On 08/08/2012 10:46 PM, Wesley Miaw wrote:
 
 I did modify veritysetup on my own so the format and verify commands will 
 work with regular files on disk instead of having to mount through loop 
 devices.
 
 Which veritysetup? In upstream (cryptsetup repository) it allocates loop 
 automatically.
 (And for userspace verification it doesn't need loop at all.)
 
 Anyway, please send a patch for userspace as well then ;-)

I grabbed cryptsetup from http://code.google.com/p/cryptsetup as what I read 
said that is the most recent. And then modified the code in there because the 
final block device images need to combine the file system, hash data, and some 
metadata into a single image and I don't want my users to need root privileges.

I can send a separate patch of those changes, but I'm not sure to where? Also 
to the LKML?

Thanks,
--
Wesley Miaw

signature.asc
Description: Message signed with OpenPGP using GPGMail

Re: [dm-devel] [PATCH v2 1/2] dm: verity support data device offset (Linux 3.4.7)

2012-08-08 Thread Wesley Miaw 
On Aug 8, 2012, at 1:31 PM, Milan Broz wrote:

 On 08/08/2012 08:46 PM, Mikulas Patocka wrote:
 
 The problem with the patch is that it changes interface to the userspace 
 tool. The userspace tool veritysetup already exists in recent cryptsetup 
 package, so we can't change the interface - you should change the patch so 
 that the starting data block is the last argument and the argument is 
 optional - so that it is compatible with the existing userspace too.
 
 yes. Please never change interface without at least increasing target version.
 
 I have to add userspace support as well to veritysetup and we need a way
 how to detect that option is supported by running kernel.

Apologies if the version increment is incorrect; I was not sure if the minor or 
patch number should be incremented. I assume the different version number is 
what would be used to detect if the data offset option is supported. Thanks.

From: Wesley Miaw wm...@netflix.com

Add data device start block index as optional dm-verity target parameters to
support verity targets where the data does not begin at sector 0 of the block
device.

Also fix the hash block index computations so they take into account any data
offset.

Signed-off-by: Wesley Miaw wm...@netflix.com
---
 Documentation/device-mapper/verity.txt |8 ++-
 drivers/md/dm-verity.c |   24 ++-
 2 files changed, 26 insertions(+), 6 deletions(-)
--- a/drivers/md/dm-verity.c2012-08-07 16:03:03.778759000 -0700
+++ b/drivers/md/dm-verity.c2012-08-08 17:04:16.344682266 -0700
@@ -477,7 +477,7 @@ static int verity_map(struct dm_target *
return -EIO;
}
 
-   if ((bio-bi_sector + bio_sectors(bio)) 
+   if ((bio-bi_sector - v-data_start + bio_sectors(bio)) 
(v-data_dev_block_bits - SECTOR_SHIFT)  v-data_blocks) {
DMERR_LIMIT(io out of range);
return -EIO;
@@ -491,7 +491,7 @@ static int verity_map(struct dm_target *
io-bio = bio;
io-orig_bi_end_io = bio-bi_end_io;
io-orig_bi_private = bio-bi_private;
-   io-block = bio-bi_sector  (v-data_dev_block_bits - SECTOR_SHIFT);
+   io-block = (bio-bi_sector - v-data_start)  (v-data_dev_block_bits 
- SECTOR_SHIFT);
io-n_blocks = bio-bi_size  v-data_dev_block_bits;
 
bio-bi_end_io = verity_end_io;
@@ -646,6 +646,7 @@ static void verity_dtr(struct dm_target 
  * algorithm
  * digest
  * salt  Hex string or - if no salt.
+ * data start block  Optional. The default is zero.
  */
 static int verity_ctr(struct dm_target *ti, unsigned argc, char **argv)
 {
@@ -671,8 +672,8 @@ static int verity_ctr(struct dm_target *
goto bad;
}
 
-   if (argc != 10) {
-   ti-error = Invalid argument count: exactly 10 arguments 
required;
+   if (argc != 10  argc != 11) {
+   ti-error = Invalid argument count: 10 or 11 arguments 
required;
r = -EINVAL;
goto bad;
}
@@ -793,6 +794,19 @@ static int verity_ctr(struct dm_target *
}
}
 
+   if (argc == 11) {
+   if (sscanf(argv[10], %llu%c, num_ll, dummy) != 1 ||
+   num_ll  (v-data_dev_block_bits - SECTOR_SHIFT) !=
+   (sector_t)num_ll  (v-data_dev_block_bits - 
SECTOR_SHIFT)) {
+   ti-error = Invalid data start;
+   r = -EINVAL;
+   goto bad;
+   }
+   v-data_start = num_ll  (v-data_dev_block_bits - 
SECTOR_SHIFT);
+   } else {
+   v-data_start = 0;
+   }
+
v-hash_per_block_bits =
fls((1  v-hash_dev_block_bits) / v-digest_size) - 1;
 
@@ -875,7 +889,7 @@ bad:
 
 static struct target_type verity_target = {
.name   = verity,
-   .version= {1, 0, 0},
+   .version= {1, 1, 0},
.module = THIS_MODULE,
.ctr= verity_ctr,
.dtr= verity_dtr,
--- a/Documentation/device-mapper/verity.txt2012-08-08 11:02:48.558883756 
-0700
+++ b/Documentation/device-mapper/verity.txt2012-08-08 16:50:04.114864090 
-0700
@@ -11,6 +11,7 @@ Construction Parameters
 data_block_size hash_block_size
 num_data_blocks hash_start_block
 algorithm digest salt
+[data_start_block]
 
 version
 This is the type of the on-disk hash format.
@@ -62,6 +63,10 @@ Construction Parameters
 salt
 The hexadecimal encoding of the salt value.
 
+data_start_block
+This is the offset, in data_block_size-blocks, from the start of data_dev
+to the first block of the data.
+
 Theory of operation
 ===
 
@@ -138,7 +143,8 @@ Set up a device:
   # dmsetup create vroot --readonly --table \
 0 2097152 verity 1 /dev/sda1 /dev/sda2 4096 4096 262144 1 sha256 \
 4392712ba01368efdf14b05c76f9e4df0d53664630b5d48632ed17a137f39076

Re: [dm-devel] [PATCH v2 2/2] dm: verity support data device offset (Linux 3.4.7)

2012-08-08 Thread Wesley Miaw 
On Aug 8, 2012, at 1:56 PM, Milan Broz wrote:

 On 08/08/2012 10:46 PM, Wesley Miaw wrote:
 
 I did modify veritysetup on my own so the format and verify commands will 
 work with regular files on disk instead of having to mount through loop 
 devices.
 
 Which veritysetup? In upstream (cryptsetup repository) it allocates loop 
 automatically.
 (And for userspace verification it doesn't need loop at all.)
 
 Anyway, please send a patch for userspace as well then ;-)

This isn't as polished because I pretty much just added support to do what I 
needed. I'm not sure if the LKML is the right place to post, so let me know if 
I should send this somewhere else.

Your previous email implied that veritysetup would need a way to determine if 
the data offset option is supported; I did not modify veritysetup to support 
this idea as I didn't need it.

Thanks.

From: Wesley Miaw wm...@netflix.com

Allow veritysetup format and verify commands to directly operate on regular
files instead of requiring mounts through loop devices.

Signed-off-by: Wesley Miaw wm...@netflix.com
---
 cryptsetup/lib/internal.h|1 
 cryptsetup/lib/libcryptsetup.h   |   22 
 cryptsetup/lib/libcryptsetup.sym |2 
 cryptsetup/lib/setup.c   |  133 -
 cryptsetup/lib/utils.c   |   12 ++
 cryptsetup/src/veritysetup.c |   23 +++--
 6 files changed, 183 insertions(+), 10 deletions(-)
--- a/cryptsetup/lib/internal.h 2012-08-08 17:11:20.366392301 -0700
+++ b/cryptsetup/lib/internal.h 2012-08-06 16:17:35.154719491 -0700
@@ -76,6 +76,7 @@ ssize_t read_blockwise(int fd, void *_bu
 ssize_t write_lseek_blockwise(int fd, char *buf, size_t count, off_t offset);
 int device_ready(struct crypt_device *cd, const char *device, int mode);
 int device_size(const char *device, uint64_t *size);
+int file_size(const char *filename, uint64_t *size);
 
 unsigned crypt_getpagesize(void);
 
--- a/cryptsetup/lib/libcryptsetup.h2012-08-08 17:11:20.375392929 -0700
+++ b/cryptsetup/lib/libcryptsetup.h2012-08-06 16:17:35.159720699 -0700
@@ -56,6 +57,19 @@ struct crypt_device; /* crypt device han
 int crypt_init(struct crypt_device **cd, const char *device);
 
 /**
+ * Initial crypt device handle from a file and check if provided file exists.
+ *
+ * @param cd Returns pointer to crypt device handle.
+ * @param filename Path to the backing file.
+ *
+ * @return @e 0 on success or negative errno value otherwise.
+ *
+ * @note Note that logging is not initialized here, possible messages uses
+ *   default log function.
+ */
+int crypt_initfile(struct crypt_device **cd, const char *filename);
+
+/**
  * Initialize crypt device handle from provided active device name,
  * and, optionally, from separate metadata (header) device
  * and check if provided device exists.
@@ -237,6 +251,15 @@ void crypt_set_password_verify(struct cr
 int crypt_set_data_device(struct crypt_device *cd, const char *device);
 
 /**
+ * Set data file
+ * For VERITY it is data file when hash device is separated.
+ *
+ * @param cd crypt device handle
+ * @param filename path to data file
+ */
+int crypt_set_data_file(struct crypt_device *cd, const char *device);
+
+/**
  * @defgroup rng Cryptsetup RNG
  *
  * @addtogroup rng
--- a/cryptsetup/lib/libcryptsetup.sym  2012-08-08 17:11:20.375392930 -0700
+++ b/cryptsetup/lib/libcryptsetup.sym  2012-08-06 16:17:35.160720941 -0700
@@ -1,6 +1,7 @@
 CRYPTSETUP_1.0 {
global:
crypt_init;
+   crypt_initfile;
crypt_init_by_name;
crypt_init_by_name_and_header;
crypt_set_log_callback;
@@ -13,6 +14,7 @@ CRYPTSETUP_1.0 {
crypt_set_password_verify;
crypt_set_uuid;
crypt_set_data_device;
+   crypt_set_data_file;
 
crypt_memory_lock;
crypt_format;
--- a/cryptsetup/lib/setup.c2012-08-08 17:11:20.428396640 -0700
+++ b/cryptsetup/lib/setup.c2012-08-06 16:17:35.192728669 -0700
@@ -25,6 +25,8 @@
 #include stdarg.h
 #include fcntl.h
 #include errno.h
+#include sys/types.h
+#include sys/stat.h
 
 #include libcryptsetup.h
 #include luks.h
@@ -585,6 +587,56 @@ bad:
return r;
 }
 
+int crypt_initfile(struct crypt_device **cd, const char *filename)
+{
+   struct crypt_device *h = NULL;
+   int fd;
+   struct stat st;
+   int r;
+
+   if (!cd)
+   return -EINVAL;
+
+   if (stat(filename, st)  0) {
+   log_err(NULL, _(File %s doesn't exist or access denied.\n), 
filename);
+   return -EINVAL;
+   }
+
+   log_dbg(Trying to open and write file %s., filename);
+   fd = open(filename, O_RDWR);
+   if (fd  0) {
+   log_err(NULL, _(Cannot open file %s for writeable access.\n), 
filename);
+   return -EINVAL;
+   }
+   close(fd);
+
+   log_dbg(Allocating crypt device %s context., filename);
+
+   if (!(h = malloc(sizeof

[PATCH] dm: verity support data device offset

2012-08-07 Thread Wesley Miaw 
I needed to add support for dm-verity with data that is offset into a block 
device. As part of this I found that the existing code did not compute the 
correct hash block index if the data_start might be non-zero. Here's a patch to 
add support for a data offset target parameter as well as a fix to the hash 
block index computation.

Patch and (hopefully proper) commit message below. Thanks.
--
Wesley Miaw



Add data device start block index to dm-verity target parameters to support 
verity targets where the data does not begin at sector 0 of the block device. 
Also fix the hash block index computation so it takes into account data offsets.

---

--- a/drivers/md/dm-verity.c2012-08-07 16:03:03.778759000 -0700
+++ b/drivers/md/dm-verity.c2012-08-07 17:30:56.914569414 -0700
@@ -491,7 +491,7 @@
io->bio = bio;
io->orig_bi_end_io = bio->bi_end_io;
io->orig_bi_private = bio->bi_private;
-   io->block = bio->bi_sector >> (v->data_dev_block_bits - SECTOR_SHIFT);
+   io->block = (bio->bi_sector - v->data_start)  >> 
(v->data_dev_block_bits - SECTOR_SHIFT);
io->n_blocks = bio->bi_size >> v->data_dev_block_bits;
 
bio->bi_end_io = verity_end_io;
@@ -641,6 +641,7 @@
  * 
  * 
  * 
+ * 
  * 
  * 
  * 
@@ -671,8 +672,8 @@
goto bad;
}
 
-   if (argc != 10) {
-   ti->error = "Invalid argument count: exactly 10 arguments 
required";
+   if (argc != 11) {
+   ti->error = "Invalid argument count: exactly 11 arguments 
required";
r = -EINVAL;
goto bad;
}
@@ -718,6 +719,15 @@
v->hash_dev_block_bits = ffs(num) - 1;
 
if (sscanf(argv[5], "%llu%c", _ll, ) != 1 ||
+   num_ll << (v->data_dev_block_bits - SECTOR_SHIFT) !=
+   (sector_t)num_ll << (v->data_dev_block_bits - SECTOR_SHIFT)) {
+   ti->error = "Invalid data start";
+   r = -EINVAL;
+   goto bad;
+   }
+   v->data_start = num_ll << (v->data_dev_block_bits - SECTOR_SHIFT);
+
+   if (sscanf(argv[6], "%llu%c", _ll, ) != 1 ||
num_ll << (v->data_dev_block_bits - SECTOR_SHIFT) !=
(sector_t)num_ll << (v->data_dev_block_bits - SECTOR_SHIFT)) {
ti->error = "Invalid data blocks";
@@ -732,7 +742,7 @@
goto bad;
}
 
-   if (sscanf(argv[6], "%llu%c", _ll, ) != 1 ||
+   if (sscanf(argv[7], "%llu%c", _ll, ) != 1 ||
num_ll << (v->hash_dev_block_bits - SECTOR_SHIFT) !=
(sector_t)num_ll << (v->hash_dev_block_bits - SECTOR_SHIFT)) {
ti->error = "Invalid hash start";
@@ -741,7 +751,7 @@
}
v->hash_start = num_ll;
 
-   v->alg_name = kstrdup(argv[7], GFP_KERNEL);
+   v->alg_name = kstrdup(argv[8], GFP_KERNEL);
if (!v->alg_name) {
ti->error = "Cannot allocate algorithm name";
r = -ENOMEM;
@@ -770,23 +780,23 @@
r = -ENOMEM;
goto bad;
}
-   if (strlen(argv[8]) != v->digest_size * 2 ||
-   hex2bin(v->root_digest, argv[8], v->digest_size)) {
+   if (strlen(argv[9]) != v->digest_size * 2 ||
+   hex2bin(v->root_digest, argv[9], 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(argv[10], "-")) {
+   v->salt_size = strlen(argv[10]) / 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(argv[10]) != v->salt_size * 2 ||
+   hex2bin(v->salt, argv[10], v->salt_size)) {
ti->error = "Invalid salt";
r = -EINVAL;
goto bad;



signature.asc
Description: Message signed with OpenPGP using GPGMail

[PATCH] dm: verity support data device offset

2012-08-07 Thread Wesley Miaw 
I needed to add support for dm-verity with data that is offset into a block 
device. As part of this I found that the existing code did not compute the 
correct hash block index if the data_start might be non-zero. Here's a patch to 
add support for a data offset target parameter as well as a fix to the hash 
block index computation.

Patch and (hopefully proper) commit message below. Thanks.
--
Wesley Miaw



Add data device start block index to dm-verity target parameters to support 
verity targets where the data does not begin at sector 0 of the block device. 
Also fix the hash block index computation so it takes into account data offsets.

---

--- a/drivers/md/dm-verity.c2012-08-07 16:03:03.778759000 -0700
+++ b/drivers/md/dm-verity.c2012-08-07 17:30:56.914569414 -0700
@@ -491,7 +491,7 @@
io-bio = bio;
io-orig_bi_end_io = bio-bi_end_io;
io-orig_bi_private = bio-bi_private;
-   io-block = bio-bi_sector  (v-data_dev_block_bits - SECTOR_SHIFT);
+   io-block = (bio-bi_sector - v-data_start)   
(v-data_dev_block_bits - SECTOR_SHIFT);
io-n_blocks = bio-bi_size  v-data_dev_block_bits;
 
bio-bi_end_io = verity_end_io;
@@ -641,6 +641,7 @@
  * hash device
  * data block size
  * hash block size
+ * data start block
  * the number of data blocks
  * hash start block
  * algorithm
@@ -671,8 +672,8 @@
goto bad;
}
 
-   if (argc != 10) {
-   ti-error = Invalid argument count: exactly 10 arguments 
required;
+   if (argc != 11) {
+   ti-error = Invalid argument count: exactly 11 arguments 
required;
r = -EINVAL;
goto bad;
}
@@ -718,6 +719,15 @@
v-hash_dev_block_bits = ffs(num) - 1;
 
if (sscanf(argv[5], %llu%c, num_ll, dummy) != 1 ||
+   num_ll  (v-data_dev_block_bits - SECTOR_SHIFT) !=
+   (sector_t)num_ll  (v-data_dev_block_bits - SECTOR_SHIFT)) {
+   ti-error = Invalid data start;
+   r = -EINVAL;
+   goto bad;
+   }
+   v-data_start = num_ll  (v-data_dev_block_bits - SECTOR_SHIFT);
+
+   if (sscanf(argv[6], %llu%c, num_ll, dummy) != 1 ||
num_ll  (v-data_dev_block_bits - SECTOR_SHIFT) !=
(sector_t)num_ll  (v-data_dev_block_bits - SECTOR_SHIFT)) {
ti-error = Invalid data blocks;
@@ -732,7 +742,7 @@
goto bad;
}
 
-   if (sscanf(argv[6], %llu%c, num_ll, dummy) != 1 ||
+   if (sscanf(argv[7], %llu%c, num_ll, dummy) != 1 ||
num_ll  (v-hash_dev_block_bits - SECTOR_SHIFT) !=
(sector_t)num_ll  (v-hash_dev_block_bits - SECTOR_SHIFT)) {
ti-error = Invalid hash start;
@@ -741,7 +751,7 @@
}
v-hash_start = num_ll;
 
-   v-alg_name = kstrdup(argv[7], GFP_KERNEL);
+   v-alg_name = kstrdup(argv[8], GFP_KERNEL);
if (!v-alg_name) {
ti-error = Cannot allocate algorithm name;
r = -ENOMEM;
@@ -770,23 +780,23 @@
r = -ENOMEM;
goto bad;
}
-   if (strlen(argv[8]) != v-digest_size * 2 ||
-   hex2bin(v-root_digest, argv[8], v-digest_size)) {
+   if (strlen(argv[9]) != v-digest_size * 2 ||
+   hex2bin(v-root_digest, argv[9], 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(argv[10], -)) {
+   v-salt_size = strlen(argv[10]) / 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(argv[10]) != v-salt_size * 2 ||
+   hex2bin(v-salt, argv[10], v-salt_size)) {
ti-error = Invalid salt;
r = -EINVAL;
goto bad;



signature.asc
Description: Message signed with OpenPGP using GPGMail