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 <[email protected]>
>>
>> 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 <[email protected]>
>> Signed-off-by: Linlin Zhang <[email protected]>
>> ---
>> 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
>
