[PATCH 9/9] dm-integrity: recalculate checksums

[PATCH 9/9] dm-integrity: recalculate checksums

[Mikulas Patocka]

[-- Attachment #1: dm-integrity-recalc.patch --]
[-- Type: text/plain, Size: 9826 bytes --]

When using external metadata device and internal hash, recalculate the
checksums when the device is created - so that dm-integrity doesn't have
to overwrite the device. The superblock stores the last position when the
recalculation ended, so that it is properly restarted.

Integrity tags that haven't been recalculated yet are ignored.

This patch also increases the target version to 1.2.0.

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

---
 Documentation/device-mapper/dm-integrity.txt |    4 
 drivers/md/dm-integrity.c                    |  175 ++++++++++++++++++++++++++-
 2 files changed, 177 insertions(+), 2 deletions(-)

Index: linux-2.6/drivers/md/dm-integrity.c
===================================================================
--- linux-2.6.orig/drivers/md/dm-integrity.c	2018-06-06 17:13:37.000000000 +0200
+++ linux-2.6/drivers/md/dm-integrity.c	2018-06-06 17:14:23.000000000 +0200
@@ -31,6 +31,8 @@
 #define MIN_LOG2_INTERLEAVE_SECTORS	3
 #define MAX_LOG2_INTERLEAVE_SECTORS	31
 #define METADATA_WORKQUEUE_MAX_ACTIVE	16
+#define RECALC_SECTORS			8192
+#define RECALC_WRITE_SUPER		16
 
 /*
  * Warning - DEBUG_PRINT prints security-sensitive data to the log,
@@ -58,9 +60,12 @@ struct superblock {
 	__u64 provided_data_sectors;	/* userspace uses this value */
 	__u32 flags;
 	__u8 log2_sectors_per_block;
+	__u8 pad[3];
+	__u64 recalc_sector;
 };
 
 #define SB_FLAG_HAVE_JOURNAL_MAC	0x1
+#define SB_FLAG_RECALCULATING		0x2
 
 #define	JOURNAL_ENTRY_ROUNDUP		8
 
