[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <76fb5763-c765-b3cf-9eec-1ac344cf49a9@intel.com>
Date: Thu, 11 May 2023 16:19:14 -0700
From: "Chang S. Bae" <chang.seok.bae@...el.com>
To: Eric Biggers <ebiggers@...nel.org>
CC: <linux-kernel@...r.kernel.org>, <linux-crypto@...r.kernel.org>,
<dm-devel@...hat.com>, <gmazyland@...il.com>, <luto@...nel.org>,
<dave.hansen@...ux.intel.com>, <tglx@...utronix.de>,
<bp@...en8.de>, <mingo@...nel.org>, <x86@...nel.org>,
<herbert@...dor.apana.org.au>, <ardb@...nel.org>,
<dan.j.williams@...el.com>, <bernie.keany@...el.com>,
<charishma1.gairuboyina@...el.com>,
<lalithambika.krishnakumar@...el.com>,
"David S. Miller" <davem@...emloft.net>,
Ingo Molnar <mingo@...hat.com>,
"H. Peter Anvin" <hpa@...or.com>
Subject: Re: [PATCH v6 10/12] crypto: x86/aes - Prepare for a new AES
implementation
On 5/11/2023 2:39 PM, Eric Biggers wrote:
> On Thu, May 11, 2023 at 12:05:17PM -0700, Chang S. Bae wrote:
>> +
>> +struct aes_xts_ctx {
>> + struct crypto_aes_ctx tweak_ctx AES_ALIGN_ATTR;
>> + struct crypto_aes_ctx crypt_ctx AES_ALIGN_ATTR;
>> +};
>> +
>> +static inline struct crypto_aes_ctx *aes_ctx(void *raw_ctx)
>> +{
>> + unsigned long addr = (unsigned long)raw_ctx;
>> + unsigned long align = AES_ALIGN;
>> +
>> + if (align <= crypto_tfm_ctx_alignment())
>> + align = 1;
>> +
>> + return (struct crypto_aes_ctx *)ALIGN(addr, align);
>> +}
>
> It seems you took my suggestion to fix the definition of struct aes_xts_ctx, but
> you didn't make the corresponding change to the runtime alignment code.
Sigh. This particular change was unintentionally leaked here from my WIP
code. Yes, I'm aware of your comment:
> The runtime alignment to a 16-byte boundary should happen when
translating the
> raw crypto_skcipher_ctx() into the pointer to the aes_xts_ctx. It
should not
> happen when accessing each individual field in the aes_xts_ctx.
> There should be a helper function aes_xts_ctx() that is used like:
>
> struct aes_xts_ctx *ctx = aes_xts_ctx(tfm);
>
> It would do the runtime alignment. Then, aes_ctx() should be removed.
Yes, I could think of some changes like the one below. I guess the aeskl
code can live with it. The aesni glue code still wants aes_cts() as it
deals with other modes. Then, that can be left there as it is.
diff --git a/arch/x86/crypto/aes-intel_glue.h
b/arch/x86/crypto/aes-intel_glue.h
index 5877d0988e36..b22de77594fe 100644
--- a/arch/x86/crypto/aes-intel_glue.h
+++ b/arch/x86/crypto/aes-intel_glue.h
@@ -31,15 +31,15 @@ struct aes_xts_ctx {
struct crypto_aes_ctx crypt_ctx AES_ALIGN_ATTR;
};
-static inline struct crypto_aes_ctx *aes_ctx(void *raw_ctx)
+static inline struct aes_xts_ctx *aes_xts_ctx(struct crypto_skcipher *tfm)
{
- unsigned long addr = (unsigned long)raw_ctx;
+ unsigned long addr = (unsigned long)crypto_skcipher_ctx(tfm);
unsigned long align = AES_ALIGN;
if (align <= crypto_tfm_ctx_alignment())
align = 1;
- return (struct crypto_aes_ctx *)ALIGN(addr, align);
+ return (struct aes_xts_ctx *)ALIGN(addr, align);
}
static inline int
@@ -47,7 +47,7 @@ xts_setkey_common(struct crypto_skcipher *tfm, const
u8 *key, unsigned int keyle
int (*fn)(struct crypto_tfm *tfm, void *ctx, const u8
*in_key,
unsigned int key_len))
{
- struct aes_xts_ctx *ctx = crypto_skcipher_ctx(tfm);
+ struct aes_xts_ctx *ctx = aes_xts_ctx(tfm);
int err;
err = xts_verify_key(tfm, key, keylen);
@@ -72,7 +72,7 @@ xts_crypt_common(struct skcipher_request *req,
int (*crypt1_fn)(const void *ctx, u8 *out, const u8 *in))
{
struct crypto_skcipher *tfm = crypto_skcipher_reqtfm(req);
- struct aes_xts_ctx *ctx = crypto_skcipher_ctx(tfm);
+ struct aes_xts_ctx *ctx = aes_xts_ctx(tfm);
int tail = req->cryptlen % AES_BLOCK_SIZE;
struct skcipher_request subreq;
struct skcipher_walk walk;
@@ -108,7 +108,7 @@ xts_crypt_common(struct skcipher_request *req,
kernel_fpu_begin();
/* calculate first value of T */
- err = crypt1_fn(aes_ctx(&ctx->tweak_ctx), walk.iv, walk.iv);
+ err = crypt1_fn(&ctx->tweak_ctx, walk.iv, walk.iv);
if (err) {
kernel_fpu_end();
return err;
@@ -120,7 +120,7 @@ xts_crypt_common(struct skcipher_request *req,
if (nbytes < walk.total)
nbytes &= ~(AES_BLOCK_SIZE - 1);
- err = crypt_fn(aes_ctx(&ctx->crypt_ctx),
walk.dst.virt.addr, walk.src.virt.addr,
+ err = crypt_fn(&ctx->crypt_ctx, walk.dst.virt.addr,
walk.src.virt.addr,
nbytes, walk.iv);
kernel_fpu_end();
if (err)
@@ -148,7 +148,7 @@ xts_crypt_common(struct skcipher_request *req,
return err;
kernel_fpu_begin();
- err = crypt_fn(aes_ctx(&ctx->crypt_ctx),
walk.dst.virt.addr, walk.src.virt.addr,
+ err = crypt_fn(&ctx->crypt_ctx, walk.dst.virt.addr,
walk.src.virt.addr,
walk.nbytes, walk.iv);
kernel_fpu_end();
if (err)
Thanks,
Chang
Powered by blists - more mailing lists