Re: [PATCH 1/1] dm crypt: Add inline encryption support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2022/01/14 4:11, Israel Rukshin wrote:
> Using inline encryption means that the block layer handles the
> decryption/encryption as part of the bio, instead of dm-crypt
> doing the crypto by itself via Linux's crypto API. This model
> is needed to take advantage of the inline encryption hardware
> on the market.
> 
> To use inline encryption, the new dm-crypt optional parameter
> "inline_crypt" should be set for the configured mapping. Afterwards,
> dm-crypt will provide the crypto parameters to the block layer by
> creating a cypto profile and by filling the bios with crypto context.
> In case the block device or the fallback algorithm doesn't support
> this feature, the mapping will fail.
> 
> Signed-off-by: Israel Rukshin <israelr@xxxxxxxxxx>
> ---
>  block/blk-crypto.c    |   3 +
>  drivers/md/dm-crypt.c | 202 ++++++++++++++++++++++++++++++++++++------
>  2 files changed, 180 insertions(+), 25 deletions(-)
> 
> diff --git a/block/blk-crypto.c b/block/blk-crypto.c
> index 1c08d3b6e24a..65f13549eb5f 100644
> --- a/block/blk-crypto.c
> +++ b/block/blk-crypto.c
> @@ -102,6 +102,7 @@ void bio_crypt_set_ctx(struct bio *bio, const struct blk_crypto_key *key,
>  
>  	bio->bi_crypt_context = bc;
>  }
> +EXPORT_SYMBOL_GPL(bio_crypt_set_ctx);
>  
>  void __bio_crypt_free_ctx(struct bio *bio)
>  {
> @@ -370,6 +371,7 @@ int blk_crypto_init_key(struct blk_crypto_key *blk_key,
>  
>  	return 0;
>  }
> +EXPORT_SYMBOL_GPL(blk_crypto_init_key);
>  
>  /*
>   * Check if bios with @cfg can be en/decrypted by blk-crypto (i.e. either the
> @@ -411,6 +413,7 @@ int blk_crypto_start_using_key(const struct blk_crypto_key *key,
>  	}
>  	return blk_crypto_fallback_start_using_mode(key->crypto_cfg.crypto_mode);
>  }
> +EXPORT_SYMBOL_GPL(blk_crypto_start_using_key);
>  
>  /**
>   * blk_crypto_evict_key() - Evict a key from any inline encryption hardware

These changes could probably go into a separate prep patch.

> diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
> index d4ae31558826..4c0e1904942b 100644
> --- a/drivers/md/dm-crypt.c
> +++ b/drivers/md/dm-crypt.c
> @@ -39,6 +39,7 @@
>  #include <keys/user-type.h>
>  #include <keys/encrypted-type.h>
>  #include <keys/trusted-type.h>
> +#include <linux/blk-crypto.h>
>  
>  #include <linux/device-mapper.h>
>  
> @@ -134,7 +135,7 @@ struct iv_elephant_private {
>  enum flags { DM_CRYPT_SUSPENDED, DM_CRYPT_KEY_VALID,
>  	     DM_CRYPT_SAME_CPU, DM_CRYPT_NO_OFFLOAD,
>  	     DM_CRYPT_NO_READ_WORKQUEUE, DM_CRYPT_NO_WRITE_WORKQUEUE,
> -	     DM_CRYPT_WRITE_INLINE };
> +	     DM_CRYPT_WRITE_INLINE, DM_CRYPT_INLINE_ENCRYPTION };
>  
>  enum cipher_flags {
>  	CRYPT_MODE_INTEGRITY_AEAD,	/* Use authenticated mode for cipher */
> @@ -220,6 +221,11 @@ struct crypt_config {
>  	struct bio_set bs;
>  	struct mutex bio_alloc_lock;
>  
> +#ifdef CONFIG_BLK_INLINE_ENCRYPTION
> +	enum blk_crypto_mode_num crypto_mode;
> +	enum blk_crypto_key_type key_type;
> +	struct blk_crypto_key *blk_key;
> +#endif
>  	u8 *authenc_key; /* space for keys in authenc() format (if used) */
>  	u8 key[];
>  };
> @@ -2383,11 +2389,103 @@ static void crypt_copy_authenckey(char *p, const void *key,
>  	memcpy(p, key, enckeylen);
>  }
>  
> +#ifdef CONFIG_BLK_INLINE_ENCRYPTION
> +static int crypt_select_inline_crypt_mode(struct dm_target *ti, char *cipher,
> +					  char *ivmode)
> +{
> +	struct crypt_config *cc = ti->private;
> +
> +	if (strcmp(cipher, "xts(aes)") == 0) {
> +		cc->crypto_mode = BLK_ENCRYPTION_MODE_AES_256_XTS;
> +		cc->key_type = BLK_CRYPTO_KEY_TYPE_STANDARD;
> +	} else if (strcmp(cipher, "xts(paes)") == 0) {
> +		cc->crypto_mode = BLK_ENCRYPTION_MODE_AES_256_XTS;
> +		cc->key_type = BLK_CRYPTO_KEY_TYPE_HW_WRAPPED;
> +	} else {
> +		ti->error = "Invalid cipher for inline_crypt";
> +		return -EINVAL;
> +	}
> +
> +	if (ivmode == NULL || (strcmp(ivmode, "plain64") == 0)) {
> +		cc->iv_size = 8;
> +	} else {
> +		ti->error = "Invalid IV mode for inline_crypt";
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int crypt_prepare_inline_crypt_key(struct crypt_config *cc)
> +{
> +	int ret;
> +
> +	cc->blk_key = kzalloc(sizeof(*cc->blk_key), GFP_KERNEL);
> +	if (!cc->blk_key)
> +		return -ENOMEM;
> +
> +	ret = blk_crypto_init_key(cc->blk_key, cc->key, cc->key_size,
> +				  cc->key_type, cc->crypto_mode, cc->iv_size,
> +				  cc->sector_size);
> +	if (ret) {
> +		DMERR("Failed to init inline encryption key");
> +		goto bad_key;
> +	}
> +
> +	ret = blk_crypto_start_using_key(cc->blk_key,
> +					 bdev_get_queue(cc->dev->bdev));
> +	if (ret) {
> +		DMERR("Failed to use inline encryption key");
> +		goto bad_key;
> +	}
> +
> +	return 0;
> +bad_key:
> +	kfree_sensitive(cc->blk_key);
> +	cc->blk_key = NULL;
> +	return ret;
> +}
> +
> +static void crypt_destroy_inline_crypt_key(struct crypt_config *cc)
> +{
> +	if (cc->blk_key) {
> +		blk_crypto_evict_key(bdev_get_queue(cc->dev->bdev),
> +				     cc->blk_key);
> +		kfree_sensitive(cc->blk_key);
> +		cc->blk_key = NULL;
> +	}
> +}
> +
> +static void crypt_inline_encrypt_submit(struct dm_target *ti, struct bio *bio)
> +{
> +	struct crypt_config *cc = ti->private;
> +	u64 dun[BLK_CRYPTO_DUN_ARRAY_SIZE];
> +
> +	bio_set_dev(bio, cc->dev->bdev);
> +	if (bio_sectors(bio)) {
> +		memset(dun, 0, BLK_CRYPTO_MAX_IV_SIZE);
> +		bio->bi_iter.bi_sector = cc->start +
> +			dm_target_offset(ti, bio->bi_iter.bi_sector);
> +		dun[0] = le64_to_cpu(bio->bi_iter.bi_sector + cc->iv_offset);
> +		bio_crypt_set_ctx(bio, cc->blk_key, dun, GFP_KERNEL);
> +	}
> +
> +	submit_bio_noacct(bio);
> +}

#else

define the above functions as empty (see below).

> +#endif
> +
>  static int crypt_setkey(struct crypt_config *cc)
>  {
>  	unsigned subkey_size;
>  	int err = 0, i, r;
>  
> +#ifdef CONFIG_BLK_INLINE_ENCRYPTION
> +	if (test_bit(DM_CRYPT_INLINE_ENCRYPTION, &cc->flags)) {
> +		crypt_destroy_inline_crypt_key(cc);
> +		return crypt_prepare_inline_crypt_key(cc);
> +	}
> +#endif

You could avoid the ifdef here using IS_ENABLED(CONFIG_BLK_INLINE_ENCRYPTION)
directly in the if condition. That will make the code cleaner. That said, since
DM_CRYPT_INLINE_ENCRYPTION can only be set if CONFIG_BLK_INLINE_ENCRYPTION is
enabled, I am not sure if the ifdef buys you much in the
!CONFIG_BLK_INLINE_ENCRYPTION case.

> +
>  	/* Ignore extra keys (which are used for IV etc) */
>  	subkey_size = crypt_subkey_size(cc);
>  
> @@ -2648,6 +2746,15 @@ static int crypt_wipe_key(struct crypt_config *cc)
>  
>  	kfree_sensitive(cc->key_string);
>  	cc->key_string = NULL;
> +
> +#ifdef CONFIG_BLK_INLINE_ENCRYPTION
> +	if (test_bit(DM_CRYPT_INLINE_ENCRYPTION, &cc->flags)) {
> +		crypt_destroy_inline_crypt_key(cc);
> +		memset(&cc->key, 0, cc->key_size * sizeof(u8));
> +		return 0;
> +	}
> +#endif

same comment as above and for most of the following ifdef additions.

> +
>  	r = crypt_setkey(cc);
>  	memset(&cc->key, 0, cc->key_size * sizeof(u8));
>  
> @@ -2713,6 +2820,10 @@ static void crypt_dtr(struct dm_target *ti)
>  	if (cc->crypt_queue)
>  		destroy_workqueue(cc->crypt_queue);
>  
> +#ifdef CONFIG_BLK_INLINE_ENCRYPTION
> +	crypt_destroy_inline_crypt_key(cc);
> +#endif

You can avoid the ifdef here if this function is defined as empty in the
!CONFIG_BLK_INLINE_ENCRYPTION case (see above the comment about #else).

> +
>  	crypt_free_tfms(cc);
>  
>  	bioset_exit(&cc->bs);
> @@ -2888,6 +2999,11 @@ static int crypt_ctr_cipher_new(struct dm_target *ti, char *cipher_in, char *key
>  	/* The rest is crypto API spec */
>  	cipher_api = tmp;
>  
> +#ifdef CONFIG_BLK_INLINE_ENCRYPTION
> +	if (test_bit(DM_CRYPT_INLINE_ENCRYPTION, &cc->flags))
> +		return crypt_select_inline_crypt_mode(ti, cipher_api, *ivmode);
> +#endif
> +
>  	/* Alloc AEAD, can be used only in new format. */
>  	if (crypt_integrity_aead(cc)) {
>  		ret = crypt_ctr_auth_cipher(cc, cipher_api);
> @@ -3001,6 +3117,11 @@ static int crypt_ctr_cipher_old(struct dm_target *ti, char *cipher_in, char *key
>  		goto bad_mem;
>  	}
>  
> +#ifdef CONFIG_BLK_INLINE_ENCRYPTION
> +	if (test_bit(DM_CRYPT_INLINE_ENCRYPTION, &cc->flags))
> +		return crypt_select_inline_crypt_mode(ti, cipher_api, *ivmode);
> +#endif
> +
>  	/* Allocate cipher */
>  	ret = crypt_alloc_tfms(cc, cipher_api);
>  	if (ret < 0) {
> @@ -3036,9 +3157,11 @@ static int crypt_ctr_cipher(struct dm_target *ti, char *cipher_in, char *key)
>  		return ret;
>  
>  	/* Initialize IV */
> -	ret = crypt_ctr_ivmode(ti, ivmode);
> -	if (ret < 0)
> -		return ret;
> +	if (!test_bit(DM_CRYPT_INLINE_ENCRYPTION, &cc->flags)) {
> +		ret = crypt_ctr_ivmode(ti, ivmode);
> +		if (ret < 0)
> +			return ret;
> +	}
>  
>  	/* Initialize and set key */
>  	ret = crypt_set_key(cc, key);
> @@ -3111,6 +3234,10 @@ static int crypt_ctr_optional(struct dm_target *ti, unsigned int argc, char **ar
>  			set_bit(DM_CRYPT_NO_READ_WORKQUEUE, &cc->flags);
>  		else if (!strcasecmp(opt_string, "no_write_workqueue"))
>  			set_bit(DM_CRYPT_NO_WRITE_WORKQUEUE, &cc->flags);
> +#ifdef CONFIG_BLK_INLINE_ENCRYPTION
> +		else if (!strcasecmp(opt_string, "inline_crypt"))
> +			set_bit(DM_CRYPT_INLINE_ENCRYPTION, &cc->flags);
> +#endif

May be add a warning here for the !CONFIG_BLK_INLINE_ENCRYPTION case ?

>  		else if (sscanf(opt_string, "integrity:%u:", &val) == 1) {
>  			if (val == 0 || val > MAX_TAG_SIZE) {
>  				ti->error = "Invalid integrity arguments";
> @@ -3218,10 +3345,36 @@ static int crypt_ctr(struct dm_target *ti, unsigned int argc, char **argv)
>  			goto bad;
>  	}
>  
> +	ret = -EINVAL;
> +	if ((sscanf(argv[2], "%llu%c", &tmpll, &dummy) != 1) ||
> +	    (tmpll & ((cc->sector_size >> SECTOR_SHIFT) - 1))) {
> +		ti->error = "Invalid iv_offset sector";
> +		goto bad;
> +	}
> +	cc->iv_offset = tmpll;
> +
> +	ret = dm_get_device(ti, argv[3], dm_table_get_mode(ti->table),
> +			    &cc->dev);
> +	if (ret) {
> +		ti->error = "Device lookup failed";
> +		goto bad;
> +	}
> +
> +	ret = -EINVAL;
> +	if (sscanf(argv[4], "%llu%c", &tmpll, &dummy) != 1 ||
> +	    tmpll != (sector_t)tmpll) {
> +		ti->error = "Invalid device sector";
> +		goto bad;
> +	}
> +	cc->start = tmpll;
> +
>  	ret = crypt_ctr_cipher(ti, argv[0], argv[1]);
>  	if (ret < 0)
>  		goto bad;
>  
> +	if (test_bit(DM_CRYPT_INLINE_ENCRYPTION, &cc->flags))
> +		return 0;
> +
>  	if (crypt_integrity_aead(cc)) {
>  		cc->dmreq_start = sizeof(struct aead_request);
>  		cc->dmreq_start += crypto_aead_reqsize(any_tfm_aead(cc));
> @@ -3277,27 +3430,6 @@ static int crypt_ctr(struct dm_target *ti, unsigned int argc, char **argv)
>  
>  	mutex_init(&cc->bio_alloc_lock);
>  
> -	ret = -EINVAL;
> -	if ((sscanf(argv[2], "%llu%c", &tmpll, &dummy) != 1) ||
> -	    (tmpll & ((cc->sector_size >> SECTOR_SHIFT) - 1))) {
> -		ti->error = "Invalid iv_offset sector";
> -		goto bad;
> -	}
> -	cc->iv_offset = tmpll;
> -
> -	ret = dm_get_device(ti, argv[3], dm_table_get_mode(ti->table), &cc->dev);
> -	if (ret) {
> -		ti->error = "Device lookup failed";
> -		goto bad;
> -	}
> -
> -	ret = -EINVAL;
> -	if (sscanf(argv[4], "%llu%c", &tmpll, &dummy) != 1 || tmpll != (sector_t)tmpll) {
> -		ti->error = "Invalid device sector";
> -		goto bad;
> -	}
> -	cc->start = tmpll;
> -
>  	if (bdev_is_zoned(cc->dev->bdev)) {
>  		/*
>  		 * For zoned block devices, we need to preserve the issuer write
> @@ -3419,6 +3551,13 @@ static int crypt_map(struct dm_target *ti, struct bio *bio)
>  	if (unlikely(bio->bi_iter.bi_size & (cc->sector_size - 1)))
>  		return DM_MAPIO_KILL;
>  
> +#ifdef CONFIG_BLK_INLINE_ENCRYPTION
> +	if (test_bit(DM_CRYPT_INLINE_ENCRYPTION, &cc->flags)) {
> +		crypt_inline_encrypt_submit(ti, bio);
> +		return DM_MAPIO_SUBMITTED;
> +	}
> +#endif
> +
>  	io = dm_per_bio_data(bio, cc->per_bio_data_size);
>  	crypt_io_init(io, cc, bio, dm_target_offset(ti, bio->bi_iter.bi_sector));
>  
> @@ -3481,6 +3620,10 @@ static void crypt_status(struct dm_target *ti, status_type_t type,
>  		num_feature_args += test_bit(DM_CRYPT_NO_OFFLOAD, &cc->flags);
>  		num_feature_args += test_bit(DM_CRYPT_NO_READ_WORKQUEUE, &cc->flags);
>  		num_feature_args += test_bit(DM_CRYPT_NO_WRITE_WORKQUEUE, &cc->flags);
> +#ifdef CONFIG_BLK_INLINE_ENCRYPTION
> +		num_feature_args +=
> +			test_bit(DM_CRYPT_INLINE_ENCRYPTION, &cc->flags);
> +#endif

You do not think that you need the ifdef here. If CONFIG_BLK_INLINE_ENCRYPTION
is not enabled, then DM_CRYPT_INLINE_ENCRYPTION is never set.

>  		num_feature_args += cc->sector_size != (1 << SECTOR_SHIFT);
>  		num_feature_args += test_bit(CRYPT_IV_LARGE_SECTORS, &cc->cipher_flags);
>  		if (cc->on_disk_tag_size)
> @@ -3497,6 +3640,10 @@ static void crypt_status(struct dm_target *ti, status_type_t type,
>  				DMEMIT(" no_read_workqueue");
>  			if (test_bit(DM_CRYPT_NO_WRITE_WORKQUEUE, &cc->flags))
>  				DMEMIT(" no_write_workqueue");
> +#ifdef CONFIG_BLK_INLINE_ENCRYPTION
> +			if (test_bit(DM_CRYPT_INLINE_ENCRYPTION, &cc->flags))
> +				DMEMIT(" inline_crypt");
> +#endif

ditto.

>  			if (cc->on_disk_tag_size)
>  				DMEMIT(" integrity:%u:%s", cc->on_disk_tag_size, cc->cipher_auth);
>  			if (cc->sector_size != (1 << SECTOR_SHIFT))
> @@ -3516,6 +3663,11 @@ static void crypt_status(struct dm_target *ti, status_type_t type,
>  		       'y' : 'n');
>  		DMEMIT(",no_write_workqueue=%c", test_bit(DM_CRYPT_NO_WRITE_WORKQUEUE, &cc->flags) ?
>  		       'y' : 'n');
> +#ifdef CONFIG_BLK_INLINE_ENCRYPTION
> +		DMEMIT(",inline_crypt=%c",
> +		       test_bit(DM_CRYPT_INLINE_ENCRYPTION, &cc->flags) ?
> +		       'y' : 'n');
> +#endif

You do not think that you need the ifdef here. If CONFIG_BLK_INLINE_ENCRYPTION
is not enabled, then inline_crypt=n will always be reported, which is fine I think.

>  		DMEMIT(",iv_large_sectors=%c", test_bit(CRYPT_IV_LARGE_SECTORS, &cc->cipher_flags) ?
>  		       'y' : 'n');
>  


-- 
Damien Le Moal
Western Digital Research

--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/dm-devel




[Index of Archives]     [DM Crypt]     [Fedora Desktop]     [ATA RAID]     [Fedora Marketing]     [Fedora Packaging]     [Fedora SELinux]     [Yosemite Discussion]     [KDE Users]     [Fedora Docs]

  Powered by Linux