@@ -214,6 +219,11 @@ struct dm_integrity_c {
 	struct workqueue_struct *writer_wq;
 	struct work_struct writer_work;
 
+	struct workqueue_struct *recalc_wq;
+	struct work_struct recalc_work;
+	u8 *recalc_buffer;
+	u8 *recalc_tags;
+
 	struct bio_list flush_bio_list;
 
 	unsigned long autocommit_jiffies;
@@ -417,7 +427,7 @@ static void wraparound_section(struct dm
 
 static void sb_set_version(struct dm_integrity_c *ic)
 {
-	if (ic->meta_dev)
+	if (ic->meta_dev || ic->sb->flags & cpu_to_le32(SB_FLAG_RECALCULATING))
 		ic->sb->version = SB_VERSION_2;
 	else
 		ic->sb->version = SB_VERSION_1;
@@ -1776,9 +1786,14 @@ offload_to_thread:
 
 	if (need_sync_io) {
 		wait_for_completion_io(&read_comp);
+		if (unlikely(ic->recalc_wq != NULL) &&
+		    ic->sb->flags & cpu_to_le32(SB_FLAG_RECALCULATING) &&
+		    dio->range.logical_sector + dio->range.n_sectors > le64_to_cpu(ic->sb->recalc_sector))
+			goto skip_check;
 		if (likely(!bio->bi_status))
 			integrity_metadata(&dio->work);
 		else
+skip_check:
 			dec_in_flight(dio);
 
 	} else {
@@ -2078,6 +2093,108 @@ static void integrity_writer(struct work
 	spin_unlock_irq(&ic->endio_wait.lock);
 }
 
+static void recalc_write_super(struct dm_integrity_c *ic)
+{
+	int r;
+
+	dm_integrity_flush_buffers(ic);
+	if (dm_integrity_failed(ic))
+		return;
+
+	sb_set_version(ic);
+	r = sync_rw_sb(ic, REQ_OP_WRITE, 0);
+	if (unlikely(r))
+		dm_integrity_io_error(ic, "writing superblock", r);
+}
+
+static void integrity_recalc(struct work_struct *w)
+{
+	struct dm_integrity_c *ic = container_of(w, struct dm_integrity_c, recalc_work);
+	struct dm_integrity_range range;
+	struct dm_io_request io_req;
+	struct dm_io_region io_loc;
+	sector_t area, offset;
+	sector_t metadata_block;
+	unsigned metadata_offset;
+	__u8 *t;
+	unsigned i;
+	int r;
+	unsigned super_counter = 0;
+
+	spin_lock_irq(&ic->endio_wait.lock);
+
+next_chunk:
+
+	if (unlikely(READ_ONCE(ic->suspending)))
+		goto unlock_ret;
+
+	range.logical_sector = le64_to_cpu(ic->sb->recalc_sector);
+	if (unlikely(range.logical_sector >= ic->provided_data_sectors))
+		goto unlock_ret;
+
+	get_area_and_offset(ic, range.logical_sector, &area, &offset);
+	range.n_sectors = min((sector_t)RECALC_SECTORS, ic->provided_data_sectors - range.logical_sector);
+	if (!ic->meta_dev)
+		range.n_sectors = min(range.n_sectors, (1U << ic->sb->log2_interleave_sectors) - (unsigned)offset);
+
+	if (unlikely(!add_new_range(ic, &range, true)))
+		wait_and_add_new_range(ic, &range);
+
+	spin_unlock_irq(&ic->endio_wait.lock);
+
+	if (unlikely(++super_counter == RECALC_WRITE_SUPER)) {
+		recalc_write_super(ic);
+		super_counter = 0;
+	}
+
+	if (unlikely(dm_integrity_failed(ic)))
+		goto err;
+
+	io_req.bi_op = REQ_OP_READ;
+	io_req.bi_op_flags = 0;
+	io_req.mem.type = DM_IO_VMA;
+	io_req.mem.ptr.addr = ic->recalc_buffer;
+	io_req.notify.fn = NULL;
+	io_req.client = ic->io;
+	io_loc.bdev = ic->dev->bdev;
+	io_loc.sector = get_data_sector(ic, area, offset);
+	io_loc.count = range.n_sectors;
+
+	r = dm_io(&io_req, 1, &io_loc, NULL);
+	if (unlikely(r)) {
+		dm_integrity_io_error(ic, "reading data", r);
+		goto err;
+	}
+
+	t = ic->recalc_tags;
+	for (i = 0; i < range.n_sectors; i += ic->sectors_per_block) {
+		integrity_sector_checksum(ic, range.logical_sector + i, ic->recalc_buffer + (i << SECTOR_SHIFT), t);
+		t += ic->tag_size;
+	}
+
+	metadata_block = get_metadata_sector_and_offset(ic, area, offset, &metadata_offset);
+
+	r = dm_integrity_rw_tag(ic, ic->recalc_tags, &metadata_block, &metadata_offset, t - ic->recalc_tags, TAG_WRITE);
+	if (unlikely(r)) {
+		dm_integrity_io_error(ic, "writing tags", r);
+		goto err;
+	}
+
+	spin_lock_irq(&ic->endio_wait.lock);
+	remove_range_unlocked(ic, &range);
+	ic->sb->recalc_sector = cpu_to_le64(range.logical_sector + range.n_sectors);
+	goto next_chunk;
+
+err:
+	remove_range(ic, &range);
+	return;
+
+unlock_ret:
+	spin_unlock_irq(&ic->endio_wait.lock);
+
+	recalc_write_super(ic);
+}
+
 static void init_journal(struct dm_integrity_c *ic, unsigned start_section,
 			 unsigned n_sections, unsigned char commit_seq)
 {
@@ -2282,6 +2399,9 @@ static void dm_integrity_postsuspend(str
 
 	WRITE_ONCE(ic->suspending, 1);
 
+	if (ic->recalc_wq)
+		drain_workqueue(ic->recalc_wq);
+
 	queue_work(ic->commit_wq, &ic->commit_work);
 	drain_workqueue(ic->commit_wq);
 
@@ -2304,6 +2424,16 @@ static void dm_integrity_resume(struct d
 	struct dm_integrity_c *ic = (struct dm_integrity_c *)ti->private;
 
 	replay_journal(ic);
+
+	if (ic->recalc_wq && ic->sb->flags & cpu_to_le32(SB_FLAG_RECALCULATING)) {
+		__u64 recalc_pos = le64_to_cpu(ic->sb->recalc_sector);
+		if (recalc_pos < ic->provided_data_sectors) {
+			queue_work(ic->recalc_wq, &ic->recalc_work);
+		} else if (recalc_pos > ic->provided_data_sectors) {
+			ic->sb->recalc_sector = cpu_to_le64(ic->provided_data_sectors);
+			recalc_write_super(ic);
+		}
+	}
 }
 
 static void dm_integrity_status(struct dm_target *ti, status_type_t type,
@@ -2941,6 +3071,7 @@ static int dm_integrity_ctr(struct dm_ta
 	bool should_write_sb;
 	__u64 threshold;
 	unsigned long long start;
+	bool recalculate = false;
 
 #define DIRECT_ARGUMENTS	4
 
@@ -3060,6 +3191,8 @@ static int dm_integrity_ctr(struct dm_ta
 					    "Invalid journal_mac argument");
 			if (r)
 				goto bad;
+		} else if (!strcmp(opt_string, "recalculate")) {
+			recalculate = true;
 		} else {
 			r = -EINVAL;
 			ti->error = "Invalid argument";
@@ -3289,6 +3422,38 @@ try_smaller_buffer:
 		    (unsigned long long)ic->provided_data_sectors);
 	DEBUG_print("	log2_buffer_sectors %u\n", ic->log2_buffer_sectors);
 
+	if (recalculate && !(ic->sb->flags & cpu_to_le32(SB_FLAG_RECALCULATING))) {
+		ic->sb->flags |= cpu_to_le32(SB_FLAG_RECALCULATING);
+		ic->sb->recalc_sector = cpu_to_le64(0);
+	}
+
+	if (ic->sb->flags & cpu_to_le32(SB_FLAG_RECALCULATING)) {
+		if (!ic->internal_hash) {
+			r = -EINVAL;
+			ti->error = "Recalculate is only valid with internal hash";
+			goto bad;
+		}
+		ic->recalc_wq = alloc_workqueue("dm-intergrity-recalc", WQ_MEM_RECLAIM, 1);
+		if (!ic->recalc_wq ) {
+			ti->error = "Cannot allocate workqueue";
+			r = -ENOMEM;
+			goto bad;
+		}
+		INIT_WORK(&ic->recalc_work, integrity_recalc);
+		ic->recalc_buffer = vmalloc(RECALC_SECTORS << SECTOR_SHIFT);
+		if (!ic->recalc_buffer) {
+			ti->error = "Cannot allocate buffer for recalculating";
+			r = -ENOMEM;
+			goto bad;
+		}
+		ic->recalc_tags = kvmalloc((RECALC_SECTORS >> ic->sb->log2_sectors_per_block) * ic->tag_size, GFP_KERNEL);
+		if (!ic->recalc_tags) {
+			ti->error = "Cannot allocate tags for recalculating";
+			r = -ENOMEM;
+			goto bad;
+		}
+	}
+
 	ic->bufio = dm_bufio_client_create(ic->meta_dev ? ic->meta_dev->bdev : ic->dev->bdev,
 			1U << (SECTOR_SHIFT + ic->log2_buffer_sectors), 1, 0, NULL, NULL);
 	if (IS_ERR(ic->bufio)) {
@@ -3355,6 +3520,12 @@ static void dm_integrity_dtr(struct dm_t
 		destroy_workqueue(ic->commit_wq);
 	if (ic->writer_wq)
 		destroy_workqueue(ic->writer_wq);
+	if (ic->recalc_wq)
+		destroy_workqueue(ic->recalc_wq);
+	if (ic->recalc_buffer)
+		vfree(ic->recalc_buffer);
+	if (ic->recalc_tags)
+		kvfree(ic->recalc_tags);
 	if (ic->bufio)
 		dm_bufio_client_destroy(ic->bufio);
 	mempool_destroy(ic->journal_io_mempool);
@@ -3404,7 +3575,7 @@ static void dm_integrity_dtr(struct dm_t
 
 static struct target_type integrity_target = {
 	.name			= "integrity",
-	.version		= {1, 1, 0},
+	.version		= {1, 2, 0},
 	.module			= THIS_MODULE,
 	.features		= DM_TARGET_SINGLETON | DM_TARGET_INTEGRITY,
 	.ctr			= dm_integrity_ctr,
Index: linux-2.6/Documentation/device-mapper/dm-integrity.txt
===================================================================
--- linux-2.6.orig/Documentation/device-mapper/dm-integrity.txt	2018-06-06 17:13:37.000000000 +0200
+++ linux-2.6/Documentation/device-mapper/dm-integrity.txt	2018-06-06 17:13:37.000000000 +0200
@@ -113,6 +113,10 @@ internal_hash:algorithm(:key)	(the key i
 	from an upper layer target, such as dm-crypt. The upper layer
 	target should check the validity of the integrity tags.
 
+recalculate
+	Recalculate the integrity tags automatically. It is only valid
+	when using internal hash.
+
 journal_crypt:algorithm(:key)	(the key is optional)
 	Encrypt the journal using given algorithm to make sure that the
 	attacker can't read the journal. You can use a block cipher here

Re: [PATCH 9/9] dm-integrity: recalculate checksums

[Milan Broz]

Hi Mikulas,

I have some questions for the last dm-integrity recalculation patch (copied below).

- The "recalculate" dm table attribute is never visible in "dmsetup table",
it is parsed only on table dm-ioctl input.
I think if sb flags has SB_FLAG_RECALCULATING, it should appear there.

- So, SB_FLAG_RECALCULATING bit is persistent and never disappears?
Is it intentional, that after recalculation is done, it is not cleared?
(Is it because or later resize recalc the rest of block?)

- What about monitoring progress - for now, we can only read superblock
and parse recalc_sector (not sure if you want this). What about status line option?

- How can I stop/restart recalculation (activate device without it / clear sb flag)?
Shouldn't this be connected to the presence of "recalculate" flag in mapping table?

Thanks,
Milan

> Subject: [PATCH 9/9] dm-integrity: recalculate checksums
> Date: Wed, 06 Jun 2018 17:33:16 +0200
> From: Mikulas Patocka <mpatocka@redhat.com>
> To: Mike Snitzer <msnitzer@redhat.com>, Milan Broz <mbroz@redhat.com>
> CC: dm-devel@redhat.com, Mikulas Patocka <mpatocka@redhat.com>
> 
> When using external metadata device and internal hash, recalculate the
> checksums when the device is created - so that dm-integrity doesn't have
> to overwrite the device. The superblock stores the last position when the
> recalculation ended, so that it is properly restarted.
> 
> Integrity tags that haven't been recalculated yet are ignored.
> 
> This patch also increases the target version to 1.2.0.
> 
> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
> 
> ---
>  Documentation/device-mapper/dm-integrity.txt |    4 
>  drivers/md/dm-integrity.c                    |  175 ++++++++++++++++++++++++++-
>  2 files changed, 177 insertions(+), 2 deletions(-)
> 
> Index: linux-2.6/drivers/md/dm-integrity.c
> ===================================================================
> --- linux-2.6.orig/drivers/md/dm-integrity.c	2018-06-06 17:13:37.000000000 +0200
> +++ linux-2.6/drivers/md/dm-integrity.c	2018-06-06 17:14:23.000000000 +0200
> @@ -31,6 +31,8 @@
>  #define MIN_LOG2_INTERLEAVE_SECTORS	3
>  #define MAX_LOG2_INTERLEAVE_SECTORS	31
>  #define METADATA_WORKQUEUE_MAX_ACTIVE	16
> +#define RECALC_SECTORS			8192
> +#define RECALC_WRITE_SUPER		16
>  
>  /*
>   * Warning - DEBUG_PRINT prints security-sensitive data to the log,
> @@ -58,9 +60,12 @@ struct superblock {
>  	__u64 provided_data_sectors;	/* userspace uses this value */
>  	__u32 flags;
>  	__u8 log2_sectors_per_block;
> +	__u8 pad[3];
> +	__u64 recalc_sector;
>  };
>  
>  #define SB_FLAG_HAVE_JOURNAL_MAC	0x1
> +#define  		0x2
>  
>  #define	JOURNAL_ENTRY_ROUNDUP		8
>  
> @@ -214,6 +219,11 @@ struct dm_integrity_c {
>  	struct workqueue_struct *writer_wq;
>  	struct work_struct writer_work;
>  
> +	struct workqueue_struct *recalc_wq;
> +	struct work_struct recalc_work;
> +	u8 *recalc_buffer;
> +	u8 *recalc_tags;
> +
>  	struct bio_list flush_bio_list;
>  
>  	unsigned long autocommit_jiffies;
> @@ -417,7 +427,7 @@ static void wraparound_section(struct dm
>  
>  static void sb_set_version(struct dm_integrity_c *ic)
>  {
> -	if (ic->meta_dev)
> +	if (ic->meta_dev || ic->sb->flags & cpu_to_le32(SB_FLAG_RECALCULATING))
>  		ic->sb->version = SB_VERSION_2;
>  	else
>  		ic->sb->version = SB_VERSION_1;
> @@ -1776,9 +1786,14 @@ offload_to_thread:
>  
>  	if (need_sync_io) {
>  		wait_for_completion_io(&read_comp);
> +		if (unlikely(ic->recalc_wq != NULL) &&
> +		    ic->sb->flags & cpu_to_le32(SB_FLAG_RECALCULATING) &&
> +		    dio->range.logical_sector + dio->range.n_sectors > le64_to_cpu(ic->sb->recalc_sector))
> +			goto skip_check;
>  		if (likely(!bio->bi_status))
>  			integrity_metadata(&dio->work);
>  		else
> +skip_check:
>  			dec_in_flight(dio);
>  
>  	} else {
> @@ -2078,6 +2093,108 @@ static void integrity_writer(struct work
>  	spin_unlock_irq(&ic->endio_wait.lock);
>  }
>  
> +static void recalc_write_super(struct dm_integrity_c *ic)
> +{
> +	int r;
> +
> +	dm_integrity_flush_buffers(ic);
> +	if (dm_integrity_failed(ic))
> +		return;
> +
> +	sb_set_version(ic);
> +	r = sync_rw_sb(ic, REQ_OP_WRITE, 0);
> +	if (unlikely(r))
> +		dm_integrity_io_error(ic, "writing superblock", r);
> +}
> +
> +static void integrity_recalc(struct work_struct *w)
> +{
> +	struct dm_integrity_c *ic = container_of(w, struct dm_integrity_c, recalc_work);
> +	struct dm_integrity_range range;
> +	struct dm_io_request io_req;
> +	struct dm_io_region io_loc;
> +	sector_t area, offset;
> +	sector_t metadata_block;
> +	unsigned metadata_offset;
> +	__u8 *t;
> +	unsigned i;
> +	int r;
> +	unsigned super_counter = 0;
> +
> +	spin_lock_irq(&ic->endio_wait.lock);
> +
> +next_chunk:
> +
> +	if (unlikely(READ_ONCE(ic->suspending)))
> +		goto unlock_ret;
> +
> +	range.logical_sector = le64_to_cpu(ic->sb->recalc_sector);
> +	if (unlikely(range.logical_sector >= ic->provided_data_sectors))
> +		goto unlock_ret;
> +
> +	get_area_and_offset(ic, range.logical_sector, &area, &offset);
> +	range.n_sectors = min((sector_t)RECALC_SECTORS, ic->provided_data_sectors - range.logical_sector);
> +	if (!ic->meta_dev)
> +		range.n_sectors = min(range.n_sectors, (1U << ic->sb->log2_interleave_sectors) - (unsigned)offset);
> +
> +	if (unlikely(!add_new_range(ic, &range, true)))
> +		wait_and_add_new_range(ic, &range);
> +
> +	spin_unlock_irq(&ic->endio_wait.lock);
> +
> +	if (unlikely(++super_counter == RECALC_WRITE_SUPER)) {
> +		recalc_write_super(ic);
> +		super_counter = 0;
> +	}
> +
> +	if (unlikely(dm_integrity_failed(ic)))
> +		goto err;
> +
> +	io_req.bi_op = REQ_OP_READ;
> +	io_req.bi_op_flags = 0;
> +	io_req.mem.type = DM_IO_VMA;
> +	io_req.mem.ptr.addr = ic->recalc_buffer;
> +	io_req.notify.fn = NULL;
> +	io_req.client = ic->io;
> +	io_loc.bdev = ic->dev->bdev;
> +	io_loc.sector = get_data_sector(ic, area, offset);
> +	io_loc.count = range.n_sectors;
> +
> +	r = dm_io(&io_req, 1, &io_loc, NULL);
> +	if (unlikely(r)) {
> +		dm_integrity_io_error(ic, "reading data", r);
> +		goto err;
> +	}
> +
> +	t = ic->recalc_tags;
> +	for (i = 0; i < range.n_sectors; i += ic->sectors_per_block) {
> +		integrity_sector_checksum(ic, range.logical_sector + i, ic->recalc_buffer + (i << SECTOR_SHIFT), t);
> +		t += ic->tag_size;
> +	}
> +
> +	metadata_block = get_metadata_sector_and_offset(ic, area, offset, &metadata_offset);
> +
> +	r = dm_integrity_rw_tag(ic, ic->recalc_tags, &metadata_block, &metadata_offset, t - ic->recalc_tags, TAG_WRITE);
> +	if (unlikely(r)) {
> +		dm_integrity_io_error(ic, "writing tags", r);
> +		goto err;
> +	}
> +
> +	spin_lock_irq(&ic->endio_wait.lock);
> +	remove_range_unlocked(ic, &range);
> +	ic->sb->recalc_sector = cpu_to_le64(range.logical_sector + range.n_sectors);
> +	goto next_chunk;
> +
> +err:
> +	remove_range(ic, &range);
> +	return;
> +
> +unlock_ret:
> +	spin_unlock_irq(&ic->endio_wait.lock);
> +
> +	recalc_write_super(ic);
> +}
> +
>  static void init_journal(struct dm_integrity_c *ic, unsigned start_section,
>  			 unsigned n_sections, unsigned char commit_seq)
>  {
> @@ -2282,6 +2399,9 @@ static void dm_integrity_postsuspend(str
>  
>  	WRITE_ONCE(ic->suspending, 1);
>  
> +	if (ic->recalc_wq)
> +		drain_workqueue(ic->recalc_wq);
> +
>  	queue_work(ic->commit_wq, &ic->commit_work);
>  	drain_workqueue(ic->commit_wq);
>  
> @@ -2304,6 +2424,16 @@ static void dm_integrity_resume(struct d
>  	struct dm_integrity_c *ic = (struct dm_integrity_c *)ti->private;
>  
>  	replay_journal(ic);
> +
> +	if (ic->recalc_wq && ic->sb->flags & cpu_to_le32(SB_FLAG_RECALCULATING)) {
> +		__u64 recalc_pos = le64_to_cpu(ic->sb->recalc_sector);
> +		if (recalc_pos < ic->provided_data_sectors) {
> +			queue_work(ic->recalc_wq, &ic->recalc_work);
> +		} else if (recalc_pos > ic->provided_data_sectors) {
> +			ic->sb->recalc_sector = cpu_to_le64(ic->provided_data_sectors);
> +			recalc_write_super(ic);
> +		}
> +	}
>  }
>  
>  static void dm_integrity_status(struct dm_target *ti, status_type_t type,
> @@ -2941,6 +3071,7 @@ static int dm_integrity_ctr(struct dm_ta
>  	bool should_write_sb;
>  	__u64 threshold;
>  	unsigned long long start;
> +	bool recalculate = false;
>  
>  #define DIRECT_ARGUMENTS	4
>  
> @@ -3060,6 +3191,8 @@ static int dm_integrity_ctr(struct dm_ta
>  					    "Invalid journal_mac argument");
>  			if (r)
>  				goto bad;
> +		} else if (!strcmp(opt_string, "recalculate")) {
> +			recalculate = true;
>  		} else {
>  			r = -EINVAL;
>  			ti->error = "Invalid argument";
> @@ -3289,6 +3422,38 @@ try_smaller_buffer:
>  		    (unsigned long long)ic->provided_data_sectors);
>  	DEBUG_print("	log2_buffer_sectors %u\n", ic->log2_buffer_sectors);
>  
> +	if (recalculate && !(ic->sb->flags & cpu_to_le32(SB_FLAG_RECALCULATING))) {
> +		ic->sb->flags |= cpu_to_le32(SB_FLAG_RECALCULATING);
> +		ic->sb->recalc_sector = cpu_to_le64(0);
> +	}
> +
> +	if (ic->sb->flags & cpu_to_le32(SB_FLAG_RECALCULATING)) {
> +		if (!ic->internal_hash) {
> +			r = -EINVAL;
> +			ti->error = "Recalculate is only valid with internal hash";
> +			goto bad;
> +		}
> +		ic->recalc_wq = alloc_workqueue("dm-intergrity-recalc", WQ_MEM_RECLAIM, 1);
> +		if (!ic->recalc_wq ) {
> +			ti->error = "Cannot allocate workqueue";
> +			r = -ENOMEM;
> +			goto bad;
> +		}
> +		INIT_WORK(&ic->recalc_work, integrity_recalc);
> +		ic->recalc_buffer = vmalloc(RECALC_SECTORS << SECTOR_SHIFT);
> +		if (!ic->recalc_buffer) {
> +			ti->error = "Cannot allocate buffer for recalculating";
> +			r = -ENOMEM;
> +			goto bad;
> +		}
> +		ic->recalc_tags = kvmalloc((RECALC_SECTORS >> ic->sb->log2_sectors_per_block) * ic->tag_size, GFP_KERNEL);
> +		if (!ic->recalc_tags) {
> +			ti->error = "Cannot allocate tags for recalculating";
> +			r = -ENOMEM;
> +			goto bad;
> +		}
> +	}
> +
>  	ic->bufio = dm_bufio_client_create(ic->meta_dev ? ic->meta_dev->bdev : ic->dev->bdev,
>  			1U << (SECTOR_SHIFT + ic->log2_buffer_sectors), 1, 0, NULL, NULL);
>  	if (IS_ERR(ic->bufio)) {
> @@ -3355,6 +3520,12 @@ static void dm_integrity_dtr(struct dm_t
>  		destroy_workqueue(ic->commit_wq);
>  	if (ic->writer_wq)
>  		destroy_workqueue(ic->writer_wq);
> +	if (ic->recalc_wq)
> +		destroy_workqueue(ic->recalc_wq);
> +	if (ic->recalc_buffer)
> +		vfree(ic->recalc_buffer);
> +	if (ic->recalc_tags)
> +		kvfree(ic->recalc_tags);
>  	if (ic->bufio)
>  		dm_bufio_client_destroy(ic->bufio);
>  	mempool_destroy(ic->journal_io_mempool);
> @@ -3404,7 +3575,7 @@ static void dm_integrity_dtr(struct dm_t
>  
>  static struct target_type integrity_target = {
>  	.name			= "integrity",
> -	.version		= {1, 1, 0},
> +	.version		= {1, 2, 0},
>  	.module			= THIS_MODULE,
>  	.features		= DM_TARGET_SINGLETON | DM_TARGET_INTEGRITY,
>  	.ctr			= dm_integrity_ctr,
> Index: linux-2.6/Documentation/device-mapper/dm-integrity.txt
> ===================================================================
> --- linux-2.6.orig/Documentation/device-mapper/dm-integrity.txt	2018-06-06 17:13:37.000000000 +0200
> +++ linux-2.6/Documentation/device-mapper/dm-integrity.txt	2018-06-06 17:13:37.000000000 +0200
> @@ -113,6 +113,10 @@ internal_hash:algorithm(:key)	(the key i
>  	from an upper layer target, such as dm-crypt. The upper layer
>  	target should check the validity of the integrity tags.
>  
> +recalculate
> +	Recalculate the integrity tags automatically. It is only valid
> +	when using internal hash.
> +
>  journal_crypt:algorithm(:key)	(the key is optional)
>  	Encrypt the journal using given algorithm to make sure that the
>  	attacker can't read the journal. You can use a block cipher here
> 

Re: [PATCH 9/9] dm-integrity: recalculate checksums

[Mikulas Patocka]

On Sun, 17 Jun 2018, Milan Broz wrote:

> Hi Mikulas,
> 
> I have some questions for the last dm-integrity recalculation patch (copied below).
> 
> - The "recalculate" dm table attribute is never visible in "dmsetup table",
> it is parsed only on table dm-ioctl input.
> I think if sb flags has SB_FLAG_RECALCULATING, it should appear there.

I'll fix it - I will return the value from the target line.

> - So, SB_FLAG_RECALCULATING bit is persistent and never disappears?
> Is it intentional, that after recalculation is done, it is not cleared?
> (Is it because or later resize recalc the rest of block?)

It is intentional, because someone may extend the device and then, 
recalculating must continue.

> - What about monitoring progress - for now, we can only read superblock
> and parse recalc_sector (not sure if you want this). What about status line option?

I will add the progress counter to the status line.

> - How can I stop/restart recalculation (activate device without it / clear sb flag)?
> Shouldn't this be connected to the presence of "recalculate" flag in mapping table?

Should we even have the possibility to stop recalculating?

If the device was recalculating and we activate it without the 
"recalculate" flag, what should it do? If the user reads data that haven't 
been recalculated yet, we could:
1. return an error
2. ignore the non-recalculated checksum and return data
3. continue with recalculating even if the flag "recalculate" is not present

In my opinion, the option 3 is the best. Option 1 makes the device 
unreliable and option 2 makes the device silently lose the integrity 
protection.

> Thanks,
> Milan

Mikulas

Re: [PATCH 9/9] dm-integrity: recalculate checksums

[Mike Snitzer]

On Mon, Jul 02 2018 at  9:07pm -0400,
Mikulas Patocka <mpatocka@redhat.com> wrote:

> 
> 
> On Sun, 17 Jun 2018, Milan Broz wrote:
> 
> > Hi Mikulas,
> > 
> > I have some questions for the last dm-integrity recalculation patch (copied below).
> > 
> > - The "recalculate" dm table attribute is never visible in "dmsetup table",
> > it is parsed only on table dm-ioctl input.
> > I think if sb flags has SB_FLAG_RECALCULATING, it should appear there.
> 
> I'll fix it - I will return the value from the target line.
> 
> > - So, SB_FLAG_RECALCULATING bit is persistent and never disappears?
> > Is it intentional, that after recalculation is done, it is not cleared?
> > (Is it because or later resize recalc the rest of block?)
> 
> It is intentional, because someone may extend the device and then, 
> recalculating must continue.
> 
> > - What about monitoring progress - for now, we can only read superblock
> > and parse recalc_sector (not sure if you want this). What about status line option?
> 
> I will add the progress counter to the status line.

SO like <completed>/<total> ?

> > - How can I stop/restart recalculation (activate device without it / clear sb flag)?
> > Shouldn't this be connected to the presence of "recalculate" flag in mapping table?
> 
> Should we even have the possibility to stop recalculating?
> 
> If the device was recalculating and we activate it without the 
> "recalculate" flag, what should it do? If the user reads data that haven't 
> been recalculated yet, we could:
> 1. return an error
> 2. ignore the non-recalculated checksum and return data
> 3. continue with recalculating even if the flag "recalculate" is not present
> 
> In my opinion, the option 3 is the best. Option 1 makes the device 
> unreliable and option 2 makes the device silently lose the integrity 
> protection.

Yes, option 3.  But you could even error out on table load; though that
may be too unforgiving.. reality is userspace needs to account for
staying in recalculating mode until it is complete.

But to do that they'd need to monitor (similar to how snapshot-merge
polls progress?)

Mike

Re: [PATCH 9/9] dm-integrity: recalculate checksums

[Milan Broz]

On 07/03/2018 03:21 PM, Mike Snitzer wrote:
>>
>> Should we even have the possibility to stop recalculating?
>>
>> If the device was recalculating and we activate it without the 
>> "recalculate" flag, what should it do? If the user reads data that haven't 
>> been recalculated yet, we could:
>> 1. return an error
>> 2. ignore the non-recalculated checksum and return data
>> 3. continue with recalculating even if the flag "recalculate" is not present
>>
>> In my opinion, the option 3 is the best. Option 1 makes the device 
>> unreliable and option 2 makes the device silently lose the integrity 
>> protection.
> 
> Yes, option 3.  But you could even error out on table load; though that
> may be too unforgiving.. reality is userspace needs to account for
> staying in recalculating mode until it is complete.

I do not think table load error is the way to go.

Can we activate dmintegrity read-only (for example with read-only backend device)
that is not yet fully recalculated?

This can needed in some recovery scenarios... (But her we can use recovery mode).

> But to do that they'd need to monitor (similar to how snapshot-merge
> polls progress?)

For veritysetup I will just add this to "status" command output.

Milan

[PATCH 9/9] dm-integrity: recalculate checksums

[Mikulas Patocka]

[-- Attachment #1: dm-integrity-recalc.patch --]
[-- Type: text/plain, Size: 11851 bytes --]

When using external metadata device and internal hash, recalculate the
checksums when the device is created - so that dm-integrity doesn't have
to overwrite the device. The superblock stores the last position when the
recalculation ended, so that it is properly restarted.

Integrity tags that haven't been recalculated yet are ignored.

This patch also increases the target version to 1.2.0.

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

---
 Documentation/device-mapper/dm-integrity.txt |    4 
 drivers/md/dm-integrity.c                    |  187 ++++++++++++++++++++++++++-
 2 files changed, 187 insertions(+), 4 deletions(-)

Index: linux-2.6/drivers/md/dm-integrity.c
===================================================================
--- linux-2.6.orig/drivers/md/dm-integrity.c	2018-07-03 00:03:58.980000000 +0200
+++ linux-2.6/drivers/md/dm-integrity.c	2018-07-03 20:08:58.270000000 +0200
@@ -31,6 +31,8 @@
 #define MIN_LOG2_INTERLEAVE_SECTORS	3
 #define MAX_LOG2_INTERLEAVE_SECTORS	31
 #define METADATA_WORKQUEUE_MAX_ACTIVE	16
+#define RECALC_SECTORS			8192
+#define RECALC_WRITE_SUPER		16
 
 /*
  * Warning - DEBUG_PRINT prints security-sensitive data to the log,
@@ -58,9 +60,12 @@ struct superblock {
 	__u64 provided_data_sectors;	/* userspace uses this value */
 	__u32 flags;
 	__u8 log2_sectors_per_block;
+	__u8 pad[3];
+	__u64 recalc_sector;
 };
 
 #define SB_FLAG_HAVE_JOURNAL_MAC	0x1
+#define SB_FLAG_RECALCULATING		0x2
 
 #define	JOURNAL_ENTRY_ROUNDUP		8
 
@@ -214,6 +219,11 @@ struct dm_integrity_c {
 	struct workqueue_struct *writer_wq;
 	struct work_struct writer_work;
 
+	struct workqueue_struct *recalc_wq;
+	struct work_struct recalc_work;
+	u8 *recalc_buffer;
+	u8 *recalc_tags;
+
 	struct bio_list flush_bio_list;
 
 	unsigned long autocommit_jiffies;
@@ -417,7 +427,7 @@ static void wraparound_section(struct dm
 
 static void sb_set_version(struct dm_integrity_c *ic)
 {
-	if (ic->meta_dev)
+	if (ic->meta_dev || ic->sb->flags & cpu_to_le32(SB_FLAG_RECALCULATING))
 		ic->sb->version = SB_VERSION_2;
 	else
 		ic->sb->version = SB_VERSION_1;
@@ -1780,9 +1790,14 @@ offload_to_thread:
 
 	if (need_sync_io) {
 		wait_for_completion_io(&read_comp);
+		if (unlikely(ic->recalc_wq != NULL) &&
+		    ic->sb->flags & cpu_to_le32(SB_FLAG_RECALCULATING) &&
+		    dio->range.logical_sector + dio->range.n_sectors > le64_to_cpu(ic->sb->recalc_sector))
+			goto skip_check;
 		if (likely(!bio->bi_status))
 			integrity_metadata(&dio->work);
 		else
+skip_check:
 			dec_in_flight(dio);
 
 	} else {
@@ -2082,6 +2097,108 @@ static void integrity_writer(struct work
 	spin_unlock_irq(&ic->endio_wait.lock);
 }
 
+static void recalc_write_super(struct dm_integrity_c *ic)
+{
+	int r;
+
+	dm_integrity_flush_buffers(ic);
+	if (dm_integrity_failed(ic))
+		return;
+
+	sb_set_version(ic);
+	r = sync_rw_sb(ic, REQ_OP_WRITE, 0);
+	if (unlikely(r))
+		dm_integrity_io_error(ic, "writing superblock", r);
+}
+
+static void integrity_recalc(struct work_struct *w)
+{
+	struct dm_integrity_c *ic = container_of(w, struct dm_integrity_c, recalc_work);
+	struct dm_integrity_range range;
+	struct dm_io_request io_req;
+	struct dm_io_region io_loc;
+	sector_t area, offset;
+	sector_t metadata_block;
+	unsigned metadata_offset;
+	__u8 *t;
+	unsigned i;
+	int r;
+	unsigned super_counter = 0;
+
+	spin_lock_irq(&ic->endio_wait.lock);
+
+next_chunk:
+
+	if (unlikely(READ_ONCE(ic->suspending)))
+		goto unlock_ret;
+
+	range.logical_sector = le64_to_cpu(ic->sb->recalc_sector);
+	if (unlikely(range.logical_sector >= ic->provided_data_sectors))
+		goto unlock_ret;
+
+	get_area_and_offset(ic, range.logical_sector, &area, &offset);
+	range.n_sectors = min((sector_t)RECALC_SECTORS, ic->provided_data_sectors - range.logical_sector);
+	if (!ic->meta_dev)
+		range.n_sectors = min(range.n_sectors, (1U << ic->sb->log2_interleave_sectors) - (unsigned)offset);
+
+	if (unlikely(!add_new_range(ic, &range, true)))
+		wait_and_add_new_range(ic, &range);
+
+	spin_unlock_irq(&ic->endio_wait.lock);
+
+	if (unlikely(++super_counter == RECALC_WRITE_SUPER)) {
+		recalc_write_super(ic);
+		super_counter = 0;
+	}
+
+	if (unlikely(dm_integrity_failed(ic)))
+		goto err;
+
+	io_req.bi_op = REQ_OP_READ;
+	io_req.bi_op_flags = 0;
+	io_req.mem.type = DM_IO_VMA;
+	io_req.mem.ptr.addr = ic->recalc_buffer;
+	io_req.notify.fn = NULL;
+	io_req.client = ic->io;
+	io_loc.bdev = ic->dev->bdev;
+	io_loc.sector = get_data_sector(ic, area, offset);
+	io_loc.count = range.n_sectors;
+
+	r = dm_io(&io_req, 1, &io_loc, NULL);
+	if (unlikely(r)) {
+		dm_integrity_io_error(ic, "reading data", r);
+		goto err;
+	}
+
+	t = ic->recalc_tags;
+	for (i = 0; i < range.n_sectors; i += ic->sectors_per_block) {
+		integrity_sector_checksum(ic, range.logical_sector + i, ic->recalc_buffer + (i << SECTOR_SHIFT), t);
+		t += ic->tag_size;
+	}
+
+	metadata_block = get_metadata_sector_and_offset(ic, area, offset, &metadata_offset);
+
+	r = dm_integrity_rw_tag(ic, ic->recalc_tags, &metadata_block, &metadata_offset, t - ic->recalc_tags, TAG_WRITE);
+	if (unlikely(r)) {
+		dm_integrity_io_error(ic, "writing tags", r);
+		goto err;
+	}
+
+	spin_lock_irq(&ic->endio_wait.lock);
+	remove_range_unlocked(ic, &range);
+	ic->sb->recalc_sector = cpu_to_le64(range.logical_sector + range.n_sectors);
+	goto next_chunk;
+
+err:
+	remove_range(ic, &range);
+	return;
+
+unlock_ret:
+	spin_unlock_irq(&ic->endio_wait.lock);
+
+	recalc_write_super(ic);
+}
+
 static void init_journal(struct dm_integrity_c *ic, unsigned start_section,
 			 unsigned n_sections, unsigned char commit_seq)
 {
@@ -2286,6 +2403,9 @@ static void dm_integrity_postsuspend(str
 
 	WRITE_ONCE(ic->suspending, 1);
 
+	if (ic->recalc_wq)
+		drain_workqueue(ic->recalc_wq);
+
 	queue_work(ic->commit_wq, &ic->commit_work);
 	drain_workqueue(ic->commit_wq);
 
@@ -2308,6 +2428,16 @@ static void dm_integrity_resume(struct d
 	struct dm_integrity_c *ic = (struct dm_integrity_c *)ti->private;
 
 	replay_journal(ic);
+
+	if (ic->recalc_wq && ic->sb->flags & cpu_to_le32(SB_FLAG_RECALCULATING)) {
+		__u64 recalc_pos = le64_to_cpu(ic->sb->recalc_sector);
+		if (recalc_pos < ic->provided_data_sectors) {
+			queue_work(ic->recalc_wq, &ic->recalc_work);
+		} else if (recalc_pos > ic->provided_data_sectors) {
+			ic->sb->recalc_sector = cpu_to_le64(ic->provided_data_sectors);
+			recalc_write_super(ic);
+		}
+	}
 }
 
 static void dm_integrity_status(struct dm_target *ti, status_type_t type,
@@ -2322,6 +2452,10 @@ static void dm_integrity_status(struct d
 		DMEMIT("%llu %llu",
 			(unsigned long long)atomic64_read(&ic->number_of_mismatches),
 			(unsigned long long)ic->provided_data_sectors);
+		if (ic->sb->flags & cpu_to_le32(SB_FLAG_RECALCULATING))
+			DMEMIT(" %llu", (unsigned long long)le64_to_cpu(ic->sb->recalc_sector));
+		else
+			DMEMIT(" -");
 		break;
 
 	case STATUSTYPE_TABLE: {
@@ -2331,6 +2465,7 @@ static void dm_integrity_status(struct d
 		arg_count = 5;
 		arg_count += !!ic->meta_dev;
 		arg_count += ic->sectors_per_block != 1;
+		arg_count += !!(ic->sb->flags & cpu_to_le32(SB_FLAG_RECALCULATING));
 		arg_count += !!ic->internal_hash_alg.alg_string;
 		arg_count += !!ic->journal_crypt_alg.alg_string;
 		arg_count += !!ic->journal_mac_alg.alg_string;
@@ -2338,13 +2473,15 @@ static void dm_integrity_status(struct d
 		       ic->tag_size, ic->mode, arg_count);
 		if (ic->meta_dev)
 			DMEMIT(" meta_device:%s", ic->meta_dev->name);
+		if (ic->sectors_per_block != 1)
+			DMEMIT(" block_size:%u", ic->sectors_per_block << SECTOR_SHIFT);
+		if (ic->sb->flags & cpu_to_le32(SB_FLAG_RECALCULATING))
+			DMEMIT(" recalculate");
 		DMEMIT(" journal_sectors:%u", ic->initial_sectors - SB_SECTORS);
 		DMEMIT(" interleave_sectors:%u", 1U << ic->sb->log2_interleave_sectors);
 		DMEMIT(" buffer_sectors:%u", 1U << ic->log2_buffer_sectors);
 		DMEMIT(" journal_watermark:%u", (unsigned)watermark_percentage);
 		DMEMIT(" commit_time:%u", ic->autocommit_msec);
-		if (ic->sectors_per_block != 1)
-			DMEMIT(" block_size:%u", ic->sectors_per_block << SECTOR_SHIFT);
 
 #define EMIT_ALG(a, n)							\
 		do {							\
@@ -2950,6 +3087,7 @@ static int dm_integrity_ctr(struct dm_ta
 		{0, 9, "Invalid number of feature args"},
 	};
 	unsigned journal_sectors, interleave_sectors, buffer_sectors, journal_watermark, sync_msec;
+	bool recalculate;
 	bool should_write_sb;
 	__u64 threshold;
 	unsigned long long start;
@@ -3011,6 +3149,7 @@ static int dm_integrity_ctr(struct dm_ta
 	buffer_sectors = DEFAULT_BUFFER_SECTORS;
 	journal_watermark = DEFAULT_JOURNAL_WATERMARK;
 	sync_msec = DEFAULT_SYNC_MSEC;
+	recalculate = false;
 	ic->sectors_per_block = 1;
 
 	as.argc = argc - DIRECT_ARGUMENTS;
@@ -3072,6 +3211,8 @@ static int dm_integrity_ctr(struct dm_ta
 					    "Invalid journal_mac argument");
 			if (r)
 				goto bad;
+		} else if (!strcmp(opt_string, "recalculate")) {
+			recalculate = true;
 		} else {
 			r = -EINVAL;
 			ti->error = "Invalid argument";
@@ -3300,6 +3441,38 @@ try_smaller_buffer:
 		    (unsigned long long)ic->provided_data_sectors);
 	DEBUG_print("	log2_buffer_sectors %u\n", ic->log2_buffer_sectors);
 
+	if (recalculate && !(ic->sb->flags & cpu_to_le32(SB_FLAG_RECALCULATING))) {
+		ic->sb->flags |= cpu_to_le32(SB_FLAG_RECALCULATING);
+		ic->sb->recalc_sector = cpu_to_le64(0);
+	}
+
+	if (ic->sb->flags & cpu_to_le32(SB_FLAG_RECALCULATING)) {
+		if (!ic->internal_hash) {
+			r = -EINVAL;
+			ti->error = "Recalculate is only valid with internal hash";
+			goto bad;
+		}
+		ic->recalc_wq = alloc_workqueue("dm-intergrity-recalc", WQ_MEM_RECLAIM, 1);
+		if (!ic->recalc_wq ) {
+			ti->error = "Cannot allocate workqueue";
+			r = -ENOMEM;
+			goto bad;
+		}
+		INIT_WORK(&ic->recalc_work, integrity_recalc);
+		ic->recalc_buffer = vmalloc(RECALC_SECTORS << SECTOR_SHIFT);
+		if (!ic->recalc_buffer) {
+			ti->error = "Cannot allocate buffer for recalculating";
+			r = -ENOMEM;
+			goto bad;
+		}
+		ic->recalc_tags = kvmalloc((RECALC_SECTORS >> ic->sb->log2_sectors_per_block) * ic->tag_size, GFP_KERNEL);
+		if (!ic->recalc_tags) {
+			ti->error = "Cannot allocate tags for recalculating";
+			r = -ENOMEM;
+			goto bad;
+		}
+	}
+
 	ic->bufio = dm_bufio_client_create(ic->meta_dev ? ic->meta_dev->bdev : ic->dev->bdev,
 			1U << (SECTOR_SHIFT + ic->log2_buffer_sectors), 1, 0, NULL, NULL);
 	if (IS_ERR(ic->bufio)) {
@@ -3366,6 +3539,12 @@ static void dm_integrity_dtr(struct dm_t
 		destroy_workqueue(ic->commit_wq);
 	if (ic->writer_wq)
 		destroy_workqueue(ic->writer_wq);
+	if (ic->recalc_wq)
+		destroy_workqueue(ic->recalc_wq);
+	if (ic->recalc_buffer)
+		vfree(ic->recalc_buffer);
+	if (ic->recalc_tags)
+		kvfree(ic->recalc_tags);
 	if (ic->bufio)
 		dm_bufio_client_destroy(ic->bufio);
 	mempool_exit(&ic->journal_io_mempool);
@@ -3415,7 +3594,7 @@ static void dm_integrity_dtr(struct dm_t
 
 static struct target_type integrity_target = {
 	.name			= "integrity",
-	.version		= {1, 1, 0},
+	.version		= {1, 2, 0},
 	.module			= THIS_MODULE,
 	.features		= DM_TARGET_SINGLETON | DM_TARGET_INTEGRITY,
 	.ctr			= dm_integrity_ctr,
Index: linux-2.6/Documentation/device-mapper/dm-integrity.txt
===================================================================
--- linux-2.6.orig/Documentation/device-mapper/dm-integrity.txt	2018-07-03 00:03:58.980000000 +0200
+++ linux-2.6/Documentation/device-mapper/dm-integrity.txt	2018-07-03 00:03:58.970000000 +0200
@@ -113,6 +113,10 @@ internal_hash:algorithm(:key)	(the key i
 	from an upper layer target, such as dm-crypt. The upper layer
 	target should check the validity of the integrity tags.
 
+recalculate
+	Recalculate the integrity tags automatically. It is only valid
+	when using internal hash.
+
 journal_crypt:algorithm(:key)	(the key is optional)
 	Encrypt the journal using given algorithm to make sure that the
 	attacker can't read the journal. You can use a block cipher here

Re: [PATCH 9/9] dm-integrity: recalculate checksums

[Mikulas Patocka]

On Tue, 3 Jul 2018, Milan Broz wrote:

> On 07/03/2018 03:21 PM, Mike Snitzer wrote:
> >>
> >> Should we even have the possibility to stop recalculating?
> >>
> >> If the device was recalculating and we activate it without the 
> >> "recalculate" flag, what should it do? If the user reads data that haven't 
> >> been recalculated yet, we could:
> >> 1. return an error
> >> 2. ignore the non-recalculated checksum and return data
> >> 3. continue with recalculating even if the flag "recalculate" is not present
> >>
> >> In my opinion, the option 3 is the best. Option 1 makes the device 
> >> unreliable and option 2 makes the device silently lose the integrity 
> >> protection.
> > 
> > Yes, option 3.  But you could even error out on table load; though that
> > may be too unforgiving.. reality is userspace needs to account for
> > staying in recalculating mode until it is complete.
> 
> I do not think table load error is the way to go.
> 
> Can we activate dmintegrity read-only (for example with read-only backend device)
> that is not yet fully recalculated?

There's no read-only activation because the journal must be replayed. Most 
journaled filesystems also don't provide read-only activation because they 
need to replay the journal.

> This can needed in some recovery scenarios... (But her we can use recovery mode).
> 
> > But to do that they'd need to monitor (similar to how snapshot-merge
> > polls progress?)
> 
> For veritysetup I will just add this to "status" command output.

I added a field to the status that show the recalculate position. If 
recalculating was never activated, it is "-".

> Milan

Mikulas

Re: [PATCH 9/9] dm-integrity: recalculate checksums

[Zdenek Kabelac]

Dne 3.7.2018 v 20:24 Mikulas Patocka napsal(a):
> 
> 
> On Tue, 3 Jul 2018, Milan Broz wrote:
> 
>> On 07/03/2018 03:21 PM, Mike Snitzer wrote:
>>>>
>>>> Should we even have the possibility to stop recalculating?
>>>>
>>>> If the device was recalculating and we activate it without the
>>>> "recalculate" flag, what should it do? If the user reads data that haven't
>>>> been recalculated yet, we could:
>>>> 1. return an error
>>>> 2. ignore the non-recalculated checksum and return data
>>>> 3. continue with recalculating even if the flag "recalculate" is not present
>>>>
>>>> In my opinion, the option 3 is the best. Option 1 makes the device
>>>> unreliable and option 2 makes the device silently lose the integrity
>>>> protection.
>>>
>>> Yes, option 3.  But you could even error out on table load; though that
>>> may be too unforgiving.. reality is userspace needs to account for
>>> staying in recalculating mode until it is complete.
>>
>> I do not think table load error is the way to go.
>>
>> Can we activate dmintegrity read-only (for example with read-only backend device)
>> that is not yet fully recalculated?
> 
> There's no read-only activation because the journal must be replayed. Most
> journaled filesystems also don't provide read-only activation because they
> need to replay the journal.

There would  always an option to use some temporary space with old snapshot 
COW target - and use read-only device on top of that.

Zdenek