[PATCH v5 0/2] dm-integrity: Implement asynch digest support

[PATCH v5 0/2] dm-integrity: Implement asynch digest support

[Harald Freudenberger]

Support for ahashes in dm-integrity.

Changelog:

v1: First implementation. Tested with crc32, sha256, hmac-sha256 and
    the s390 specific implementations for hmac-sha256 and protected
    key phmac-sha256. Also ran with some instrumented code (in the digest
    implementation) to verify that in fact now the code runs asynchronous.
v2: Support shash and ahash. Based on Mikulas' idea about implementing
    ahash support similar to dm-verity this version now adds support
    for ahash but does not replace the shash support. For more details
    see the text of the patch header.
v3: The line to store the digestsize into the new internal variable
    did not make it into the patch set which was sent out. So now
    this important code piece is also there. Also rebuilded, sparse
    checked and tested to make sure the patches are ok.
v4: Thanks to Mikulas a total new implementation of the ahash support
    for the dm-integrity layer :-)
v5: Slight rework around the allocation and comparing of ahash and
    shash algorithm.
    V5 has been tested with the new introduced ahash phmac which is a
    protected key ("hardware key") version of a hmac for s390. As of now
    phmac is only available in Herbert Xu's cryptodev-2.6 kernel tree
    but will be merged into mainline with the next merge window for
    the 6.17 development kernel.

Mikulas Patocka (2):
  dm-integrity: use internal variable for digestsize
  dm-integrity: introduce ahash support for the internal hash

 drivers/md/dm-integrity.c | 370 +++++++++++++++++++++++++++-----------
 1 file changed, 265 insertions(+), 105 deletions(-)


base-commit: 89be9a83ccf1f88522317ce02f854f30d6115c41
--
2.43.0

[PATCH v5 1/2] dm-integrity: use internal variable for digestsize

[Harald Freudenberger]

From: Mikulas Patocka <mpatocka@redhat.com>

Instead of calling digestsize() each time the digestsize for
the internal hash is needed, store the digestsize in a new
field internal_hash_digestsize within struct dm_integrity_c
once and use this value when needed.

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
---
 drivers/md/dm-integrity.c | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/drivers/md/dm-integrity.c b/drivers/md/dm-integrity.c
