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).
The intended alignment requirement is on the *final* DUN, i.e. on
(iv_offset + sector_in_target) in 512-byte sector units, before it gets
shifted down to crypto-sector units. That's why the code checks alignment
on the sum (iv_offset + sector_in_target).
With this definition, iv_offset itself does not need to be aligned to the
data-unit size; any non-negative value is valid as long as the resulting
DUN for a given bio is data-unit aligned. For example, iv_offset = 1 can
still be valid when sector_in_target is 7 (4096-byte sector case), since
their sum is aligned. Validating iv_offset alone in inlinecrypt_ctr()
would therefore reject configurations that are otherwise correct per the
(sum-based) DUN definition.
So I believe the runtime check in ->map() is the right place to enforce
the data-unit alignment constraint, as it has the actual bio offset
(sector_in_target) needed to evaluate the constraint.
Let me know if you'd prefer we document this more explicitly in the map()
argument description; I'm fine adding a short note about the
iv_offset units and the sum-based alignment rule.
>
>>
>> -Ben
>>
>>> + dun[0] >>= ctx->sector_bits - SECTOR_SHIFT; /* crypto sectors */
>>> +
>>> + /*
>>> + * This check isn't necessary as we should have calculated max_dun
>>> + * correctly, but be safe.
>>> + */
>>> + if (WARN_ON_ONCE(dun[0] > ctx->max_dun))
>>> + return DM_MAPIO_KILL;
>>> +
>>> + bio_crypt_set_ctx(bio, &ctx->key, dun, GFP_NOIO);
>>> +
>>> + /*
>>> + * Since we've added an encryption context to the bio and
>>> + * blk-crypto-fallback may be needed to process it, it's necessary to
>>> + * use the fallback-aware bio submission code rather than
>>> + * unconditionally returning DM_MAPIO_REMAPPED.
>>> + *
>>> + * To get the correct accounting for a dm target in the case where
>>> + * __blk_crypto_submit_bio() doesn't take ownership of the bio (returns
>>> + * true), call __blk_crypto_submit_bio() directly and return
>>> + * DM_MAPIO_REMAPPED in that case, rather than relying on
>>> + * blk_crypto_submit_bio() which calls submit_bio() in that case.
>>> + */
>>> + if (__blk_crypto_submit_bio(bio))
>>> + return DM_MAPIO_REMAPPED;
>>> + return DM_MAPIO_SUBMITTED;
>>> +}
>>> +
<snip>
>>> +MODULE_AUTHOR("Eric Biggers <[email protected]>");
>>> +MODULE_AUTHOR("Linlin Zhang <[email protected]>");
>>> +MODULE_DESCRIPTION(DM_NAME " target for inline encryption");
>>> +MODULE_LICENSE("GPL");
>>> --
>>> 2.34.1
>>>
>>
>
