On 4/29/2026 12:20 AM, Benjamin Marzinski wrote:
> 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 <[email protected]>
>>>>> +
>>>>> +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.
Thanks for the detailed clarification!
In the next patch, I will add an explicit alignment check in ctr() to ensure
that
iv_offset is aligned to the configured sector size. For example:
if (ctx->iv_offset & ((ctx->sector_size >> SECTOR_SHIFT) - 1)) {
ti->error = "Wrong alignment of iv_offset sector";
err = -EINVAL;
}
Please let me know if you would like this validation to be handled differently,
or if additional checks are needed.
>
> -Ben
>
