On Thu, Jun 01 2023 at 4:47P -0400,
Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx> wrote:
> MAX_CIPHER_BLOCKSIZE is an internal implementation detail and should
> not be relied on by users of the Crypto API.
>
> Instead of storing the IV on the stack, allocate it together with
> the crypto request.
>
> Signed-off-by: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
> ---
>
> drivers/md/dm-crypt.c | 15 +++++++++++----
> 1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
> index 40cb1719ae4d..0e7e443dde11 100644
> --- a/drivers/md/dm-crypt.c
> +++ b/drivers/md/dm-crypt.c
> @@ -31,10 +31,10 @@
> #include <asm/unaligned.h>
> #include <crypto/hash.h>
> #include <crypto/md5.h>
> -#include <crypto/algapi.h>
> #include <crypto/skcipher.h>
> #include <crypto/aead.h>
> #include <crypto/authenc.h>
> +#include <crypto/utils.h>
> #include <linux/rtnetlink.h> /* for struct rtattr and RTA macros only */
> #include <linux/key-type.h>
> #include <keys/user-type.h>
> @@ -743,16 +743,23 @@ static int crypt_iv_eboiv_ctr(struct crypt_config *cc, struct dm_target *ti,
> static int crypt_iv_eboiv_gen(struct crypt_config *cc, u8 *iv,
> struct dm_crypt_request *dmreq)
> {
> - u8 buf[MAX_CIPHER_BLOCKSIZE] __aligned(__alignof__(__le64));
> + struct crypto_skcipher *tfm = any_tfm(cc);
> struct skcipher_request *req;
> struct scatterlist src, dst;
> DECLARE_CRYPTO_WAIT(wait);
> + unsigned int reqsize;
> int err;
> + u8 *buf;
>
> - req = skcipher_request_alloc(any_tfm(cc), GFP_NOIO);
> + reqsize = ALIGN(crypto_skcipher_reqsize(tfm), __alignof__(__le64));
> +
> + req = kmalloc(reqsize + cc->iv_size, GFP_NOIO);
> if (!req)
> return -ENOMEM;
>
> + skcipher_request_set_tfm(req, tfm);
> +
> + buf = (u8 *)req + reqsize;
> memset(buf, 0, cc->iv_size);
> *(__le64 *)buf = cpu_to_le64(dmreq->iv_sector * cc->sector_size);
>
> @@ -761,7 +768,7 @@ static int crypt_iv_eboiv_gen(struct crypt_config *cc, u8 *iv,
> skcipher_request_set_crypt(req, &src, &dst, cc->iv_size, buf);
> skcipher_request_set_callback(req, 0, crypto_req_done, &wait);
> err = crypto_wait_req(crypto_skcipher_encrypt(req), &wait);
> - skcipher_request_free(req);
> + kfree_sensitive(req);
>
> return err;
> }
Strikes me as strange that open-coding skcipher_request_{alloc,free}
is ideal, but dm-crypt is the only non-crypto consumer of
MAX_CIPHER_BLOCKSIZE so really not worth standing up yet another
interface wrapper.
Anyway, this code is certainly better for dm-crypt's needs. I'm happy
with you applying this change via your crypto tree.
Reviewed-by: Mike Snitzer <snitzer@xxxxxxxxxx>
Thanks.