index 4395657fa583..40f5bd5b0764 100644
--- a/drivers/md/dm-integrity.c
+++ b/drivers/md/dm-integrity.c
@@ -223,6 +223,7 @@ struct dm_integrity_c {
 	int failed;
 
 	struct crypto_shash *internal_hash;
+	unsigned int internal_hash_digestsize;
 
 	struct dm_target *ti;
 
@@ -1676,7 +1677,7 @@ static void integrity_sector_checksum(struct dm_integrity_c *ic, sector_t sector
 		goto failed;
 	}
 
-	digest_size = crypto_shash_digestsize(ic->internal_hash);
+	digest_size = ic->internal_hash_digestsize;
 	if (unlikely(digest_size < ic->tag_size))
 		memset(result + digest_size, 0, ic->tag_size - digest_size);
 
@@ -1776,7 +1777,7 @@ static void integrity_metadata(struct work_struct *w)
 	if (ic->internal_hash) {
 		struct bvec_iter iter;
 		struct bio_vec bv;
-		unsigned int digest_size = crypto_shash_digestsize(ic->internal_hash);
+		unsigned int digest_size = ic->internal_hash_digestsize;
 		struct bio *bio = dm_bio_from_per_bio_data(dio, sizeof(struct dm_integrity_io));
 		char *checksums;
 		unsigned int extra_space = unlikely(digest_size > ic->tag_size) ? digest_size - ic->tag_size : 0;
@@ -2124,7 +2125,7 @@ static bool __journal_read_write(struct dm_integrity_io *dio, struct bio *bio,
 				} while (++s < ic->sectors_per_block);
 
 				if (ic->internal_hash) {
-					unsigned int digest_size = crypto_shash_digestsize(ic->internal_hash);
+					unsigned int digest_size = ic->internal_hash_digestsize;
 
 					if (unlikely(digest_size > ic->tag_size)) {
 						char checksums_onstack[HASH_MAX_DIGESTSIZE];
@@ -2428,7 +2429,7 @@ static int dm_integrity_map_inline(struct dm_integrity_io *dio, bool from_map)
 	if (!dio->integrity_payload) {
 		unsigned digest_size, extra_size;
 		dio->payload_len = ic->tuple_size * (bio_sectors(bio) >> ic->sb->log2_sectors_per_block);
-		digest_size = crypto_shash_digestsize(ic->internal_hash);
+		digest_size = ic->internal_hash_digestsize;
 		extra_size = unlikely(digest_size > ic->tag_size) ? digest_size - ic->tag_size : 0;
 		dio->payload_len += extra_size;
 		dio->integrity_payload = kmalloc(dio->payload_len, GFP_NOIO | __GFP_NORETRY | __GFP_NOMEMALLOC | __GFP_NOWARN);
@@ -2589,7 +2590,8 @@ static void dm_integrity_inline_recheck(struct work_struct *w)
 		bio_put(outgoing_bio);
 
 		integrity_sector_checksum(ic, dio->bio_details.bi_iter.bi_sector, outgoing_data, digest);
-		if (unlikely(crypto_memneq(digest, dio->integrity_payload, min(crypto_shash_digestsize(ic->internal_hash), ic->tag_size)))) {
+		if (unlikely(crypto_memneq(digest, dio->integrity_payload,
+					   min(ic->internal_hash_digestsize, ic->tag_size)))) {
 			DMERR_LIMIT("%pg: Checksum failed at sector 0x%llx",
 				ic->dev->bdev, dio->bio_details.bi_iter.bi_sector);
 			atomic64_inc(&ic->number_of_mismatches);
@@ -2629,7 +2631,7 @@ static int dm_integrity_end_io(struct dm_target *ti, struct bio *bio, blk_status
 				//memset(mem, 0xff, ic->sectors_per_block << SECTOR_SHIFT);
 				integrity_sector_checksum(ic, dio->bio_details.bi_iter.bi_sector, mem, digest);
 				if (unlikely(crypto_memneq(digest, dio->integrity_payload + pos,
-						min(crypto_shash_digestsize(ic->internal_hash), ic->tag_size)))) {
+							   min(ic->internal_hash_digestsize, ic->tag_size)))) {
 					kunmap_local(mem);
 					dm_integrity_free_payload(dio);
 					INIT_WORK(&dio->work, dm_integrity_inline_recheck);
@@ -3011,8 +3013,8 @@ static void integrity_recalc(struct work_struct *w)
 		goto free_ret;
 	}
 	recalc_tags_size = (recalc_sectors >> ic->sb->log2_sectors_per_block) * ic->tag_size;
-	if (crypto_shash_digestsize(ic->internal_hash) > ic->tag_size)
-		recalc_tags_size += crypto_shash_digestsize(ic->internal_hash) - ic->tag_size;
+	if (ic->internal_hash_digestsize > ic->tag_size)
+		recalc_tags_size += ic->internal_hash_digestsize - ic->tag_size;
 	recalc_tags = kvmalloc(recalc_tags_size, GFP_NOIO);
 	if (!recalc_tags) {
 		vfree(recalc_buffer);
@@ -3171,8 +3173,8 @@ static void integrity_recalc_inline(struct work_struct *w)
 	}
 
 	recalc_tags_size = (recalc_sectors >> ic->sb->log2_sectors_per_block) * ic->tuple_size;
-	if (crypto_shash_digestsize(ic->internal_hash) > ic->tuple_size)
-		recalc_tags_size += crypto_shash_digestsize(ic->internal_hash) - ic->tuple_size;
+	if (ic->internal_hash_digestsize > ic->tuple_size)
+		recalc_tags_size += ic->internal_hash_digestsize - ic->tuple_size;
 	recalc_tags = kmalloc(recalc_tags_size, GFP_NOIO | __GFP_NOWARN);
 	if (!recalc_tags) {
 		kfree(recalc_buffer);
@@ -4694,6 +4696,8 @@ static int dm_integrity_ctr(struct dm_target *ti, unsigned int argc, char **argv
 		    "Invalid internal hash", "Error setting internal hash key");
 	if (r)
 		goto bad;
+	if (ic->internal_hash)
+		ic->internal_hash_digestsize = crypto_shash_digestsize(ic->internal_hash);
 
 	r = get_mac(&ic->journal_mac, &ic->journal_mac_alg, &ti->error,
 		    "Invalid journal mac", "Error setting journal mac key");
@@ -4706,7 +4710,7 @@ static int dm_integrity_ctr(struct dm_target *ti, unsigned int argc, char **argv
 			r = -EINVAL;
 			goto bad;
 		}
-		ic->tag_size = crypto_shash_digestsize(ic->internal_hash);
+		ic->tag_size = ic->internal_hash_digestsize;
 	}
 	if (ic->tag_size > MAX_TAG_SIZE) {
 		ti->error = "Too big tag size";
-- 
2.43.0

[PATCH v5 2/2] dm-integrity: introduce ahash support for the internal hash

[Harald Freudenberger]

From: Mikulas Patocka <mpatocka@redhat.com>

Introduce ahash support for the "internal hash" algorithm.

Rework the dm-integrity code to be able to run the "internal hash"
either with a synchronous ("shash") or asynchronous ("ahash") hash
algorithm implementation.

The get_mac() function now tries to decide which of the digest
implementations to use if there is a choice:
- If an ahash and shash tfm is available and both are backed by the
  same driver name it is assumed that the shash is the faster
  implementation and thus the shash tfm is delivered to the caller.
- If an ahash and shash tfm is available but the backing device driver
  divers (different driver names) it is assumed that the ahash
  implementation is a "better" hardware based implementation and thus
  the ahash tfm is delivered to the caller.
- If there is no choice, for example only an ahash or an shash
  implementation is available then this tfm is delivered to the
  caller. Especially in cases where only an ahash implementation is
  available this is now used instead of failing.
- The caller can steer this choice by passing a NULL to the ahash or
  shash parameter, thus enforcing to only allocate an algorithm of the
  remaining possibility.

The function integrity_sector_checksum() is now only a dispatcher
function calling one of the two new functions
integrity_ahash_sector_checksum() or integrity_shash_sector_checksum()
based on which tfm is allocated based on the two new fields
internal_shash and internal_ahash in struct dm_integrity_c.

Together with this comes some slight rework around availability and
digest size of the internal hash in use.

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Signed-off-by: Harald Freudenberger <freude@linux.ibm.com>
---
 drivers/md/dm-integrity.c | 352 +++++++++++++++++++++++++++-----------
 1 file changed, 254 insertions(+), 98 deletions(-)

diff --git a/drivers/md/dm-integrity.c b/drivers/md/dm-integrity.c
index 40f5bd5b0764..9dd1f18e0adb 100644
--- a/drivers/md/dm-integrity.c
+++ b/drivers/md/dm-integrity.c
@@ -222,7 +222,9 @@ struct dm_integrity_c {
 
 	int failed;
 
-	struct crypto_shash *internal_hash;
+	bool have_internal_hash;
+	struct crypto_shash *internal_shash;
+	struct crypto_ahash *internal_ahash;
 	unsigned int internal_hash_digestsize;
 
 	struct dm_target *ti;
@@ -289,6 +291,8 @@ struct dm_integrity_c {
 	struct bio_set recalc_bios;
 
 	struct notifier_block reboot_notifier;
+
+	struct dm_integrity_io *dummy_dio;
 };
 
 struct dm_integrity_range {
@@ -327,6 +331,8 @@ struct dm_integrity_io {
 	unsigned payload_len;
 	bool integrity_payload_from_mempool;
 	bool integrity_range_locked;
+
+	__le64 sector_le;
 };
 
 struct journal_completion {
@@ -1560,7 +1566,7 @@ static void submit_flush_bio(struct dm_integrity_c *ic, struct dm_integrity_io *
 	unsigned long flags;
 
 	spin_lock_irqsave(&ic->endio_wait.lock, flags);
-	bio = dm_bio_from_per_bio_data(dio, sizeof(struct dm_integrity_io));
+	bio = dm_bio_from_per_bio_data(dio, ic->ti->per_io_data_size);
 	bio_list_add(&ic->flush_bio_list, bio);
 	spin_unlock_irqrestore(&ic->endio_wait.lock, flags);
 
@@ -1588,7 +1594,7 @@ static void do_endio(struct dm_integrity_c *ic, struct bio *bio)
 
 static void do_endio_flush(struct dm_integrity_c *ic, struct dm_integrity_io *dio)
 {
-	struct bio *bio = dm_bio_from_per_bio_data(dio, sizeof(struct dm_integrity_io));
+	struct bio *bio = dm_bio_from_per_bio_data(dio, ic->ti->per_io_data_size);
 
 	if (unlikely(dio->fua) && likely(!bio->bi_status) && likely(!dm_integrity_failed(ic)))
 		submit_flush_bio(ic, dio);
@@ -1607,7 +1613,7 @@ static void dec_in_flight(struct dm_integrity_io *dio)
 		if (dio->op == REQ_OP_WRITE || unlikely(dio->op == REQ_OP_DISCARD))
 			schedule_autocommit(ic);
 
-		bio = dm_bio_from_per_bio_data(dio, sizeof(struct dm_integrity_io));
+		bio = dm_bio_from_per_bio_data(dio, ic->ti->per_io_data_size);
 		if (unlikely(dio->bi_status) && !bio->bi_status)
 			bio->bi_status = dio->bi_status;
 		if (likely(!bio->bi_status) && unlikely(bio_sectors(bio) != dio->range.n_sectors)) {
@@ -1623,7 +1629,7 @@ static void dec_in_flight(struct dm_integrity_io *dio)
 
 static void integrity_end_io(struct bio *bio)
 {
-	struct dm_integrity_io *dio = dm_per_bio_data(bio, sizeof(struct dm_integrity_io));
+	struct dm_integrity_io *dio = bio->bi_private;
 
 	dm_bio_restore(&dio->bio_details, bio);
 	if (bio->bi_integrity)
@@ -1635,63 +1641,121 @@ static void integrity_end_io(struct bio *bio)
 	dec_in_flight(dio);
 }
 
-static void integrity_sector_checksum(struct dm_integrity_c *ic, sector_t sector,
-				      const char *data, char *result)
+static bool integrity_shash_sector_checksum(struct dm_integrity_io *dio,
+					    sector_t sector, const char *data,
+					    char *result)
 {
+	struct dm_integrity_c *ic = dio->ic;
 	__le64 sector_le = cpu_to_le64(sector);
-	SHASH_DESC_ON_STACK(req, ic->internal_hash);
+	SHASH_DESC_ON_STACK(req, ic->internal_shash);
 	int r;
-	unsigned int digest_size;
 
-	req->tfm = ic->internal_hash;
+	req->tfm = ic->internal_shash;
 
 	r = crypto_shash_init(req);
 	if (unlikely(r < 0)) {
 		dm_integrity_io_error(ic, "crypto_shash_init", r);
-		goto failed;
+		return false;
 	}
 
 	if (ic->sb->flags & cpu_to_le32(SB_FLAG_FIXED_HMAC)) {
 		r = crypto_shash_update(req, (__u8 *)&ic->sb->salt, SALT_SIZE);
 		if (unlikely(r < 0)) {
 			dm_integrity_io_error(ic, "crypto_shash_update", r);
-			goto failed;
+			return false;
 		}
 	}
 
 	r = crypto_shash_update(req, (const __u8 *)&sector_le, sizeof(sector_le));
 	if (unlikely(r < 0)) {
 		dm_integrity_io_error(ic, "crypto_shash_update", r);
-		goto failed;
+		return false;
 	}
 
 	r = crypto_shash_update(req, data, ic->sectors_per_block << SECTOR_SHIFT);
 	if (unlikely(r < 0)) {
 		dm_integrity_io_error(ic, "crypto_shash_update", r);
-		goto failed;
+		return false;
 	}
 
 	r = crypto_shash_final(req, result);
 	if (unlikely(r < 0)) {
 		dm_integrity_io_error(ic, "crypto_shash_final", r);
-		goto failed;
+		return false;
 	}
 
-	digest_size = ic->internal_hash_digestsize;
-	if (unlikely(digest_size < ic->tag_size))
-		memset(result + digest_size, 0, ic->tag_size - digest_size);
+	return true;
+}
 
-	return;
+static bool integrity_ahash_sector_checksum(struct dm_integrity_io *dio,
+					    sector_t sector, const char *data,
+					    char *result)
+{
+	struct dm_integrity_c *ic = dio->ic;
+	struct ahash_request *req = (struct ahash_request *)(dio + 1);
+	__le64 *sector_le = &dio->sector_le;
+	struct scatterlist sg[3], *s;
+	DECLARE_CRYPTO_WAIT(wait);
+	int nbytes = 0;
+	int r;
+
+	ahash_request_set_tfm(req, ic->internal_ahash);
+	ahash_request_set_callback(req, 0, crypto_req_done, &wait);
+
+	s = sg;
+	if (ic->sb->flags & cpu_to_le32(SB_FLAG_FIXED_HMAC)) {
+		sg_init_table(sg, 3);
+		sg_set_buf(s, (const __u8 *)&ic->sb->salt, SALT_SIZE);
+		nbytes += SALT_SIZE;
+		s++;
+	} else {
+		sg_init_table(sg, 2);
+	}
+
+	*sector_le = cpu_to_le64(sector);
+	sg_set_buf(s, (const __u8 *)sector_le, sizeof(*sector_le));
+	nbytes += sizeof(*sector_le);
+	s++;
+
+	sg_set_buf(s, data, ic->sectors_per_block << SECTOR_SHIFT);
+	nbytes += ic->sectors_per_block << SECTOR_SHIFT;
+
+	ahash_request_set_crypt(req, sg, result, nbytes);
 
-failed:
-	/* this shouldn't happen anyway, the hash functions have no reason to fail */
-	get_random_bytes(result, ic->tag_size);
+	r = crypto_wait_req(crypto_ahash_digest(req), &wait);
+	if (r) {
+		dm_integrity_io_error(ic, "crypto_ahash_digest", r);
+		return false;
+	}
+
+	return true;
+}
+
+static void integrity_sector_checksum(struct dm_integrity_io *dio,
+				      sector_t sector, const char *data,
+				      char *result)
+{
+	struct dm_integrity_c *ic = dio->ic;
+	bool r;
+
+	if (likely(ic->internal_shash))
+		r = integrity_shash_sector_checksum(dio, sector, data, result);
+	else
+		r = integrity_ahash_sector_checksum(dio, sector, data, result);
+
+	if (unlikely(!r)) {
+		get_random_bytes(result, ic->tag_size);
+	} else {
+		if (unlikely(ic->internal_hash_digestsize < ic->tag_size))
+			memset(result + ic->internal_hash_digestsize,
+			       0, ic->tag_size - ic->internal_hash_digestsize);
+	}
 }
 
 static noinline void integrity_recheck(struct dm_integrity_io *dio, char *checksum)
 {
-	struct bio *bio = dm_bio_from_per_bio_data(dio, sizeof(struct dm_integrity_io));
 	struct dm_integrity_c *ic = dio->ic;
+	struct bio *bio = dm_bio_from_per_bio_data(dio, ic->ti->per_io_data_size);
 	struct bvec_iter iter;
 	struct bio_vec bv;
 	sector_t sector, logical_sector, area, offset;
@@ -1738,7 +1802,7 @@ static noinline void integrity_recheck(struct dm_integrity_io *dio, char *checks
 				goto free_ret;
 			}
 
-			integrity_sector_checksum(ic, logical_sector, buffer, checksum);
+			integrity_sector_checksum(dio, logical_sector, buffer, checksum);
 			r = dm_integrity_rw_tag(ic, checksum, &dio->metadata_block,
 						&dio->metadata_offset, ic->tag_size, TAG_CMP);
 			if (r) {
@@ -1774,11 +1838,11 @@ static void integrity_metadata(struct work_struct *w)
 
 	int r;
 
-	if (ic->internal_hash) {
+	if (ic->have_internal_hash) {
 		struct bvec_iter iter;
 		struct bio_vec bv;
 		unsigned int digest_size = ic->internal_hash_digestsize;
-		struct bio *bio = dm_bio_from_per_bio_data(dio, sizeof(struct dm_integrity_io));
+		struct bio *bio = dm_bio_from_per_bio_data(dio, ic->ti->per_io_data_size);
 		char *checksums;
 		unsigned int extra_space = unlikely(digest_size > ic->tag_size) ? digest_size - ic->tag_size : 0;
 		char checksums_onstack[MAX_T(size_t, HASH_MAX_DIGESTSIZE, MAX_TAG_SIZE)];
@@ -1842,7 +1906,7 @@ static void integrity_metadata(struct work_struct *w)
 			pos = 0;
 			checksums_ptr = checksums;
 			do {
-				integrity_sector_checksum(ic, sector, mem + pos, checksums_ptr);
+				integrity_sector_checksum(dio, sector, mem + pos, checksums_ptr);
 				checksums_ptr += ic->tag_size;
 				sectors_to_process -= ic->sectors_per_block;
 				pos += ic->sectors_per_block << SECTOR_SHIFT;
@@ -1942,7 +2006,7 @@ static inline bool dm_integrity_check_limits(struct dm_integrity_c *ic, sector_t
 static int dm_integrity_map(struct dm_target *ti, struct bio *bio)
 {
 	struct dm_integrity_c *ic = ti->private;
-	struct dm_integrity_io *dio = dm_per_bio_data(bio, sizeof(struct dm_integrity_io));
+	struct dm_integrity_io *dio = dm_per_bio_data(bio, ic->ti->per_io_data_size);
 	struct bio_integrity_payload *bip;
 
 	sector_t area, offset;
@@ -1992,7 +2056,7 @@ static int dm_integrity_map(struct dm_target *ti, struct bio *bio)
 		return DM_MAPIO_KILL;
 
 	bip = bio_integrity(bio);
-	if (!ic->internal_hash) {
+	if (!ic->have_internal_hash) {
 		if (bip) {
 			unsigned int wanted_tag_size = bio_sectors(bio) >> ic->sb->log2_sectors_per_block;
 
@@ -2073,10 +2137,10 @@ static bool __journal_read_write(struct dm_integrity_io *dio, struct bio *bio,
 					mem_ptr += 1 << SECTOR_SHIFT;
 				} while (++s < ic->sectors_per_block);
 #ifdef INTERNAL_VERIFY
-				if (ic->internal_hash) {
+				if (ic->have_internal_hash) {
 					char checksums_onstack[MAX_T(size_t, HASH_MAX_DIGESTSIZE, MAX_TAG_SIZE)];
 
-					integrity_sector_checksum(ic, logical_sector, mem + bv.bv_offset, checksums_onstack);
+					integrity_sector_checksum(dio, logical_sector, mem + bv.bv_offset, checksums_onstack);
 					if (unlikely(crypto_memneq(checksums_onstack, journal_entry_tag(ic, je), ic->tag_size))) {
 						DMERR_LIMIT("Checksum failed when reading from journal, at sector 0x%llx",
 							    logical_sector);
@@ -2087,7 +2151,7 @@ static bool __journal_read_write(struct dm_integrity_io *dio, struct bio *bio,
 #endif
 			}
 
-			if (!ic->internal_hash) {
+			if (!ic->have_internal_hash) {
 				struct bio_integrity_payload *bip = bio_integrity(bio);
 				unsigned int tag_todo = ic->tag_size;
 				char *tag_ptr = journal_entry_tag(ic, je);
@@ -2124,16 +2188,16 @@ static bool __journal_read_write(struct dm_integrity_io *dio, struct bio *bio,
 					je->last_bytes[s] = js[s].commit_id;
 				} while (++s < ic->sectors_per_block);
 
-				if (ic->internal_hash) {
+				if (ic->have_internal_hash) {
 					unsigned int digest_size = ic->internal_hash_digestsize;
 
 					if (unlikely(digest_size > ic->tag_size)) {
 						char checksums_onstack[HASH_MAX_DIGESTSIZE];
 
-						integrity_sector_checksum(ic, logical_sector, (char *)js, checksums_onstack);
+						integrity_sector_checksum(dio, logical_sector, (char *)js, checksums_onstack);
 						memcpy(journal_entry_tag(ic, je), checksums_onstack, ic->tag_size);
 					} else
-						integrity_sector_checksum(ic, logical_sector, (char *)js, journal_entry_tag(ic, je));
+						integrity_sector_checksum(dio, logical_sector, (char *)js, journal_entry_tag(ic, je));
 				}
 
 				journal_entry_set_sector(je, logical_sector);
@@ -2181,13 +2245,13 @@ static bool __journal_read_write(struct dm_integrity_io *dio, struct bio *bio,
 static void dm_integrity_map_continue(struct dm_integrity_io *dio, bool from_map)
 {
 	struct dm_integrity_c *ic = dio->ic;
-	struct bio *bio = dm_bio_from_per_bio_data(dio, sizeof(struct dm_integrity_io));
+	struct bio *bio = dm_bio_from_per_bio_data(dio, ic->ti->per_io_data_size);
 	unsigned int journal_section, journal_entry;
 	unsigned int journal_read_pos;
 	sector_t recalc_sector;
 	struct completion read_comp;
 	bool discard_retried = false;
-	bool need_sync_io = ic->internal_hash && dio->op == REQ_OP_READ;
+	bool need_sync_io = ic->have_internal_hash && dio->op == REQ_OP_READ;
 
 	if (unlikely(dio->op == REQ_OP_DISCARD) && ic->mode != 'D')
 		need_sync_io = true;
@@ -2360,6 +2424,7 @@ static void dm_integrity_map_continue(struct dm_integrity_io *dio, bool from_map
 	bio_set_dev(bio, ic->dev->bdev);
 	bio->bi_integrity = NULL;
 	bio->bi_opf &= ~REQ_INTEGRITY;
+	bio->bi_private = dio;
 	bio->bi_end_io = integrity_end_io;
 	bio->bi_iter.bi_size = dio->range.n_sectors << SECTOR_SHIFT;
 
@@ -2410,7 +2475,7 @@ static void dm_integrity_map_continue(struct dm_integrity_io *dio, bool from_map
 static int dm_integrity_map_inline(struct dm_integrity_io *dio, bool from_map)
 {
 	struct dm_integrity_c *ic = dio->ic;
-	struct bio *bio = dm_bio_from_per_bio_data(dio, sizeof(struct dm_integrity_io));
+	struct bio *bio = dm_bio_from_per_bio_data(dio, ic->ti->per_io_data_size);
 	struct bio_integrity_payload *bip;
 	unsigned ret;
 	sector_t recalc_sector;
@@ -2509,7 +2574,7 @@ static int dm_integrity_map_inline(struct dm_integrity_io *dio, bool from_map)
 			const char *mem = bvec_kmap_local(&bv);
 			if (ic->tag_size < ic->tuple_size)
 				memset(dio->integrity_payload + pos + ic->tag_size, 0, ic->tuple_size - ic->tuple_size);
-			integrity_sector_checksum(ic, dio->bio_details.bi_iter.bi_sector, mem, dio->integrity_payload + pos);
+			integrity_sector_checksum(dio, dio->bio_details.bi_iter.bi_sector, mem, dio->integrity_payload + pos);
 			kunmap_local(mem);
 			pos += ic->tuple_size;
 			bio_advance_iter_single(bio, &dio->bio_details.bi_iter, ic->sectors_per_block << SECTOR_SHIFT);
@@ -2541,8 +2606,8 @@ static inline void dm_integrity_free_payload(struct dm_integrity_io *dio)
 static void dm_integrity_inline_recheck(struct work_struct *w)
 {
 	struct dm_integrity_io *dio = container_of(w, struct dm_integrity_io, work);
-	struct bio *bio = dm_bio_from_per_bio_data(dio, sizeof(struct dm_integrity_io));
 	struct dm_integrity_c *ic = dio->ic;
+	struct bio *bio = dm_bio_from_per_bio_data(dio, ic->ti->per_io_data_size);
 	struct bio *outgoing_bio;
 	void *outgoing_data;
 
@@ -2589,7 +2654,7 @@ static void dm_integrity_inline_recheck(struct work_struct *w)
 		}
 		bio_put(outgoing_bio);
 
-		integrity_sector_checksum(ic, dio->bio_details.bi_iter.bi_sector, outgoing_data, digest);
+		integrity_sector_checksum(dio, dio->bio_details.bi_iter.bi_sector, outgoing_data, digest);
 		if (unlikely(crypto_memneq(digest, dio->integrity_payload,
 					   min(ic->internal_hash_digestsize, ic->tag_size)))) {
 			DMERR_LIMIT("%pg: Checksum failed at sector 0x%llx",
@@ -2614,34 +2679,63 @@ static void dm_integrity_inline_recheck(struct work_struct *w)
 	bio_endio(bio);
 }
 
+static bool dm_integrity_inline_verify(struct dm_integrity_io *dio)
+{
+	struct dm_integrity_c *ic = dio->ic;
+	struct bio *bio = dm_bio_from_per_bio_data(dio, ic->ti->per_io_data_size);
+	unsigned pos = 0;
+
+	while (dio->bio_details.bi_iter.bi_size) {
+		char digest[HASH_MAX_DIGESTSIZE];
+		struct bio_vec bv = bio_iter_iovec(bio, dio->bio_details.bi_iter);
+		char *mem = bvec_kmap_local(&bv);
+		//memset(mem, 0xff, ic->sectors_per_block << SECTOR_SHIFT);
+		integrity_sector_checksum(dio, dio->bio_details.bi_iter.bi_sector, mem, digest);
+		if (unlikely(crypto_memneq(digest, dio->integrity_payload + pos,
+					   min(ic->internal_hash_digestsize, ic->tag_size)))) {
+			kunmap_local(mem);
+			dm_integrity_free_payload(dio);
+			INIT_WORK(&dio->work, dm_integrity_inline_recheck);
+			queue_work(ic->offload_wq, &dio->work);
+			return false;
+		}
+		kunmap_local(mem);
+		pos += ic->tuple_size;
+		bio_advance_iter_single(bio, &dio->bio_details.bi_iter, ic->sectors_per_block << SECTOR_SHIFT);
+	}
+	return true;
+}
+
+static void dm_integrity_inline_offload(struct work_struct *w)
+{
+	struct dm_integrity_io *dio = container_of(w, struct dm_integrity_io, work);
+	struct bio *bio;
+
+	if (unlikely(!dm_integrity_inline_verify(dio)))
+		return;
+
+	dm_integrity_free_payload(dio);
+
+	bio = dm_bio_from_per_bio_data(dio, dio->ic->ti->per_io_data_size);
+	bio_endio(bio);
+}
+
 static int dm_integrity_end_io(struct dm_target *ti, struct bio *bio, blk_status_t *status)
 {
 	struct dm_integrity_c *ic = ti->private;
 	if (ic->mode == 'I') {
-		struct dm_integrity_io *dio = dm_per_bio_data(bio, sizeof(struct dm_integrity_io));
+		struct dm_integrity_io *dio = dm_per_bio_data(bio, ic->ti->per_io_data_size);
 		if (dio->op == REQ_OP_READ && likely(*status == BLK_STS_OK)) {
-			unsigned pos = 0;
-			if (ic->sb->flags & cpu_to_le32(SB_FLAG_RECALCULATING) &&
-			    unlikely(dio->integrity_range_locked))
+			if (unlikely(dio->integrity_range_locked) ||
+			    !dio->bio_details.bi_iter.bi_size)
 				goto skip_check;
-			while (dio->bio_details.bi_iter.bi_size) {
-				char digest[HASH_MAX_DIGESTSIZE];
-				struct bio_vec bv = bio_iter_iovec(bio, dio->bio_details.bi_iter);
-				char *mem = bvec_kmap_local(&bv);
-				//memset(mem, 0xff, ic->sectors_per_block << SECTOR_SHIFT);
-				integrity_sector_checksum(ic, dio->bio_details.bi_iter.bi_sector, mem, digest);
-				if (unlikely(crypto_memneq(digest, dio->integrity_payload + pos,
-							   min(ic->internal_hash_digestsize, ic->tag_size)))) {
-					kunmap_local(mem);
-					dm_integrity_free_payload(dio);
-					INIT_WORK(&dio->work, dm_integrity_inline_recheck);
-					queue_work(ic->offload_wq, &dio->work);
-					return DM_ENDIO_INCOMPLETE;
-				}
-				kunmap_local(mem);
-				pos += ic->tuple_size;
-				bio_advance_iter_single(bio, &dio->bio_details.bi_iter, ic->sectors_per_block << SECTOR_SHIFT);
+			if (unlikely(ic->internal_ahash != NULL)) {
+				INIT_WORK(&dio->work, dm_integrity_inline_offload);
+				queue_work(ic->offload_wq, &dio->work);
+				return DM_ENDIO_INCOMPLETE;
 			}
+			if (unlikely(!dm_integrity_inline_verify(dio)))
+				return DM_ENDIO_INCOMPLETE;
 		}
 skip_check:
 		dm_integrity_free_payload(dio);
@@ -2657,7 +2751,7 @@ static void integrity_bio_wait(struct work_struct *w)
 	struct dm_integrity_c *ic = dio->ic;
 
 	if (ic->mode == 'I') {
-		struct bio *bio = dm_bio_from_per_bio_data(dio, sizeof(struct dm_integrity_io));
+		struct bio *bio = dm_bio_from_per_bio_data(dio, ic->ti->per_io_data_size);
 		int r = dm_integrity_map_inline(dio, false);
 		switch (r) {
 			case DM_MAPIO_KILL:
@@ -2898,14 +2992,11 @@ static void do_journal_write(struct dm_integrity_c *ic, unsigned int write_start
 				int r;
 				struct journal_entry *je2 = access_journal_entry(ic, i, l);
 
-				if (
-#ifndef INTERNAL_VERIFY
-				    unlikely(from_replay) &&
-#endif
-				    ic->internal_hash) {
+				if (unlikely(from_replay) &&
+				    ic->have_internal_hash) {
 					char test_tag[MAX_T(size_t, HASH_MAX_DIGESTSIZE, MAX_TAG_SIZE)];
 
-					integrity_sector_checksum(ic, sec + ((l - j) << ic->sb->log2_sectors_per_block),
+					integrity_sector_checksum(ic->dummy_dio, sec + ((l - j) << ic->sb->log2_sectors_per_block),
 								  (char *)access_journal_data(ic, i, l), test_tag);
 					if (unlikely(crypto_memneq(test_tag, journal_entry_tag(ic, je2), ic->tag_size))) {
 						dm_integrity_io_error(ic, "tag mismatch when replaying journal", -EILSEQ);
@@ -3099,7 +3190,7 @@ static void integrity_recalc(struct work_struct *w)
 
 	t = recalc_tags;
 	for (i = 0; i < n_sectors; i += ic->sectors_per_block) {
-		integrity_sector_checksum(ic, logical_sector + i, recalc_buffer + (i << SECTOR_SHIFT), t);
+		integrity_sector_checksum(ic->dummy_dio, logical_sector + i, recalc_buffer + (i << SECTOR_SHIFT), t);
 		t += ic->tag_size;
 	}
 
@@ -3220,7 +3311,7 @@ static void integrity_recalc_inline(struct work_struct *w)
 	t = recalc_tags;
 	for (i = 0; i < range.n_sectors; i += ic->sectors_per_block) {
 		memset(t, 0, ic->tuple_size);
-		integrity_sector_checksum(ic, range.logical_sector + i, recalc_buffer + (i << SECTOR_SHIFT), t);
+		integrity_sector_checksum(ic->dummy_dio, range.logical_sector + i, recalc_buffer + (i << SECTOR_SHIFT), t);
 		t += ic->tuple_size;
 	}
 
@@ -3292,7 +3383,7 @@ static void bitmap_block_work(struct work_struct *w)
 	while ((bio = bio_list_pop(&bio_queue))) {
 		struct dm_integrity_io *dio;
 
-		dio = dm_per_bio_data(bio, sizeof(struct dm_integrity_io));
+		dio = dm_per_bio_data(bio, ic->ti->per_io_data_size);
 
 		if (block_bitmap_op(ic, ic->may_write_bitmap, dio->range.logical_sector,
 				    dio->range.n_sectors, BITMAP_OP_TEST_ALL_SET)) {
@@ -3314,7 +3405,7 @@ static void bitmap_block_work(struct work_struct *w)
 			   BITMAP_BLOCK_SIZE >> SECTOR_SHIFT, NULL);
 
 	while ((bio = bio_list_pop(&waiting))) {
-		struct dm_integrity_io *dio = dm_per_bio_data(bio, sizeof(struct dm_integrity_io));
+		struct dm_integrity_io *dio = dm_per_bio_data(bio, ic->ti->per_io_data_size);
 
 		block_bitmap_op(ic, ic->may_write_bitmap, dio->range.logical_sector,
 				dio->range.n_sectors, BITMAP_OP_SET);
@@ -3904,7 +3995,7 @@ static void dm_integrity_io_hints(struct dm_target *ti, struct queue_limits *lim
 		limits->discard_granularity = ic->sectors_per_block << SECTOR_SHIFT;
 	}
 
-	if (!ic->internal_hash) {
+	if (!ic->have_internal_hash) {
 		struct blk_integrity *bi = &limits->integrity;
 
 		memset(bi, 0, sizeof(*bi));
@@ -4212,32 +4303,78 @@ static int get_alg_and_key(const char *arg, struct alg_spec *a, char **error, ch
 	return -ENOMEM;
 }
 
-static int get_mac(struct crypto_shash **hash, struct alg_spec *a, char **error,
+static int get_mac(struct crypto_shash **shash, struct crypto_ahash **ahash,
+		   struct alg_spec *a, char **error,
 		   char *error_alg, char *error_key)
 {
+	const char *ahash_driver_name = NULL;
 	int r;
 
-	if (a->alg_string) {
-		*hash = crypto_alloc_shash(a->alg_string, 0, CRYPTO_ALG_ALLOCATES_MEMORY);
-		if (IS_ERR(*hash)) {
+	if (!a->alg_string)
+		return 0;
+
+	if (ahash) {
+		*ahash = crypto_alloc_ahash(a->alg_string, 0, CRYPTO_ALG_ALLOCATES_MEMORY);
+		if (IS_ERR(*ahash)) {
 			*error = error_alg;
-			r = PTR_ERR(*hash);
-			*hash = NULL;
+			r = PTR_ERR(*ahash);
+			*ahash = NULL;
 			return r;
 		}
+		ahash_driver_name = crypto_ahash_driver_name(*ahash);
+	}
 
-		if (a->key) {
-			r = crypto_shash_setkey(*hash, a->key, a->key_size);
-			if (r) {
-				*error = error_key;
-				return r;
+	if (shash) {
+		*shash = crypto_alloc_shash(a->alg_string, 0, CRYPTO_ALG_ALLOCATES_MEMORY);
+		if (IS_ERR(*shash) && !ahash_driver_name) {
+			*error = error_alg;
+			r = PTR_ERR(*shash);
+			*shash = NULL;
+			return r;
+		}
+		if (!IS_ERR(*shash) && ahash_driver_name) {
+			if (strcmp(crypto_shash_driver_name(*shash), ahash_driver_name)) {
+				/*
+				 * ahash gave a different driver than shash, so probably
+				 * this is a case of real hardware offload.  Use ahash.
+				 */
+				crypto_free_shash(*shash);
+				*shash = NULL;
+			} else {
+				/* ahash and shash are same driver. Use shash. */
+				crypto_free_ahash(*ahash);
+				*ahash = NULL;
 			}
-		} else if (crypto_shash_get_flags(*hash) & CRYPTO_TFM_NEED_KEY) {
+		}
+		if (IS_ERR(*shash))
+			*shash = NULL;
+	}
+
+	/* either *ahash or *shash is set now */
+
+	if (a->key) {
+		r = shash && *shash ?
+			crypto_shash_setkey(*shash, a->key, a->key_size) :
+			crypto_ahash_setkey(*ahash, a->key, a->key_size);
+		if (r) {
 			*error = error_key;
-			return -ENOKEY;
+			return r;
 		}
+	} else if ((shash && *shash ?
+		    crypto_shash_get_flags(*shash) :
+		    crypto_ahash_get_flags(*ahash))
+		   & CRYPTO_TFM_NEED_KEY) {
+		*error = error_key;
+		return -ENOKEY;
 	}
 
+	pr_debug("Using %s %s (driver name %s)\n",
+		 ahash && *ahash ? "ahash" : "shash",
+		 a->alg_string,
+		 ahash && *ahash ?
+		 crypto_ahash_driver_name(*ahash) :
+		 crypto_shash_driver_name(*shash));
+
 	return 0;
 }
 
@@ -4692,20 +4829,35 @@ static int dm_integrity_ctr(struct dm_target *ti, unsigned int argc, char **argv
 		buffer_sectors = 1;
 	ic->log2_buffer_sectors = min((int)__fls(buffer_sectors), 31 - SECTOR_SHIFT);
 
-	r = get_mac(&ic->internal_hash, &ic->internal_hash_alg, &ti->error,
+	r = get_mac(&ic->internal_shash, &ic->internal_ahash,
+		    &ic->internal_hash_alg, &ti->error,
 		    "Invalid internal hash", "Error setting internal hash key");
 	if (r)
 		goto bad;
-	if (ic->internal_hash)
-		ic->internal_hash_digestsize = crypto_shash_digestsize(ic->internal_hash);
+	ic->have_internal_hash = ic->internal_shash || ic->internal_ahash;
+	if (ic->internal_ahash)
+		ti->per_io_data_size += sizeof(struct ahash_request) +
+					crypto_ahash_reqsize(ic->internal_ahash);
+	if (ic->have_internal_hash)
+		ic->internal_hash_digestsize = ic->internal_shash ?
+			crypto_shash_digestsize(ic->internal_shash) :
+			crypto_ahash_digestsize(ic->internal_ahash);
+
+	ic->dummy_dio = kmalloc(ti->per_io_data_size, GFP_KERNEL);
+	if (!ic->dummy_dio) {
+		ti->error = "Cannot allocate dummy dio";
+		r = -ENOMEM;
+		goto bad;
+	}
+	ic->dummy_dio->ic = ic;
 
-	r = get_mac(&ic->journal_mac, &ic->journal_mac_alg, &ti->error,
+	r = get_mac(&ic->journal_mac, NULL, &ic->journal_mac_alg, &ti->error,
 		    "Invalid journal mac", "Error setting journal mac key");
 	if (r)
 		goto bad;
 
 	if (!ic->tag_size) {
-		if (!ic->internal_hash) {
+		if (!ic->have_internal_hash) {
 			ti->error = "Unknown tag size";
 			r = -EINVAL;
 			goto bad;
@@ -4769,13 +4921,13 @@ static int dm_integrity_ctr(struct dm_target *ti, unsigned int argc, char **argv
 		}
 	}
 
-	if (ic->mode == 'B' && !ic->internal_hash) {
+	if (ic->mode == 'B' && !ic->have_internal_hash) {
 		r = -EINVAL;
 		ti->error = "Bitmap mode can be only used with internal hash";
 		goto bad;
 	}
 
-	if (ic->discard && !ic->internal_hash) {
+	if (ic->discard && !ic->have_internal_hash) {
 		r = -EINVAL;
 		ti->error = "Discard can be only used with internal hash";
 		goto bad;
@@ -5025,7 +5177,7 @@ static int dm_integrity_ctr(struct dm_target *ti, unsigned int argc, char **argv
 		ic->sb->recalc_sector = cpu_to_le64(0);
 	}
 
-	if (ic->internal_hash) {
+	if (ic->have_internal_hash) {
 		ic->recalc_wq = alloc_workqueue("dm-integrity-recalc", WQ_MEM_RECLAIM, 1);
 		if (!ic->recalc_wq) {
 			ti->error = "Cannot allocate workqueue";
@@ -5219,8 +5371,10 @@ static void dm_integrity_dtr(struct dm_target *ti)
 	if (ic->sb)
 		free_pages_exact(ic->sb, SB_SECTORS << SECTOR_SHIFT);
 
-	if (ic->internal_hash)
-		crypto_free_shash(ic->internal_hash);
+	if (ic->internal_shash)
+		crypto_free_shash(ic->internal_shash);
+	if (ic->internal_ahash)
+		crypto_free_ahash(ic->internal_ahash);
 	free_alg(&ic->internal_hash_alg);
 
 	if (ic->journal_crypt)
@@ -5231,13 +5385,15 @@ static void dm_integrity_dtr(struct dm_target *ti)
 		crypto_free_shash(ic->journal_mac);
 	free_alg(&ic->journal_mac_alg);
 
+	kfree(ic->dummy_dio);
+
 	kfree(ic);
 	dm_audit_log_dtr(DM_MSG_PREFIX, ti, 1);
 }
 
 static struct target_type integrity_target = {
 	.name			= "integrity",
-	.version		= {1, 13, 0},
+	.version		= {1, 14, 0},
 	.module			= THIS_MODULE,
 	.features		= DM_TARGET_SINGLETON | DM_TARGET_INTEGRITY,
 	.ctr			= dm_integrity_ctr,
-- 
2.43.0

Re: [PATCH v5 0/2] dm-integrity: Implement asynch digest support

[Mikulas Patocka]

On Tue, 22 Jul 2025, Harald Freudenberger wrote:

> Support for ahashes in dm-integrity.
> 
> Changelog:
> 
> v1: First implementation. Tested with crc32, sha256, hmac-sha256 and
>     the s390 specific implementations for hmac-sha256 and protected
>     key phmac-sha256. Also ran with some instrumented code (in the digest
>     implementation) to verify that in fact now the code runs asynchronous.
> v2: Support shash and ahash. Based on Mikulas' idea about implementing
>     ahash support similar to dm-verity this version now adds support
>     for ahash but does not replace the shash support. For more details
>     see the text of the patch header.
> v3: The line to store the digestsize into the new internal variable
>     did not make it into the patch set which was sent out. So now
>     this important code piece is also there. Also rebuilded, sparse
>     checked and tested to make sure the patches are ok.
> v4: Thanks to Mikulas a total new implementation of the ahash support
>     for the dm-integrity layer :-)
> v5: Slight rework around the allocation and comparing of ahash and
>     shash algorithm.
>     V5 has been tested with the new introduced ahash phmac which is a
>     protected key ("hardware key") version of a hmac for s390. As of now
>     phmac is only available in Herbert Xu's cryptodev-2.6 kernel tree
>     but will be merged into mainline with the next merge window for
>     the 6.17 development kernel.
> 
> Mikulas Patocka (2):
>   dm-integrity: use internal variable for digestsize
>   dm-integrity: introduce ahash support for the internal hash
> 
>  drivers/md/dm-integrity.c | 370 +++++++++++++++++++++++++++-----------
>  1 file changed, 265 insertions(+), 105 deletions(-)
> 
> 
> base-commit: 89be9a83ccf1f88522317ce02f854f30d6115c41
> --
> 2.43.0
> 

Hi

Eric Biggers recently removed ahash support from dm-verity - see this 
commit:
https://kernel.googlesource.com/pub/scm/linux/kernel/git/device-mapper/linux-dm/+/f43309c6743257244f11f14d31c297ee6a410ded

Should I revert Eric's patch? - would you need dm-verity with asynchronous 
hashes on zseries too?

Is this patch series needed for performance (does it perform better than 
the in-cpu instructions)? Or is it need because of better security (the 
keys are hidden in the hardware)?

Mikulas

Re: [PATCH v5 0/2] dm-integrity: Implement asynch digest support

[Harald Freudenberger]

On 2025-07-24 16:40, Mikulas Patocka wrote:
> On Tue, 22 Jul 2025, Harald Freudenberger wrote:
> 
>> Support for ahashes in dm-integrity.
>> 
>> Changelog:
>> 
>> v1: First implementation. Tested with crc32, sha256, hmac-sha256 and
>>     the s390 specific implementations for hmac-sha256 and protected
>>     key phmac-sha256. Also ran with some instrumented code (in the 
>> digest
>>     implementation) to verify that in fact now the code runs 
>> asynchronous.
>> v2: Support shash and ahash. Based on Mikulas' idea about implementing
>>     ahash support similar to dm-verity this version now adds support
>>     for ahash but does not replace the shash support. For more details
>>     see the text of the patch header.
>> v3: The line to store the digestsize into the new internal variable
>>     did not make it into the patch set which was sent out. So now
>>     this important code piece is also there. Also rebuilded, sparse
>>     checked and tested to make sure the patches are ok.
>> v4: Thanks to Mikulas a total new implementation of the ahash support
>>     for the dm-integrity layer :-)
>> v5: Slight rework around the allocation and comparing of ahash and
>>     shash algorithm.
>>     V5 has been tested with the new introduced ahash phmac which is a
>>     protected key ("hardware key") version of a hmac for s390. As of 
>> now
>>     phmac is only available in Herbert Xu's cryptodev-2.6 kernel tree
>>     but will be merged into mainline with the next merge window for
>>     the 6.17 development kernel.
>> 
>> Mikulas Patocka (2):
>>   dm-integrity: use internal variable for digestsize
>>   dm-integrity: introduce ahash support for the internal hash
>> 
>>  drivers/md/dm-integrity.c | 370 
>> +++++++++++++++++++++++++++-----------
>>  1 file changed, 265 insertions(+), 105 deletions(-)
>> 
>> 
>> base-commit: 89be9a83ccf1f88522317ce02f854f30d6115c41
>> --
>> 2.43.0
>> 
> 
> Hi
> 
> Eric Biggers recently removed ahash support from dm-verity - see this
> commit:
> https://kernel.googlesource.com/pub/scm/linux/kernel/git/device-mapper/linux-dm/+/f43309c6743257244f11f14d31c297ee6a410ded
> 
> Should I revert Eric's patch? - would you need dm-verity with 
> asynchronous
> hashes on zseries too?
> 
> Is this patch series needed for performance (does it perform better 
> than
> the in-cpu instructions)? Or is it need because of better security (the
> keys are hidden in the hardware)?
> 
> Mikulas

I've seen this. Well as of now we don't need dm-verity. However, I'll 
check
our plans and let you know within the next days.

Thanks
Harald Freudenberger
with

Re: [PATCH v5 0/2] dm-integrity: Implement asynch digest support

[Eric Biggers]

On Fri, Jul 25, 2025 at 10:14:30AM +0200, Harald Freudenberger wrote:
> On 2025-07-24 16:40, Mikulas Patocka wrote:
> > On Tue, 22 Jul 2025, Harald Freudenberger wrote:
> > 
> > > Support for ahashes in dm-integrity.
> > > 
> > > Changelog:
> > > 
> > > v1: First implementation. Tested with crc32, sha256, hmac-sha256 and
> > >     the s390 specific implementations for hmac-sha256 and protected
> > >     key phmac-sha256. Also ran with some instrumented code (in the
> > > digest
> > >     implementation) to verify that in fact now the code runs
> > > asynchronous.
> > > v2: Support shash and ahash. Based on Mikulas' idea about implementing
> > >     ahash support similar to dm-verity this version now adds support
> > >     for ahash but does not replace the shash support. For more details
> > >     see the text of the patch header.
> > > v3: The line to store the digestsize into the new internal variable
> > >     did not make it into the patch set which was sent out. So now
> > >     this important code piece is also there. Also rebuilded, sparse
> > >     checked and tested to make sure the patches are ok.
> > > v4: Thanks to Mikulas a total new implementation of the ahash support
> > >     for the dm-integrity layer :-)
> > > v5: Slight rework around the allocation and comparing of ahash and
> > >     shash algorithm.
> > >     V5 has been tested with the new introduced ahash phmac which is a
> > >     protected key ("hardware key") version of a hmac for s390. As of
> > > now
> > >     phmac is only available in Herbert Xu's cryptodev-2.6 kernel tree
> > >     but will be merged into mainline with the next merge window for
> > >     the 6.17 development kernel.
> > > 
> > > Mikulas Patocka (2):
> > >   dm-integrity: use internal variable for digestsize
> > >   dm-integrity: introduce ahash support for the internal hash
> > > 
> > >  drivers/md/dm-integrity.c | 370
> > > +++++++++++++++++++++++++++-----------
> > >  1 file changed, 265 insertions(+), 105 deletions(-)
> > > 
> > > 
> > > base-commit: 89be9a83ccf1f88522317ce02f854f30d6115c41
> > > --
> > > 2.43.0
> > > 
> > 
> > Hi
> > 
> > Eric Biggers recently removed ahash support from dm-verity - see this
> > commit:
> > https://kernel.googlesource.com/pub/scm/linux/kernel/git/device-mapper/linux-dm/+/f43309c6743257244f11f14d31c297ee6a410ded
> > 
> > Should I revert Eric's patch? - would you need dm-verity with
> > asynchronous
> > hashes on zseries too?
> > 
> > Is this patch series needed for performance (does it perform better than
> > the in-cpu instructions)? Or is it need because of better security (the
> > keys are hidden in the hardware)?
> > 
> > Mikulas
> 
> I've seen this. Well as of now we don't need dm-verity. However, I'll check
> our plans and let you know within the next days.
> 
> Thanks

Isn't your use case the "s390 specific protected key hash phmac"
(https://lore.kernel.org/linux-crypto/20250617134440.48000-1-freude@linux.ibm.com/)?
dm-verity uses an unkeyed hash, so that isn't applicable there.

BTW, did you consider a lib/crypto/ API for phmac?  I suspect it could
be much simpler than the asynchronous hash based version.

- Eric

Re: [PATCH v5 0/2] dm-integrity: Implement asynch digest support

[Harald Freudenberger]

On 2025-07-25 19:38, Eric Biggers wrote:
> On Fri, Jul 25, 2025 at 10:14:30AM +0200, Harald Freudenberger wrote:
>> On 2025-07-24 16:40, Mikulas Patocka wrote:
>> > On Tue, 22 Jul 2025, Harald Freudenberger wrote:
>> >
>> > > Support for ahashes in dm-integrity.
>> > >
>> > > Changelog:
>> > >
>> > > v1: First implementation. Tested with crc32, sha256, hmac-sha256 and
>> > >     the s390 specific implementations for hmac-sha256 and protected
>> > >     key phmac-sha256. Also ran with some instrumented code (in the
>> > > digest
>> > >     implementation) to verify that in fact now the code runs
>> > > asynchronous.
>> > > v2: Support shash and ahash. Based on Mikulas' idea about implementing
>> > >     ahash support similar to dm-verity this version now adds support
>> > >     for ahash but does not replace the shash support. For more details
>> > >     see the text of the patch header.
>> > > v3: The line to store the digestsize into the new internal variable
>> > >     did not make it into the patch set which was sent out. So now
>> > >     this important code piece is also there. Also rebuilded, sparse
>> > >     checked and tested to make sure the patches are ok.
>> > > v4: Thanks to Mikulas a total new implementation of the ahash support
>> > >     for the dm-integrity layer :-)
>> > > v5: Slight rework around the allocation and comparing of ahash and
>> > >     shash algorithm.
>> > >     V5 has been tested with the new introduced ahash phmac which is a
>> > >     protected key ("hardware key") version of a hmac for s390. As of
>> > > now
>> > >     phmac is only available in Herbert Xu's cryptodev-2.6 kernel tree
>> > >     but will be merged into mainline with the next merge window for
>> > >     the 6.17 development kernel.
>> > >
>> > > Mikulas Patocka (2):
>> > >   dm-integrity: use internal variable for digestsize
>> > >   dm-integrity: introduce ahash support for the internal hash
>> > >
>> > >  drivers/md/dm-integrity.c | 370
>> > > +++++++++++++++++++++++++++-----------
>> > >  1 file changed, 265 insertions(+), 105 deletions(-)
>> > >
>> > >
>> > > base-commit: 89be9a83ccf1f88522317ce02f854f30d6115c41
>> > > --
>> > > 2.43.0
>> > >
>> >
>> > Hi
>> >
>> > Eric Biggers recently removed ahash support from dm-verity - see this
>> > commit:
>> > https://kernel.googlesource.com/pub/scm/linux/kernel/git/device-mapper/linux-dm/+/f43309c6743257244f11f14d31c297ee6a410ded
>> >
>> > Should I revert Eric's patch? - would you need dm-verity with
>> > asynchronous
>> > hashes on zseries too?
>> >
>> > Is this patch series needed for performance (does it perform better than
>> > the in-cpu instructions)? Or is it need because of better security (the
>> > keys are hidden in the hardware)?
>> >
>> > Mikulas
>> 
>> I've seen this. Well as of now we don't need dm-verity. However, I'll 
>> check
>> our plans and let you know within the next days.
>> 
>> Thanks
> 
> Isn't your use case the "s390 specific protected key hash phmac"
> (https://lore.kernel.org/linux-crypto/20250617134440.48000-1-freude@linux.ibm.com/)?
> dm-verity uses an unkeyed hash, so that isn't applicable there.
> 

Yes, I've also found this out. For our purpose it is enough to have
dm-integrity with phmac support - dm-verity is fine with the 
(synchronous)
s390 sha implementations and so no need for asynchronous support.

> BTW, did you consider a lib/crypto/ API for phmac?  I suspect it could
> be much simpler than the asynchronous hash based version.

This is an option for the future. However, as of now I did not 
investigate
in how to exactly implement this as a lib. Maybe a work item for the 
next
months...

> 
> - Eric

Thanks for your feedback and hints
Harald Freudenberger

Re: [PATCH v5 0/2] dm-integrity: Implement asynch digest support

[Mikulas Patocka]

On Mon, 28 Jul 2025, Harald Freudenberger wrote:

> > > > > Mikulas Patocka (2):
> > > > >   dm-integrity: use internal variable for digestsize
> > > > >   dm-integrity: introduce ahash support for the internal hash
> > > > >
> > > > >  drivers/md/dm-integrity.c | 370
> > > > > +++++++++++++++++++++++++++-----------
> > > > >  1 file changed, 265 insertions(+), 105 deletions(-)

Hi

I reviewed the code and unfortunatelly I found out that it is buggy (I 
didn't notice the bugs before, when I was working on it).

integrity_ahash_sector_checksum uses sg_set_buf and sg_set_buf needs 
directly mapped memory. It doesn't work with vmalloc memory and it doesn't 
work with memory obtained from kmap.

The result buffer may be vmallocated memory (if we have 
CONFIG_VMAP_STACK).

I'd like to ask the maintainers of crypto-api - Herbert or David - what if 
I pass vmalloc-allocated memory (or possibly stack memory) as "result" to 
ahash_request_set_crypt(req, sg, result, nbytes);? Is it legal or not?

Mikulas

Re: [PATCH v5 0/2] dm-integrity: Implement asynch digest support

[Herbert Xu]

On Mon, Jul 28, 2025 at 11:24:25PM +0200, Mikulas Patocka wrote:
>
> I'd like to ask the maintainers of crypto-api - Herbert or David - what if 
> I pass vmalloc-allocated memory (or possibly stack memory) as "result" to 
> ahash_request_set_crypt(req, sg, result, nbytes);? Is it legal or not?

It's perfectly legal to pass a stack-allocated or vmallc pointer
for the result field in ahash.

Only SG lists require DMA memory.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH v5 0/2] dm-integrity: Implement asynch digest support

[Harald Freudenberger]

On 2025-07-28 23:24, Mikulas Patocka wrote:
> On Mon, 28 Jul 2025, Harald Freudenberger wrote:
> 
>> > > > > Mikulas Patocka (2):
>> > > > >   dm-integrity: use internal variable for digestsize
>> > > > >   dm-integrity: introduce ahash support for the internal hash
>> > > > >
>> > > > >  drivers/md/dm-integrity.c | 370
>> > > > > +++++++++++++++++++++++++++-----------
>> > > > >  1 file changed, 265 insertions(+), 105 deletions(-)
> 
> Hi
> 
> I reviewed the code and unfortunatelly I found out that it is buggy (I
> didn't notice the bugs before, when I was working on it).
> 
> integrity_ahash_sector_checksum uses sg_set_buf and sg_set_buf needs
> directly mapped memory. It doesn't work with vmalloc memory and it 
> doesn't
> work with memory obtained from kmap.
> 
> The result buffer may be vmallocated memory (if we have
> CONFIG_VMAP_STACK).
> 
> I'd like to ask the maintainers of crypto-api - Herbert or David - what 
> if
> I pass vmalloc-allocated memory (or possibly stack memory) as "result" 
> to
> ahash_request_set_crypt(req, sg, result, nbytes);? Is it legal or not?
> 
> Mikulas

Hi Mikulas, what is the state of the dm-itegrity asynch digest support?
If you have a newer version or maybe if you need some more code 
coverage,
I do offer some worktime for this and a s390 machine...

Thanks for your work
Harald

Re: [PATCH v5 0/2] dm-integrity: Implement asynch digest support

[Mikulas Patocka]

On Thu, 14 Aug 2025, Harald Freudenberger wrote:

> On 2025-07-28 23:24, Mikulas Patocka wrote:
> > On Mon, 28 Jul 2025, Harald Freudenberger wrote:
> > 
> > > > > > > Mikulas Patocka (2):
> > > > > > >   dm-integrity: use internal variable for digestsize
> > > > > > >   dm-integrity: introduce ahash support for the internal hash
> > > > > > >
> > > > > > >  drivers/md/dm-integrity.c | 370
> > > > > > > +++++++++++++++++++++++++++-----------
> > > > > > >  1 file changed, 265 insertions(+), 105 deletions(-)
> > 
> > Hi
> > 
> > I reviewed the code and unfortunatelly I found out that it is buggy (I
> > didn't notice the bugs before, when I was working on it).
> > 
> > integrity_ahash_sector_checksum uses sg_set_buf and sg_set_buf needs
> > directly mapped memory. It doesn't work with vmalloc memory and it doesn't
> > work with memory obtained from kmap.
> > 
> > The result buffer may be vmallocated memory (if we have
> > CONFIG_VMAP_STACK).
> > 
> > I'd like to ask the maintainers of crypto-api - Herbert or David - what if
> > I pass vmalloc-allocated memory (or possibly stack memory) as "result" to
> > ahash_request_set_crypt(req, sg, result, nbytes);? Is it legal or not?
> > 
> > Mikulas
> 
> Hi Mikulas, what is the state of the dm-itegrity asynch digest support?
> If you have a newer version or maybe if you need some more code coverage,
> I do offer some worktime for this and a s390 machine...
> 
> Thanks for your work
> Harald

Hi

I'm trying to find a way how to implement it without bloating the code too 
much. Maybe I'll have it this week (if it passes the tests).

Mikulas