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

[Linlin Zhang <linlin.zhang@oss.qualcomm.com>]

On 3/4/2026 9:11 PM, Milan Broz wrote:
> Hi,
> 
> just few comments below, but I am not DM maintainer so feel free to ignore it :)
> 
> On 3/4/26 1:17 PM, Linlin Zhang wrote:
>> From: Eric Biggers <ebiggers@google.com>
>>
>> Add a new device-mapper target "dm-inlinecrypt" that is similar to
>> dm-crypt but uses the blk-crypto API instead of the regular crypto API.
>> This allows it to take advantage of inline encryption hardware such as
>> that commonly built into UFS host controllers.
>>
>> The table syntax matches dm-crypt's, but for now only a stripped-down
>> set of parameters is supported.  For example, for now AES-256-XTS is the
>> only supported cipher.
>>
>> dm-inlinecrypt is based on Android's dm-default-key with the
>> controversial passthrough support removed.  Note that due to the removal
>> of passthrough support, use of dm-inlinecrypt in combination with
>> fscrypt causes double encryption of file contents (similar to dm-crypt +
>> fscrypt), with the fscrypt layer not being able to use the inline
>> encryption hardware.  This makes dm-inlinecrypt unusable on systems such
>> as Android that use fscrypt and where a more optimized approach is
>> needed.  It is however suitable as a replacement for dm-crypt.
>>
>> Signed-off-by: Eric Biggers <ebiggers@google.com>
>> Signed-off-by: Linlin Zhang <linlin.zhang@oss.qualcomm.com>
>> ---
>>   drivers/md/Kconfig          |  10 +
>>   drivers/md/Makefile         |   1 +
>>   drivers/md/dm-inlinecrypt.c | 416 ++++++++++++++++++++++++++++++++++++
> 
> I think it should also add doc in
> Documentation/admin-guide/device-mapper/dm-inlinecrypt.rst

Thanks for your review.
You're right. I'll add it in next patch.

> ...
> 
>> +#define DM_MSG_PREFIX    "inlinecrypt"
>> +
>> +static const struct dm_inlinecrypt_cipher {
>> +    const char *name;
>> +    enum blk_crypto_mode_num mode_num;
>> +    int key_size;
>> +} dm_inlinecrypt_ciphers[] = {
>> +    {
>> +        .name = "aes-xts-plain64",
>> +        .mode_num = BLK_ENCRYPTION_MODE_AES_256_XTS,
>> +        .key_size = 64,
> 
> Hm. I can understand some translation table for this stupid
> dm-crypt notation to inline enum, but why you need key size here?
> 
> Shouldn't there be some helper for inline crypt returning
> keysize based on BLK_ENCRYPTION_MODE_AES_256_XTS?

Similar to dm-default-key in Android (https://android.googlesource.com/kernel/common/+/android-mainline/drivers/md/dm-default-key.c),
key_size 64 here is always corresponding to a raw key. For a HW wrapped key, the key
size differs from different manufacturers. Linux kernel only constraints that the max
key size of HW wrapped key isn't larger than BLK_CRYPTO_MAX_HW_WRAPPED_KEY_SIZE(128).
So we get the actual key size based on the input key parameter received in ctr functions.

> 
> I guess you have fixed cipher list already, but what about IV?
> Is it always little-endian, or someone already reinvented plain64be (big-endian)?
> 
> ...> +    while (opt_params--) {
> 
> ...> +/*

Same to dm-crypt, 'plain64' in "aes-xts-plain64" of cipher name is the IV mode. It uses
the 64‑bit little‑endian sector number as the IV.


>> + * Construct an inlinecrypt mapping:
>> + * <cipher> <key> <iv_offset> <dev_path> <start>
> 
> As above, it supports opt params, it should mention it here (or in doc).

ACK, refer to dm-crypt, I'll add it in doc.

> 
> 
> ...
>> +    /* <key> */
>> +    if (strlen(argv[1]) != 2 * cipher->key_size) {
>> +        ti->error = "Incorrect key size for cipher";
>> +        err = -EINVAL;
>> +        goto bad;
>> +    }
>> +    if (hex2bin(raw_key, argv[1], cipher->key_size) != 0) {
>> +        ti->error = "Malformed key string";
>> +        err = -EINVAL;
>> +        goto bad;
>> +    }
> 
> 
> Any reason it does not support keyring keys from the beginning?

No special reason. The implementation of `dm-inlinecrypt` was modeled after
'dm-default-key', which, in Android, does not support keyring keys.
I'll have a insight about keyring and dm-crypt, inspect how to add keyring support
in dm-inline-crypt.

> 
> ...
>> +static int inlinecrypt_map(struct dm_target *ti, struct bio *bio)
> 
>> +    /* 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.
>> +     */
> 
> dmcrypt uses bio_set_dev() for REQ_PREFLUSH or REQ_OP_DISCARD, why this differs?

bio_has_data() is a payload‑based check that decides whether an encryption context
is needed, while REQ_PREFLUSH and REQ_OP_DISCARD are semantic checks that ensure
flush and discard requests bypass the dm‑crypt processing pipeline to maintain
correct I/O ordering. As a result, the two mechanisms address different concerns
and are not interchangeable.

In addition, bio_has_data is called in the beginning of inlinecrypt_map, rather
in the conditional branch like dm-crypt. But they have same effect.

> 
>> +
>> +    switch (type) {
>> +    case STATUSTYPE_INFO:
>> +    case STATUSTYPE_IMA:
>> +        result[0] = '\0';
> 
> This should really emit audit information similar to dm-crypt.
> 
>> +        break;
>> +
>> +    case STATUSTYPE_TABLE:
>> +        /*
>> +         * Warning: like dm-crypt, dm-inlinecrypt includes the key in
>> +         * the returned table.  Userspace is responsible for redacting
>> +         * the key when needed.
> 
> Again, why not support keyring format? LUKS2 uses it by default for dm-crypt table.

ACK. Need support keyring format.
I see both hex key and keyring-based keys are supported in dm-crypt. can both of them
be supported in dm-inlinecrypt?

> 
> Milan
>