Re: [PATCH v2 1/1] dm-integrity: replace forgeable discard filler with a keyed sector marker

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 <[email protected]>
> Signed-off-by: Jo Van Bulck <[email protected]>
> Signed-off-by: Shukai Ni <[email protected]>
> ---
>  .../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(&sector_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,
> +								    &sector, 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
>