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
