* [PATCH v2 0/1] dm-integrity: replace forgeable discard filler with a keyed sector marker
@ 2026-07-28 14:33 Shukai Ni
2026-07-28 14:33 ` [PATCH v2 1/1] " Shukai Ni
0 siblings, 1 reply; 4+ messages in thread
From: Shukai Ni @ 2026-07-28 14:33 UTC (permalink / raw)
To: mpatocka, agk, snitzer
Cc: bmarzins, gmazyland, dm-devel, linux-kernel, Shukai Ni
dm-integrity's discard support has a keyless bypass. When a device is
opened with --allow-discards, dm_integrity_rw_tag() treats a stored
tag consisting entirely of the constant DISCARD_FILLER (0xf6) as
sufficient proof that the corresponding block was discarded, and
skips the keyed integrity check for it. That constant is public and
needs no key to reproduce, so anyone able to write raw sectors to the
backing device can stamp an arbitrary data block with an all-0xf6 tag and
have its contents served as if they had passed HMAC verification.
The exposure is limited to dm-integrity's standalone cryptographic
authentication mode: discard is only accepted when internal_hash is
configured, so the combined dm-crypt + dm-integrity AEAD setup, where
dm-crypt supplies the tags itself, is not affected.
v1 replaced the constant filler unconditionally with a per-sector
keyed marker: HMAC_key(salt || sector). A real data tag is
HMAC_key(salt || sector || block); because its input always includes
the full block while the discard marker's never does, the two cannot
structurally collide, regardless of block content.
v2 makes the fix opt-in instead. A new "allow_discards_keyed" target
argument enables the keyed marker. Once "allow_discards_keyed" is
used, SB_FLAG_DISCARD_KEYED is set in the superblock and persists
across future activations.
Changes since v1:
- Added "allow_discards_keyed" as a new, explicit opt-in target
argument instead of always replacing the discard marker scheme.
- Added SB_FLAG_DISCARD_KEYED, a sticky superblock flag recording the
choice, following the same persistence pattern as
SB_FLAG_RECALCULATING.
- Restored the original DISCARD_FILLER-based marker as the default
behavior, so existing volumes are unaffected unless the admin
explicitly opts in.
- Documented the new option, its danger, and its formula in
dm-integrity.rst; updated the status-string grammar in dm-ima.rst.
Shukai Ni (1):
dm-integrity: replace forgeable discard filler with a keyed sector
marker
.../admin-guide/device-mapper/dm-ima.rst | 7 +-
.../device-mapper/dm-integrity.rst | 13 ++
drivers/md/dm-integrity.c | 122 +++++++++++++++---
3 files changed, 121 insertions(+), 21 deletions(-)
--
2.53.0
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH v2 1/1] dm-integrity: replace forgeable discard filler with a keyed sector marker 2026-07-28 14:33 [PATCH v2 0/1] dm-integrity: replace forgeable discard filler with a keyed sector marker Shukai Ni @ 2026-07-28 14:33 ` Shukai Ni 2026-07-30 15:29 ` Mikulas Patocka 0 siblings, 1 reply; 4+ messages in thread From: Shukai Ni @ 2026-07-28 14:33 UTC (permalink / raw) To: mpatocka, agk, snitzer Cc: bmarzins, gmazyland, dm-devel, linux-kernel, Shukai Ni, Jo Van Bulck The discard-block check in dm_integrity_rw_tag() treats a stored tag of all 0xf6 bytes (DISCARD_FILLER) as proof a block was discarded and skips HMAC verification. allow_discards is only accepted in dm-integrity's standalone mode. An attacker with raw write access to the backing device, but without the integrity key, can stamp any block with an all-0xf6 tag and have it served as authentic. Add a new "allow_discards_keyed" target argument that marks discarded blocks with a keyed checksum of (salt || sector) instead, computed by integrity_discard_checksum(). Fixes: 84597a44a9d8 ("dm integrity: add optional discard support") Co-developed-by: Jo Van Bulck <jo.vanbulck@cs.kuleuven.be> Signed-off-by: Jo Van Bulck <jo.vanbulck@cs.kuleuven.be> Signed-off-by: Shukai Ni <shukai.ni@kuleuven.be> --- .../admin-guide/device-mapper/dm-ima.rst | 7 +- .../device-mapper/dm-integrity.rst | 13 ++ drivers/md/dm-integrity.c | 122 +++++++++++++++--- 3 files changed, 121 insertions(+), 21 deletions(-) diff --git a/Documentation/admin-guide/device-mapper/dm-ima.rst b/Documentation/admin-guide/device-mapper/dm-ima.rst index a4aa50a82..2a3b50ffb 100644 --- a/Documentation/admin-guide/device-mapper/dm-ima.rst +++ b/Documentation/admin-guide/device-mapper/dm-ima.rst @@ -424,7 +424,8 @@ section above) has the following data format for 'integrity' target. target_attributes := <target_name> "," <target_version> "," <dev_name> "," <start> <tag_size> "," <mode> "," [<meta_device> ","] [<block_size> ","] <recalculate> "," - <allow_discards> "," <fix_padding> "," <fix_hmac> "," <legacy_recalculate> "," + <allow_discards> "," <allow_discards_keyed> "," <fix_padding> "," <fix_hmac> "," + <legacy_recalculate> "," <journal_sectors> "," <interleave_sectors> "," <buffer_sectors> ";" target_name := "target_name=integrity" @@ -438,6 +439,7 @@ section above) has the following data format for 'integrity' target. block_size := "block_size=" <N> recalculate := "recalculate=" <yes_no> allow_discards := "allow_discards=" <yes_no> + allow_discards_keyed := "allow_discards_keyed=" <yes_no> fix_padding := "fix_padding=" <yes_no> fix_hmac := "fix_hmac=" <yes_no> legacy_recalculate := "legacy_recalculate=" <yes_no> @@ -455,7 +457,8 @@ section above) has the following data format for 'integrity' target. dm_version=4.45.0; name=integrity1,uuid=,major=253,minor=1,minor_count=1,num_targets=1; target_index=0,target_begin=0,target_len=7856,target_name=integrity,target_version=1.10.0, - dev_name=253:0,start=0,tag_size=32,mode=J,recalculate=n,allow_discards=n,fix_padding=n, + dev_name=253:0,start=0,tag_size=32,mode=J,recalculate=n,allow_discards=n, + allow_discards_keyed=n,fix_padding=n, fix_hmac=n,legacy_recalculate=n,journal_sectors=88,interleave_sectors=32768,buffer_sectors=128; diff --git a/Documentation/admin-guide/device-mapper/dm-integrity.rst b/Documentation/admin-guide/device-mapper/dm-integrity.rst index c2e18ecc0..9c2130142 100644 --- a/Documentation/admin-guide/device-mapper/dm-integrity.rst +++ b/Documentation/admin-guide/device-mapper/dm-integrity.rst @@ -190,6 +190,19 @@ allow_discards Allow block discard requests (a.k.a. TRIM) for the integrity device. Discards are only allowed to devices using internal hash. + A discarded block is marked with a constant filler tag that anyone + with raw write access to the backing device can forge without the + key. Use allow_discards_keyed instead on new volumes. + +allow_discards_keyed + Like allow_discards, but marks a discarded block with a keyed + checksum of the sector number, HMAC_key(salt || sector), instead of + the constant filler tag, so it can't be forged without the + integrity key. + + Not compatible with volumes that already have discarded blocks + marked the old way; only use on a freshly formatted volume. + fix_padding Use a smaller padding of the tag area that is more space-efficient. If this option is not present, large padding is diff --git a/drivers/md/dm-integrity.c b/drivers/md/dm-integrity.c index 06e805902..03514eb27 100644 --- a/drivers/md/dm-integrity.c +++ b/drivers/md/dm-integrity.c @@ -91,6 +91,7 @@ struct superblock { #define SB_FLAG_FIXED_PADDING 0x8 #define SB_FLAG_FIXED_HMAC 0x10 #define SB_FLAG_INLINE 0x20 +#define SB_FLAG_DISCARD_KEYED 0x40 #define JOURNAL_ENTRY_ROUNDUP 8 @@ -277,6 +278,7 @@ struct dm_integrity_c { bool recalculate_flag; bool reset_recalculate_flag; bool discard; + bool discard_keyed; bool fix_padding; bool fix_hmac; bool legacy_recalculate; @@ -1414,7 +1416,8 @@ static int dm_integrity_rw_tag(struct dm_integrity_c *ic, unsigned char *tag, se { unsigned int hash_offset = 0; unsigned char mismatch_hash = 0; - unsigned char mismatch_filler = !ic->discard; + bool discard_keyed = ic->sb->flags & cpu_to_le32(SB_FLAG_DISCARD_KEYED); + unsigned char mismatch_filler = !ic->discard || discard_keyed; do { unsigned char *data, *dp; @@ -1466,7 +1469,7 @@ static int dm_integrity_rw_tag(struct dm_integrity_c *ic, unsigned char *tag, se } hash_offset = 0; mismatch_hash = 0; - mismatch_filler = !ic->discard; + mismatch_filler = !ic->discard || discard_keyed; } } } @@ -1645,7 +1648,8 @@ static void integrity_end_io(struct bio *bio) } static void integrity_sector_checksum_shash(struct dm_integrity_c *ic, sector_t sector, - const char *data, unsigned offset, char *result) + const char *data, unsigned offset, + unsigned int len, char *result) { __le64 sector_le = cpu_to_le64(sector); SHASH_DESC_ON_STACK(req, ic->internal_shash); @@ -1674,10 +1678,12 @@ static void integrity_sector_checksum_shash(struct dm_integrity_c *ic, sector_t goto failed; } - r = crypto_shash_update(req, data + offset, ic->sectors_per_block << SECTOR_SHIFT); - if (unlikely(r < 0)) { - dm_integrity_io_error(ic, "crypto_shash_update", r); - goto failed; + if (likely(len)) { + r = crypto_shash_update(req, data + offset, len); + if (unlikely(r < 0)) { + dm_integrity_io_error(ic, "crypto_shash_update", r); + goto failed; + } } r = crypto_shash_final(req, result); @@ -1698,7 +1704,8 @@ static void integrity_sector_checksum_shash(struct dm_integrity_c *ic, sector_t } static void integrity_sector_checksum_ahash(struct dm_integrity_c *ic, struct ahash_request **ahash_req, - sector_t sector, struct page *page, unsigned offset, char *result) + sector_t sector, struct page *page, unsigned offset, + unsigned int len, char *result) { __le64 sector_le = cpu_to_le64(sector); struct ahash_request *req; @@ -1707,6 +1714,7 @@ static void integrity_sector_checksum_ahash(struct dm_integrity_c *ic, struct ah int r; unsigned int digest_size; unsigned int nbytes = 0; + unsigned int nents = 1 + (len ? 1 : 0); might_sleep(); @@ -1720,12 +1728,12 @@ static void integrity_sector_checksum_ahash(struct dm_integrity_c *ic, struct ah ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_SLEEP, crypto_req_done, &wait); if (ic->sb->flags & cpu_to_le32(SB_FLAG_FIXED_HMAC)) { - sg_init_table(sg, 3); + sg_init_table(sg, nents + 1); sg_set_buf(s, (const __u8 *)&ic->sb->salt, SALT_SIZE); nbytes += SALT_SIZE; s++; } else { - sg_init_table(sg, 2); + sg_init_table(sg, nents); } if (likely(!is_vmalloc_addr(§or_le))) { @@ -1738,8 +1746,10 @@ static void integrity_sector_checksum_ahash(struct dm_integrity_c *ic, struct ah nbytes += sizeof(sector_le); s++; - sg_set_page(s, page, ic->sectors_per_block << SECTOR_SHIFT, offset); - nbytes += ic->sectors_per_block << SECTOR_SHIFT; + if (likely(len)) { + sg_set_page(s, page, len, offset); + nbytes += len; + } ahash_request_set_crypt(req, sg, result, nbytes); @@ -1763,10 +1773,40 @@ static void integrity_sector_checksum_ahash(struct dm_integrity_c *ic, struct ah static void integrity_sector_checksum(struct dm_integrity_c *ic, struct ahash_request **ahash_req, sector_t sector, const char *data, unsigned offset, char *result) { + unsigned int len = ic->sectors_per_block << SECTOR_SHIFT; + if (likely(ic->internal_shash != NULL)) - integrity_sector_checksum_shash(ic, sector, data, offset, result); + integrity_sector_checksum_shash(ic, sector, data, offset, len, result); else - integrity_sector_checksum_ahash(ic, ahash_req, sector, (struct page *)data, offset, result); + integrity_sector_checksum_ahash(ic, ahash_req, sector, (struct page *)data, + offset, len, result); +} + +/* + * Authenticated marker for a discarded block: HMAC_key(salt || sector), with + * no data payload. Because a real data tag's input always covers a full + * block, its length differs from this marker's, so the two can never + * collide structurally, regardless of block content. + */ +static void integrity_discard_checksum(struct dm_integrity_c *ic, struct ahash_request **ahash_req, + sector_t sector, char *result) +{ + if (likely(ic->internal_shash != NULL)) + integrity_sector_checksum_shash(ic, sector, NULL, 0, 0, result); + else + integrity_sector_checksum_ahash(ic, ahash_req, sector, NULL, 0, 0, result); +} + +static void integrity_discard_fill_tags(struct dm_integrity_c *ic, struct ahash_request **ahash_req, + unsigned char *checksums, sector_t *sector, + unsigned int blocks) +{ + unsigned int i; + + for (i = 0; i < blocks; i++) { + integrity_discard_checksum(ic, ahash_req, *sector, checksums + i * ic->tag_size); + *sector += ic->sectors_per_block; + } } static void *integrity_kmap(struct dm_integrity_c *ic, struct page *p) @@ -1795,6 +1835,29 @@ static void *integrity_identity(struct dm_integrity_c *ic, void *data) return virt_to_page(data); } +static int integrity_recheck_verify_tag(struct dm_integrity_io *dio, char *checksum, + char *on_disk_tag, sector_t logical_sector) +{ + struct dm_integrity_c *ic = dio->ic; + int r; + + if (!(ic->sb->flags & cpu_to_le32(SB_FLAG_DISCARD_KEYED))) + return dm_integrity_rw_tag(ic, checksum, &dio->metadata_block, + &dio->metadata_offset, ic->tag_size, TAG_CMP); + + r = dm_integrity_rw_tag(ic, on_disk_tag, &dio->metadata_block, + &dio->metadata_offset, ic->tag_size, TAG_READ); + if (unlikely(r)) + return r; + + r = crypto_memneq(on_disk_tag, checksum, ic->tag_size); + if (unlikely(r)) { + integrity_discard_checksum(ic, &dio->ahash_req, logical_sector, checksum); + r = crypto_memneq(on_disk_tag, checksum, ic->tag_size); + } + return r; +} + 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)); @@ -1820,6 +1883,7 @@ static noinline void integrity_recheck(struct dm_integrity_io *dio, char *checks char *mem; char *buffer = page_to_virt(page); unsigned int buffer_offset; + char on_disk_tag[MAX_T(size_t, HASH_MAX_DIGESTSIZE, MAX_TAG_SIZE)]; int r; struct dm_io_request io_req; struct dm_io_region io_loc; @@ -1847,8 +1911,8 @@ static noinline void integrity_recheck(struct dm_integrity_io *dio, char *checks } integrity_sector_checksum(ic, &dio->ahash_req, logical_sector, integrity_identity(ic, buffer), buffer_offset, checksum); - r = dm_integrity_rw_tag(ic, checksum, &dio->metadata_block, - &dio->metadata_offset, ic->tag_size, TAG_CMP); + r = integrity_recheck_verify_tag(dio, checksum, on_disk_tag, + logical_sector); if (r) { if (r > 0) { DMERR_LIMIT("%pg: Checksum failed at sector 0x%llx", @@ -1914,13 +1978,19 @@ static void integrity_metadata(struct work_struct *w) unsigned int bi_size = dio->bio_details.bi_iter.bi_size; unsigned int max_size = likely(checksums != checksums_onstack) ? PAGE_SIZE : HASH_MAX_DIGESTSIZE; unsigned int max_blocks = max_size / ic->tag_size; + sector_t sector = dio->range.logical_sector; + bool discard_keyed = ic->sb->flags & cpu_to_le32(SB_FLAG_DISCARD_KEYED); - memset(checksums, DISCARD_FILLER, max_size); + if (!discard_keyed) + memset(checksums, DISCARD_FILLER, max_size); while (bi_size) { unsigned int this_step_blocks = bi_size >> (SECTOR_SHIFT + ic->sb->log2_sectors_per_block); this_step_blocks = min(this_step_blocks, max_blocks); + if (discard_keyed) + integrity_discard_fill_tags(ic, &dio->ahash_req, checksums, + §or, this_step_blocks); r = dm_integrity_rw_tag(ic, checksums, &dio->metadata_block, &dio->metadata_offset, this_step_blocks * ic->tag_size, TAG_WRITE); if (unlikely(r)) { @@ -3795,7 +3865,7 @@ static void dm_integrity_resume(struct dm_target *ti) ic->wrote_to_journal = false; - flags = ic->sb->flags & cpu_to_le32(SB_FLAG_RECALCULATING); + flags = ic->sb->flags & cpu_to_le32(SB_FLAG_RECALCULATING | SB_FLAG_DISCARD_KEYED); r = sync_rw_sb(ic, REQ_OP_READ); if (r) dm_integrity_io_error(ic, "reading superblock", r); @@ -3955,6 +4025,7 @@ static void dm_integrity_status(struct dm_target *ti, status_type_t type, arg_count += !!ic->journal_mac_alg.alg_string; arg_count += (ic->sb->flags & cpu_to_le32(SB_FLAG_FIXED_PADDING)) != 0; arg_count += (ic->sb->flags & cpu_to_le32(SB_FLAG_FIXED_HMAC)) != 0; + arg_count += (ic->sb->flags & cpu_to_le32(SB_FLAG_DISCARD_KEYED)) != 0; arg_count += ic->legacy_recalculate; DMEMIT("%s %llu %u %c %u", ic->dev->name, ic->start, ic->tag_size, ic->mode, arg_count); @@ -3968,6 +4039,8 @@ static void dm_integrity_status(struct dm_target *ti, status_type_t type, DMEMIT(" reset_recalculate"); if (ic->discard) DMEMIT(" allow_discards"); + if ((ic->sb->flags & cpu_to_le32(SB_FLAG_DISCARD_KEYED)) != 0) + DMEMIT(" allow_discards_keyed"); if (ic->mode != 'I') DMEMIT(" interleave_sectors:%u", 1U << ic->sb->log2_interleave_sectors); DMEMIT(" buffer_sectors:%u", 1U << ic->log2_buffer_sectors); @@ -4017,6 +4090,8 @@ static void dm_integrity_status(struct dm_target *ti, status_type_t type, DMEMIT(",recalculate=%c", (ic->sb->flags & cpu_to_le32(SB_FLAG_RECALCULATING)) ? 'y' : 'n'); DMEMIT(",allow_discards=%c", ic->discard ? 'y' : 'n'); + DMEMIT(",allow_discards_keyed=%c", + ((ic->sb->flags & cpu_to_le32(SB_FLAG_DISCARD_KEYED)) != 0) ? 'y' : 'n'); DMEMIT(",fix_padding=%c", ((ic->sb->flags & cpu_to_le32(SB_FLAG_FIXED_PADDING)) != 0) ? 'y' : 'n'); DMEMIT(",fix_hmac=%c", @@ -4178,6 +4253,9 @@ static int initialize_superblock(struct dm_integrity_c *ic, get_random_bytes(ic->sb->salt, SALT_SIZE); } + if (ic->discard_keyed) + ic->sb->flags |= cpu_to_le32(SB_FLAG_DISCARD_KEYED); + if (!ic->meta_dev) { if (ic->fix_padding) ic->sb->flags |= cpu_to_le32(SB_FLAG_FIXED_PADDING); @@ -4835,6 +4913,9 @@ static int dm_integrity_ctr(struct dm_target *ti, unsigned int argc, char **argv ic->reset_recalculate_flag = true; } else if (!strcmp(opt_string, "allow_discards")) { ic->discard = true; + } else if (!strcmp(opt_string, "allow_discards_keyed")) { + ic->discard = true; + ic->discard_keyed = true; } else if (!strcmp(opt_string, "fix_padding")) { ic->fix_padding = true; } else if (!strcmp(opt_string, "fix_hmac")) { @@ -5212,6 +5293,9 @@ static int dm_integrity_ctr(struct dm_target *ti, unsigned int argc, char **argv ic->sb->recalc_sector = cpu_to_le64(0); } + if (ic->discard_keyed) + ic->sb->flags |= cpu_to_le32(SB_FLAG_DISCARD_KEYED); + if (ic->internal_hash) { ic->recalc_wq = alloc_workqueue("dm-integrity-recalc", WQ_MEM_RECLAIM | WQ_PERCPU, 1); @@ -5430,7 +5514,7 @@ static void dm_integrity_dtr(struct dm_target *ti) static struct target_type integrity_target = { .name = "integrity", - .version = {1, 14, 0}, + .version = {1, 15, 0}, .module = THIS_MODULE, .features = DM_TARGET_SINGLETON | DM_TARGET_INTEGRITY, .ctr = dm_integrity_ctr, -- 2.53.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2 1/1] dm-integrity: replace forgeable discard filler with a keyed sector marker 2026-07-28 14:33 ` [PATCH v2 1/1] " Shukai Ni @ 2026-07-30 15:29 ` Mikulas Patocka 2026-07-31 21:03 ` Milan Broz 0 siblings, 1 reply; 4+ messages in thread From: Mikulas Patocka @ 2026-07-30 15:29 UTC (permalink / raw) To: Shukai Ni Cc: agk, snitzer, bmarzins, gmazyland, dm-devel, linux-kernel, Jo Van Bulck Hi I accepted the patch (and made some small modifications to it). You can get the updated patch from git://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git branch: for-next Mikulas On Tue, 28 Jul 2026, Shukai Ni wrote: > The discard-block check in dm_integrity_rw_tag() treats a stored tag > of all 0xf6 bytes (DISCARD_FILLER) as proof a block was discarded and > skips HMAC verification. allow_discards is only accepted in > dm-integrity's standalone mode. An attacker with raw write access to > the backing device, but without the integrity key, can stamp any block > with an all-0xf6 tag and have it served as authentic. > > Add a new "allow_discards_keyed" target argument that marks discarded > blocks with a keyed checksum of (salt || sector) instead, computed by > integrity_discard_checksum(). > > Fixes: 84597a44a9d8 ("dm integrity: add optional discard support") > Co-developed-by: Jo Van Bulck <jo.vanbulck@cs.kuleuven.be> > Signed-off-by: Jo Van Bulck <jo.vanbulck@cs.kuleuven.be> > Signed-off-by: Shukai Ni <shukai.ni@kuleuven.be> > --- > .../admin-guide/device-mapper/dm-ima.rst | 7 +- > .../device-mapper/dm-integrity.rst | 13 ++ > drivers/md/dm-integrity.c | 122 +++++++++++++++--- > 3 files changed, 121 insertions(+), 21 deletions(-) > > diff --git a/Documentation/admin-guide/device-mapper/dm-ima.rst b/Documentation/admin-guide/device-mapper/dm-ima.rst > index a4aa50a82..2a3b50ffb 100644 > --- a/Documentation/admin-guide/device-mapper/dm-ima.rst > +++ b/Documentation/admin-guide/device-mapper/dm-ima.rst > @@ -424,7 +424,8 @@ section above) has the following data format for 'integrity' target. > > target_attributes := <target_name> "," <target_version> "," <dev_name> "," <start> > <tag_size> "," <mode> "," [<meta_device> ","] [<block_size> ","] <recalculate> "," > - <allow_discards> "," <fix_padding> "," <fix_hmac> "," <legacy_recalculate> "," > + <allow_discards> "," <allow_discards_keyed> "," <fix_padding> "," <fix_hmac> "," > + <legacy_recalculate> "," > <journal_sectors> "," <interleave_sectors> "," <buffer_sectors> ";" > > target_name := "target_name=integrity" > @@ -438,6 +439,7 @@ section above) has the following data format for 'integrity' target. > block_size := "block_size=" <N> > recalculate := "recalculate=" <yes_no> > allow_discards := "allow_discards=" <yes_no> > + allow_discards_keyed := "allow_discards_keyed=" <yes_no> > fix_padding := "fix_padding=" <yes_no> > fix_hmac := "fix_hmac=" <yes_no> > legacy_recalculate := "legacy_recalculate=" <yes_no> > @@ -455,7 +457,8 @@ section above) has the following data format for 'integrity' target. > dm_version=4.45.0; > name=integrity1,uuid=,major=253,minor=1,minor_count=1,num_targets=1; > target_index=0,target_begin=0,target_len=7856,target_name=integrity,target_version=1.10.0, > - dev_name=253:0,start=0,tag_size=32,mode=J,recalculate=n,allow_discards=n,fix_padding=n, > + dev_name=253:0,start=0,tag_size=32,mode=J,recalculate=n,allow_discards=n, > + allow_discards_keyed=n,fix_padding=n, > fix_hmac=n,legacy_recalculate=n,journal_sectors=88,interleave_sectors=32768,buffer_sectors=128; > > > diff --git a/Documentation/admin-guide/device-mapper/dm-integrity.rst b/Documentation/admin-guide/device-mapper/dm-integrity.rst > index c2e18ecc0..9c2130142 100644 > --- a/Documentation/admin-guide/device-mapper/dm-integrity.rst > +++ b/Documentation/admin-guide/device-mapper/dm-integrity.rst > @@ -190,6 +190,19 @@ allow_discards > Allow block discard requests (a.k.a. TRIM) for the integrity device. > Discards are only allowed to devices using internal hash. > > + A discarded block is marked with a constant filler tag that anyone > + with raw write access to the backing device can forge without the > + key. Use allow_discards_keyed instead on new volumes. > + > +allow_discards_keyed > + Like allow_discards, but marks a discarded block with a keyed > + checksum of the sector number, HMAC_key(salt || sector), instead of > + the constant filler tag, so it can't be forged without the > + integrity key. > + > + Not compatible with volumes that already have discarded blocks > + marked the old way; only use on a freshly formatted volume. > + > fix_padding > Use a smaller padding of the tag area that is more > space-efficient. If this option is not present, large padding is > diff --git a/drivers/md/dm-integrity.c b/drivers/md/dm-integrity.c > index 06e805902..03514eb27 100644 > --- a/drivers/md/dm-integrity.c > +++ b/drivers/md/dm-integrity.c > @@ -91,6 +91,7 @@ struct superblock { > #define SB_FLAG_FIXED_PADDING 0x8 > #define SB_FLAG_FIXED_HMAC 0x10 > #define SB_FLAG_INLINE 0x20 > +#define SB_FLAG_DISCARD_KEYED 0x40 > > #define JOURNAL_ENTRY_ROUNDUP 8 > > @@ -277,6 +278,7 @@ struct dm_integrity_c { > bool recalculate_flag; > bool reset_recalculate_flag; > bool discard; > + bool discard_keyed; > bool fix_padding; > bool fix_hmac; > bool legacy_recalculate; > @@ -1414,7 +1416,8 @@ static int dm_integrity_rw_tag(struct dm_integrity_c *ic, unsigned char *tag, se > { > unsigned int hash_offset = 0; > unsigned char mismatch_hash = 0; > - unsigned char mismatch_filler = !ic->discard; > + bool discard_keyed = ic->sb->flags & cpu_to_le32(SB_FLAG_DISCARD_KEYED); > + unsigned char mismatch_filler = !ic->discard || discard_keyed; > > do { > unsigned char *data, *dp; > @@ -1466,7 +1469,7 @@ static int dm_integrity_rw_tag(struct dm_integrity_c *ic, unsigned char *tag, se > } > hash_offset = 0; > mismatch_hash = 0; > - mismatch_filler = !ic->discard; > + mismatch_filler = !ic->discard || discard_keyed; > } > } > } > @@ -1645,7 +1648,8 @@ static void integrity_end_io(struct bio *bio) > } > > static void integrity_sector_checksum_shash(struct dm_integrity_c *ic, sector_t sector, > - const char *data, unsigned offset, char *result) > + const char *data, unsigned offset, > + unsigned int len, char *result) > { > __le64 sector_le = cpu_to_le64(sector); > SHASH_DESC_ON_STACK(req, ic->internal_shash); > @@ -1674,10 +1678,12 @@ static void integrity_sector_checksum_shash(struct dm_integrity_c *ic, sector_t > goto failed; > } > > - r = crypto_shash_update(req, data + offset, ic->sectors_per_block << SECTOR_SHIFT); > - if (unlikely(r < 0)) { > - dm_integrity_io_error(ic, "crypto_shash_update", r); > - goto failed; > + if (likely(len)) { > + r = crypto_shash_update(req, data + offset, len); > + if (unlikely(r < 0)) { > + dm_integrity_io_error(ic, "crypto_shash_update", r); > + goto failed; > + } > } > > r = crypto_shash_final(req, result); > @@ -1698,7 +1704,8 @@ static void integrity_sector_checksum_shash(struct dm_integrity_c *ic, sector_t > } > > static void integrity_sector_checksum_ahash(struct dm_integrity_c *ic, struct ahash_request **ahash_req, > - sector_t sector, struct page *page, unsigned offset, char *result) > + sector_t sector, struct page *page, unsigned offset, > + unsigned int len, char *result) > { > __le64 sector_le = cpu_to_le64(sector); > struct ahash_request *req; > @@ -1707,6 +1714,7 @@ static void integrity_sector_checksum_ahash(struct dm_integrity_c *ic, struct ah > int r; > unsigned int digest_size; > unsigned int nbytes = 0; > + unsigned int nents = 1 + (len ? 1 : 0); > > might_sleep(); > > @@ -1720,12 +1728,12 @@ static void integrity_sector_checksum_ahash(struct dm_integrity_c *ic, struct ah > ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_SLEEP, crypto_req_done, &wait); > > if (ic->sb->flags & cpu_to_le32(SB_FLAG_FIXED_HMAC)) { > - sg_init_table(sg, 3); > + sg_init_table(sg, nents + 1); > sg_set_buf(s, (const __u8 *)&ic->sb->salt, SALT_SIZE); > nbytes += SALT_SIZE; > s++; > } else { > - sg_init_table(sg, 2); > + sg_init_table(sg, nents); > } > > if (likely(!is_vmalloc_addr(§or_le))) { > @@ -1738,8 +1746,10 @@ static void integrity_sector_checksum_ahash(struct dm_integrity_c *ic, struct ah > nbytes += sizeof(sector_le); > s++; > > - sg_set_page(s, page, ic->sectors_per_block << SECTOR_SHIFT, offset); > - nbytes += ic->sectors_per_block << SECTOR_SHIFT; > + if (likely(len)) { > + sg_set_page(s, page, len, offset); > + nbytes += len; > + } > > ahash_request_set_crypt(req, sg, result, nbytes); > > @@ -1763,10 +1773,40 @@ static void integrity_sector_checksum_ahash(struct dm_integrity_c *ic, struct ah > static void integrity_sector_checksum(struct dm_integrity_c *ic, struct ahash_request **ahash_req, > sector_t sector, const char *data, unsigned offset, char *result) > { > + unsigned int len = ic->sectors_per_block << SECTOR_SHIFT; > + > if (likely(ic->internal_shash != NULL)) > - integrity_sector_checksum_shash(ic, sector, data, offset, result); > + integrity_sector_checksum_shash(ic, sector, data, offset, len, result); > else > - integrity_sector_checksum_ahash(ic, ahash_req, sector, (struct page *)data, offset, result); > + integrity_sector_checksum_ahash(ic, ahash_req, sector, (struct page *)data, > + offset, len, result); > +} > + > +/* > + * Authenticated marker for a discarded block: HMAC_key(salt || sector), with > + * no data payload. Because a real data tag's input always covers a full > + * block, its length differs from this marker's, so the two can never > + * collide structurally, regardless of block content. > + */ > +static void integrity_discard_checksum(struct dm_integrity_c *ic, struct ahash_request **ahash_req, > + sector_t sector, char *result) > +{ > + if (likely(ic->internal_shash != NULL)) > + integrity_sector_checksum_shash(ic, sector, NULL, 0, 0, result); > + else > + integrity_sector_checksum_ahash(ic, ahash_req, sector, NULL, 0, 0, result); > +} > + > +static void integrity_discard_fill_tags(struct dm_integrity_c *ic, struct ahash_request **ahash_req, > + unsigned char *checksums, sector_t *sector, > + unsigned int blocks) > +{ > + unsigned int i; > + > + for (i = 0; i < blocks; i++) { > + integrity_discard_checksum(ic, ahash_req, *sector, checksums + i * ic->tag_size); > + *sector += ic->sectors_per_block; > + } > } > > static void *integrity_kmap(struct dm_integrity_c *ic, struct page *p) > @@ -1795,6 +1835,29 @@ static void *integrity_identity(struct dm_integrity_c *ic, void *data) > return virt_to_page(data); > } > > +static int integrity_recheck_verify_tag(struct dm_integrity_io *dio, char *checksum, > + char *on_disk_tag, sector_t logical_sector) > +{ > + struct dm_integrity_c *ic = dio->ic; > + int r; > + > + if (!(ic->sb->flags & cpu_to_le32(SB_FLAG_DISCARD_KEYED))) > + return dm_integrity_rw_tag(ic, checksum, &dio->metadata_block, > + &dio->metadata_offset, ic->tag_size, TAG_CMP); > + > + r = dm_integrity_rw_tag(ic, on_disk_tag, &dio->metadata_block, > + &dio->metadata_offset, ic->tag_size, TAG_READ); > + if (unlikely(r)) > + return r; > + > + r = crypto_memneq(on_disk_tag, checksum, ic->tag_size); > + if (unlikely(r)) { > + integrity_discard_checksum(ic, &dio->ahash_req, logical_sector, checksum); > + r = crypto_memneq(on_disk_tag, checksum, ic->tag_size); > + } > + return r; > +} > + > 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)); > @@ -1820,6 +1883,7 @@ static noinline void integrity_recheck(struct dm_integrity_io *dio, char *checks > char *mem; > char *buffer = page_to_virt(page); > unsigned int buffer_offset; > + char on_disk_tag[MAX_T(size_t, HASH_MAX_DIGESTSIZE, MAX_TAG_SIZE)]; > int r; > struct dm_io_request io_req; > struct dm_io_region io_loc; > @@ -1847,8 +1911,8 @@ static noinline void integrity_recheck(struct dm_integrity_io *dio, char *checks > } > > integrity_sector_checksum(ic, &dio->ahash_req, logical_sector, integrity_identity(ic, buffer), buffer_offset, checksum); > - r = dm_integrity_rw_tag(ic, checksum, &dio->metadata_block, > - &dio->metadata_offset, ic->tag_size, TAG_CMP); > + r = integrity_recheck_verify_tag(dio, checksum, on_disk_tag, > + logical_sector); > if (r) { > if (r > 0) { > DMERR_LIMIT("%pg: Checksum failed at sector 0x%llx", > @@ -1914,13 +1978,19 @@ static void integrity_metadata(struct work_struct *w) > unsigned int bi_size = dio->bio_details.bi_iter.bi_size; > unsigned int max_size = likely(checksums != checksums_onstack) ? PAGE_SIZE : HASH_MAX_DIGESTSIZE; > unsigned int max_blocks = max_size / ic->tag_size; > + sector_t sector = dio->range.logical_sector; > + bool discard_keyed = ic->sb->flags & cpu_to_le32(SB_FLAG_DISCARD_KEYED); > > - memset(checksums, DISCARD_FILLER, max_size); > + if (!discard_keyed) > + memset(checksums, DISCARD_FILLER, max_size); > > while (bi_size) { > unsigned int this_step_blocks = bi_size >> (SECTOR_SHIFT + ic->sb->log2_sectors_per_block); > > this_step_blocks = min(this_step_blocks, max_blocks); > + if (discard_keyed) > + integrity_discard_fill_tags(ic, &dio->ahash_req, checksums, > + §or, this_step_blocks); > r = dm_integrity_rw_tag(ic, checksums, &dio->metadata_block, &dio->metadata_offset, > this_step_blocks * ic->tag_size, TAG_WRITE); > if (unlikely(r)) { > @@ -3795,7 +3865,7 @@ static void dm_integrity_resume(struct dm_target *ti) > > ic->wrote_to_journal = false; > > - flags = ic->sb->flags & cpu_to_le32(SB_FLAG_RECALCULATING); > + flags = ic->sb->flags & cpu_to_le32(SB_FLAG_RECALCULATING | SB_FLAG_DISCARD_KEYED); > r = sync_rw_sb(ic, REQ_OP_READ); > if (r) > dm_integrity_io_error(ic, "reading superblock", r); > @@ -3955,6 +4025,7 @@ static void dm_integrity_status(struct dm_target *ti, status_type_t type, > arg_count += !!ic->journal_mac_alg.alg_string; > arg_count += (ic->sb->flags & cpu_to_le32(SB_FLAG_FIXED_PADDING)) != 0; > arg_count += (ic->sb->flags & cpu_to_le32(SB_FLAG_FIXED_HMAC)) != 0; > + arg_count += (ic->sb->flags & cpu_to_le32(SB_FLAG_DISCARD_KEYED)) != 0; > arg_count += ic->legacy_recalculate; > DMEMIT("%s %llu %u %c %u", ic->dev->name, ic->start, > ic->tag_size, ic->mode, arg_count); > @@ -3968,6 +4039,8 @@ static void dm_integrity_status(struct dm_target *ti, status_type_t type, > DMEMIT(" reset_recalculate"); > if (ic->discard) > DMEMIT(" allow_discards"); > + if ((ic->sb->flags & cpu_to_le32(SB_FLAG_DISCARD_KEYED)) != 0) > + DMEMIT(" allow_discards_keyed"); > if (ic->mode != 'I') > DMEMIT(" interleave_sectors:%u", 1U << ic->sb->log2_interleave_sectors); > DMEMIT(" buffer_sectors:%u", 1U << ic->log2_buffer_sectors); > @@ -4017,6 +4090,8 @@ static void dm_integrity_status(struct dm_target *ti, status_type_t type, > DMEMIT(",recalculate=%c", (ic->sb->flags & cpu_to_le32(SB_FLAG_RECALCULATING)) ? > 'y' : 'n'); > DMEMIT(",allow_discards=%c", ic->discard ? 'y' : 'n'); > + DMEMIT(",allow_discards_keyed=%c", > + ((ic->sb->flags & cpu_to_le32(SB_FLAG_DISCARD_KEYED)) != 0) ? 'y' : 'n'); > DMEMIT(",fix_padding=%c", > ((ic->sb->flags & cpu_to_le32(SB_FLAG_FIXED_PADDING)) != 0) ? 'y' : 'n'); > DMEMIT(",fix_hmac=%c", > @@ -4178,6 +4253,9 @@ static int initialize_superblock(struct dm_integrity_c *ic, > get_random_bytes(ic->sb->salt, SALT_SIZE); > } > > + if (ic->discard_keyed) > + ic->sb->flags |= cpu_to_le32(SB_FLAG_DISCARD_KEYED); > + > if (!ic->meta_dev) { > if (ic->fix_padding) > ic->sb->flags |= cpu_to_le32(SB_FLAG_FIXED_PADDING); > @@ -4835,6 +4913,9 @@ static int dm_integrity_ctr(struct dm_target *ti, unsigned int argc, char **argv > ic->reset_recalculate_flag = true; > } else if (!strcmp(opt_string, "allow_discards")) { > ic->discard = true; > + } else if (!strcmp(opt_string, "allow_discards_keyed")) { > + ic->discard = true; > + ic->discard_keyed = true; > } else if (!strcmp(opt_string, "fix_padding")) { > ic->fix_padding = true; > } else if (!strcmp(opt_string, "fix_hmac")) { > @@ -5212,6 +5293,9 @@ static int dm_integrity_ctr(struct dm_target *ti, unsigned int argc, char **argv > ic->sb->recalc_sector = cpu_to_le64(0); > } > > + if (ic->discard_keyed) > + ic->sb->flags |= cpu_to_le32(SB_FLAG_DISCARD_KEYED); > + > if (ic->internal_hash) { > ic->recalc_wq = alloc_workqueue("dm-integrity-recalc", > WQ_MEM_RECLAIM | WQ_PERCPU, 1); > @@ -5430,7 +5514,7 @@ static void dm_integrity_dtr(struct dm_target *ti) > > static struct target_type integrity_target = { > .name = "integrity", > - .version = {1, 14, 0}, > + .version = {1, 15, 0}, > .module = THIS_MODULE, > .features = DM_TARGET_SINGLETON | DM_TARGET_INTEGRITY, > .ctr = dm_integrity_ctr, > -- > 2.53.0 > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2 1/1] dm-integrity: replace forgeable discard filler with a keyed sector marker 2026-07-30 15:29 ` Mikulas Patocka @ 2026-07-31 21:03 ` Milan Broz 0 siblings, 0 replies; 4+ messages in thread From: Milan Broz @ 2026-07-31 21:03 UTC (permalink / raw) To: Mikulas Patocka, Shukai Ni Cc: agk, snitzer, bmarzins, dm-devel, linux-kernel, Jo Van Bulck On 7/30/26 5:29 PM, Mikulas Patocka wrote: > Hi > > I accepted the patch (and made some small modifications to it). Mikulas, you added this part to constructor: + if (!!(ic->sb->flags & cpu_to_le32(SB_FLAG_DISCARD_KEYED)) != ic->discard_keyed) { + r = -EINVAL; + ti->error = "Mismatch in the discard_keyed flag"; + goto bad; + } Unfortunately, this is not going to work. In integritysetup, allowing discard is an active flag, not a format flag. It should be ok to activate discard later, but this check blocks it. It should also allow to activate the device without discard (even if it was activated before with it). IMO, it should upgrade the superblock if allow_discards_keyed is set on activation (and then do not allow it to revert back). With the current approach, I cannot add this option to integritysetup. We just have no way to send allow_discard in format action (as it is an activation flag only in our API). Milan ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-07-31 21:03 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-07-28 14:33 [PATCH v2 0/1] dm-integrity: replace forgeable discard filler with a keyed sector marker Shukai Ni 2026-07-28 14:33 ` [PATCH v2 1/1] " Shukai Ni 2026-07-30 15:29 ` Mikulas Patocka 2026-07-31 21:03 ` Milan Broz
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.