Re: [PATCH v2 2/3] dm-inlinecrypt: add target for inline block device encryption

[Benjamin Marzinski <bmarzins@redhat.com>]

On Tue, Apr 28, 2026 at 06:43:08PM +0800, Linlin Zhang wrote:
> Correct the response to Benjamin's comments.
> 
> On 4/27/2026 8:20 PM, Linlin Zhang wrote:
> > 
> > 
> > On 4/27/2026 9:19 AM, Benjamin Marzinski wrote:
> >> On Fri, Apr 10, 2026 at 06:40:30AM -0700, Linlin Zhang wrote:
> >>> From: Eric Biggers <ebiggers@google.com>
> >>> +
> >>> +static int inlinecrypt_map(struct dm_target *ti, struct bio *bio)
> >>> +{
> >>> +	const struct inlinecrypt_ctx *ctx = ti->private;
> >>> +	sector_t sector_in_target;
> >>> +	u64 dun[BLK_CRYPTO_DUN_ARRAY_SIZE] = {};
> >>> +
> >>> +	bio_set_dev(bio, ctx->dev->bdev);
> >>> +
> >>> +	/*
> >>> +	 * If the bio is a device-level request which doesn't target a specific
> >>> +	 * sector, there's nothing more to do.
> >>> +	 */
> >>> +	if (bio_sectors(bio) == 0)
> >>> +		return DM_MAPIO_REMAPPED;
> >>> +
> >>> +	/*
> >>> +	 * The bio should never have an encryption context already, since
> >>> +	 * dm-inlinecrypt doesn't pass through any inline encryption
> >>> +	 * capabilities to the layer above it.
> >>> +	 */
> >>> +	if (WARN_ON_ONCE(bio_has_crypt_ctx(bio)))
> >>> +		return DM_MAPIO_KILL;
> >>> +
> >>> +	/* Map the bio's sector to the underlying device. (512-byte sectors) */
> >>> +	sector_in_target = dm_target_offset(ti, bio->bi_iter.bi_sector);
> >>> +	bio->bi_iter.bi_sector = ctx->start + sector_in_target;
> >>> +	/*
> >>> +	 * If the bio doesn't have any data (e.g. if it's a DISCARD request),
> >>> +	 * there's nothing more to do.
> >>> +	 */
> >>> +	if (!bio_has_data(bio))
> >>> +		return DM_MAPIO_REMAPPED;
> >>> +
> >>> +	/* Calculate the DUN and enforce data-unit (crypto sector) alignment. */
> >>> +	dun[0] = ctx->iv_offset + sector_in_target; /* 512-byte sectors */
> >>> +	if (dun[0] & ((ctx->sector_size >> SECTOR_SHIFT) - 1))
> >>> +		return DM_MAPIO_KILL;
> >>
> >> If ctx->iv_offset is not a multiple of ctx->sector_size, this will
> >> always fail. ctx->iv_offset should probably get validated in
> >> inlinecrypt_ctr()
> > 
> > ACK
> > 
> > Yes, this assumes iv_offset is aligned to sector_size when large crypto
> > sectors are used. That’s a requirement of dm-inlinecrypt semantics, and
> > adding an explicit check in inlinecrypt_ctr() would make this fail earlier
> > and more clearly.
> 
> Sorry, the last response is wrong. No need to add check in inlinecrypt_ctr().
> 
> iv_offset is the starting offset for IVs that are generated as if the target were
> preceded by iv_offset 512-byte sectors.
> 
> I think this concern is based on an implicit assumption that
> sector_in_target is always data-unit (crypto sector) aligned. In this
> target, however, sector_in_target is derived from dm_target_offset() and
> is in 512-byte sectors, so it is not guaranteed to be a multiple of
> (sector_size >> SECTOR_SHIFT).

sector_in_target should be guaranteed to be sector_size aligned.
inlinecrypt_io_hints() sets the device logical block size to at least
ctx->sector_size, and validate_hardware_logical_block_alignment() makes
sure that the target starts on a logical block boundary. The block
layer enforces IO to be aligned with the logical block size, so
an IO that starts 7 sectors into a device with a 4096 ctx->sector_size
should be impossible. 

-Ben