On 28/06/2019 17:21, Ard Biesheuvel wrote:
> Replace the explicit ESSIV handling in the dm-crypt driver with calls
> into the crypto API, which now possesses the capability to perform
> this processing within the crypto subsystem.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx>
> drivers/md/dm-crypt.c | 200 ++++----------------
...
> -/* Wipe salt and reset key derived from volume key */
> -static int crypt_iv_essiv_wipe(struct crypt_config *cc)
Do I understand it correctly, that this is now called inside the whole cipher
set key in wipe command (in crypt_wipe_key())?
(Wipe message is meant to suspend the device and wipe all key material
from memory without actually destroying the device.)
> -{
> - struct iv_essiv_private *essiv = &cc->iv_gen_private.essiv;
> - unsigned salt_size = crypto_shash_digestsize(essiv->hash_tfm);
> - struct crypto_cipher *essiv_tfm;
> - int r, err = 0;
> -
> - memset(essiv->salt, 0, salt_size);
> -
> - essiv_tfm = cc->iv_private;
> - r = crypto_cipher_setkey(essiv_tfm, essiv->salt, salt_size);
> - if (r)
> - err = r;
> -
> - return err;
> -}
...
> @@ -2435,9 +2281,19 @@ static int crypt_ctr_cipher_new(struct dm_target *ti, char *cipher_in, char *key
> }
>
> ret = crypt_ctr_blkdev_cipher(cc, cipher_api);
> - if (ret < 0) {
> - ti->error = "Cannot allocate cipher string";
> - return -ENOMEM;
> + if (ret < 0)
> + goto bad_mem;
> +
> + if (*ivmode && !strcmp(*ivmode, "essiv")) {
> + if (!*ivopts) {
> + ti->error = "Digest algorithm missing for ESSIV mode";
> + return -EINVAL;
> + }
> + ret = snprintf(buf, CRYPTO_MAX_ALG_NAME, "essiv(%s,%s,%s)",
> + cipher_api, cc->cipher, *ivopts);
> + if (ret < 0 || ret >= CRYPTO_MAX_ALG_NAME)
> + goto bad_mem;
Hm, nitpicking, but goto from only one place while we have another -ENOMEM above...
Just place this here without goto?
> + ti->error = "Cannot allocate cipher string";
> + return -ENOMEM;
Otherwise
Reviewed-by: Milan Broz <gmazyland@xxxxxxxxx>
Thanks,
Milan
