Re: [PATCH v5 1/3] block: add basic hardware-wrapped key support

[Bart Van Assche <bvanassche@acm.org>]

On 2/27/22 23:05, Eric Biggers wrote:
> -static u8 blank_key[BLK_CRYPTO_MAX_KEY_SIZE];
> +static u8 blank_key[BLK_CRYPTO_MAX_STANDARD_KEY_SIZE];
>   
>   static void blk_crypto_fallback_evict_keyslot(unsigned int slot)
>   {
> @@ -539,7 +539,7 @@ static int blk_crypto_fallback_init(void)
>   	if (blk_crypto_fallback_inited)
>   		return 0;
>   
> -	prandom_bytes(blank_key, BLK_CRYPTO_MAX_KEY_SIZE);
> +	prandom_bytes(blank_key, BLK_CRYPTO_MAX_STANDARD_KEY_SIZE);

Please use sizeof(blank_key) to make it easier for readers to verify that the 
length argument is correct.

> +int blk_crypto_derive_sw_secret(struct blk_crypto_profile *profile,
> +				const u8 *wrapped_key,
> +				unsigned int wrapped_key_size,
> +				u8 sw_secret[BLK_CRYPTO_SW_SECRET_SIZE])
> +{
> +	int err = -EOPNOTSUPP;
> +
> +	if (profile &&
> +	    (profile->key_types_supported & BLK_CRYPTO_KEY_TYPE_HW_WRAPPED) &&
> +	    profile->ll_ops.derive_sw_secret) {
> +		blk_crypto_hw_enter(profile);
> +		err = profile->ll_ops.derive_sw_secret(profile, wrapped_key,
> +						       wrapped_key_size,
> +						       sw_secret);
> +		blk_crypto_hw_exit(profile);
> +	}
> +	return err;
> +}

Please use the common kernel style: return early if the preconditions have not 
been met. That helps to keep the indentation level low.

> @@ -68,7 +71,10 @@ static int __init bio_crypt_ctx_init(void)
>   
>   	/* Sanity check that no algorithm exceeds the defined limits. */
>   	for (i = 0; i < BLK_ENCRYPTION_MODE_MAX; i++) {
> -		BUG_ON(blk_crypto_modes[i].keysize > BLK_CRYPTO_MAX_KEY_SIZE);
> +		BUG_ON(blk_crypto_modes[i].keysize >
> +		       BLK_CRYPTO_MAX_STANDARD_KEY_SIZE);
> +		BUG_ON(blk_crypto_modes[i].security_strength >
> +		       blk_crypto_modes[i].keysize);
>   		BUG_ON(blk_crypto_modes[i].ivsize > BLK_CRYPTO_MAX_IV_SIZE);
>   	}

Does the following advice from Linus Torvalds apply to the above code: "because 
there is NO EXCUSE to knowingly kill the kernel"? See also 
https://lkml.org/lkml/2016/10/4/1.

Thanks,

Bart.