| Message ID | 20260105051311.1607207-20-ebiggers@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | AES library improvements | expand |
> /* Most significant octet of plaintextData corresponds to data[0] */ > swap_buf(r, data, 16); > > - aes_encrypt(&ctx, data, data); + aes_encrypt_new(&aes, data, data); One thing you might want to consider, which reduces the churn in the series. You can use _Generic() to do type-based dispatch on the first pointer. Something like this: void aes_encrypt(const struct crypto_aes_ctx *ctx, u8 *out, const u8 *in); void aes_encrypt_new(aes_encrypt_arg key, u8 out[at_least AES_BLOCK_SIZE], const u8 in[at_least AES_BLOCK_SIZE]); #define aes_encrypt(ctx, out, in) \ _Generic(ctx, \ const struct crypto_aes_ctx *: aes_encrypt(ctx, out, in), \ aes_encrypt_arg: aes_encrypt_new(ctx, out, in)) i.e. it keeps the _new()-ism in a single header, without needing to change the drivers a second time. ~Andrew
On Mon, 5 Jan 2026 15:40:22 +0000 Andrew Cooper <andrew.cooper3@citrix.com> wrote: > > /* Most significant octet of plaintextData corresponds to data[0] */ > > swap_buf(r, data, 16); > > > > - aes_encrypt(&ctx, data, data); + aes_encrypt_new(&aes, data, data); > > One thing you might want to consider, which reduces the churn in the series. > > You can use _Generic() to do type-based dispatch on the first pointer. > Something like this: > > void aes_encrypt(const struct crypto_aes_ctx *ctx, u8 *out, const u8 *in); > void aes_encrypt_new(aes_encrypt_arg key, u8 out[at_least AES_BLOCK_SIZE], > const u8 in[at_least AES_BLOCK_SIZE]); > > #define aes_encrypt(ctx, out, in) \ > _Generic(ctx, \ > const struct crypto_aes_ctx *: aes_encrypt(ctx, out, in), \ > aes_encrypt_arg: aes_encrypt_new(ctx, out, in)) > > > i.e. it keeps the _new()-ism in a single header, without needing to > change the drivers a second time. You'll need to cast the 'ctx' argument in both calls. All the code in an _Generic() must compile cleanly in all the cases. (Totally annoying....) David > > ~Andrew >
On Mon, Jan 05, 2026 at 07:05:03PM +0000, David Laight wrote: > On Mon, 5 Jan 2026 15:40:22 +0000 > Andrew Cooper <andrew.cooper3@citrix.com> wrote: > > > > /* Most significant octet of plaintextData corresponds to data[0] */ > > > swap_buf(r, data, 16); > > > > > > - aes_encrypt(&ctx, data, data); + aes_encrypt_new(&aes, data, data); > > > > One thing you might want to consider, which reduces the churn in the series. > > > > You can use _Generic() to do type-based dispatch on the first pointer. > > Something like this: > > > > void aes_encrypt(const struct crypto_aes_ctx *ctx, u8 *out, const u8 *in); > > void aes_encrypt_new(aes_encrypt_arg key, u8 out[at_least AES_BLOCK_SIZE], > > const u8 in[at_least AES_BLOCK_SIZE]); > > > > #define aes_encrypt(ctx, out, in) \ > > _Generic(ctx, \ > > const struct crypto_aes_ctx *: aes_encrypt(ctx, out, in), \ > > aes_encrypt_arg: aes_encrypt_new(ctx, out, in)) > > > > > > i.e. it keeps the _new()-ism in a single header, without needing to > > change the drivers a second time. > > You'll need to cast the 'ctx' argument in both calls. > All the code in an _Generic() must compile cleanly in all the cases. > (Totally annoying....) > > David It seems it would actually have to be: #define aes_encrypt(key, out, in) \ _Generic(key, \ struct crypto_aes_ctx *: aes_encrypt_old((const struct crypto_aes_ctx *)key, out, in), \ const struct crypto_aes_ctx *: aes_encrypt_old((const struct crypto_aes_ctx *)key, out, in), \ struct aes_enckey *: aes_encrypt_new((const struct aes_enckey *)key, out, in), \ const struct aes_enckey *: aes_encrypt_new((const struct aes_enckey *)key, out, in), \ struct aes_key *: aes_encrypt_new((const struct aes_key *)key, out, in), \ const struct aes_key *: aes_encrypt_new((const struct aes_key *)key, out, in)) #define aes_decrypt(key, out, in) \ _Generic(key, \ struct crypto_aes_ctx *: aes_decrypt_old((const struct crypto_aes_ctx *)key, out, in), \ const struct crypto_aes_ctx *: aes_decrypt_old((const struct crypto_aes_ctx *)key, out, in), \ struct aes_key *: aes_decrypt_new((const struct aes_key *)key, out, in), \ const struct aes_key *: aes_decrypt_new((const struct aes_key *)key, out, in)) Note that both const and non-const args need to be handled. It also doesn't work for any callers passing a 'void *' or 'const void *' and relying on an implicit cast. I didn't notice any, but that needs to be considered too. I guess maybe it would still be worth it to avoid the "*_new" name temporarily leaking into too many files. (It goes away by the end of the series anyway.) It's just not quite as simple as you're suggesting, and all the callers have to be checked for compatibility with it. - Eric
diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c index 3a1ce04a7a53..69007e510177 100644 --- a/net/bluetooth/smp.c +++ b/net/bluetooth/smp.c @@ -372,36 +372,36 @@ static int smp_h7(struct crypto_shash *tfm_cmac, const u8 w[16], * s1 and ah. */ static int smp_e(const u8 *k, u8 *r) { - struct crypto_aes_ctx ctx; + struct aes_enckey aes; uint8_t tmp[16], data[16]; int err; SMP_DBG("k %16phN r %16phN", k, r); /* The most significant octet of key corresponds to k[0] */ swap_buf(k, tmp, 16); - err = aes_expandkey(&ctx, tmp, 16); + err = aes_prepareenckey(&aes, tmp, 16); if (err) { BT_ERR("cipher setkey failed: %d", err); return err; } /* Most significant octet of plaintextData corresponds to data[0] */ swap_buf(r, data, 16); - aes_encrypt(&ctx, data, data); + aes_encrypt_new(&aes, data, data); /* Most significant octet of encryptedData corresponds to data[0] */ swap_buf(data, r, 16); SMP_DBG("r %16phN", r); - memzero_explicit(&ctx, sizeof(ctx)); + memzero_explicit(&aes, sizeof(aes)); return err; } static int smp_c1(const u8 k[16], const u8 r[16], const u8 preq[7], const u8 pres[7], u8 _iat,
Switch from the old AES library functions (which use struct crypto_aes_ctx) to the new ones (which use struct aes_enckey). This eliminates the unnecessary computation and caching of the decryption round keys. The new AES en/decryption functions are also much faster and use AES instructions when supported by the CPU. Note: aes_encrypt_new() will be renamed to aes_encrypt() once all callers of the old aes_encrypt() have been updated. Signed-off-by: Eric Biggers <ebiggers@kernel.org> --- net/bluetooth/smp.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)