On Fri, 14 Jun 2019 at 11:42, Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx> wrote:
>
> The AIRO driver applies a ctr(aes) on a buffer of considerable size
> (2400 bytes), and instead of invoking the crypto API to handle this
> in its entirety, it open codes the counter manipulation and invokes
> the AES block cipher directly.
>
> Let's fix this, by switching to the sync skcipher API instead.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx>
> ---
> NOTE: build tested only, since I don't have the hardware
>
Greg, please disregard - I sent the wrong patch twice by accident (and
cc'ed you the second time)
> drivers/net/wireless/cisco/airo.c | 57 ++++++++++----------
> 1 file changed, 27 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/net/wireless/cisco/airo.c b/drivers/net/wireless/cisco/airo.c
> index 3f5a14112c6b..2d29ad10505b 100644
> --- a/drivers/net/wireless/cisco/airo.c
> +++ b/drivers/net/wireless/cisco/airo.c
> @@ -49,6 +49,9 @@
> #include <linux/kthread.h>
> #include <linux/freezer.h>
>
> +#include <crypto/aes.h>
> +#include <crypto/skcipher.h>
> +
> #include <net/cfg80211.h>
> #include <net/iw_handler.h>
>
> @@ -951,7 +954,7 @@ typedef struct {
> } mic_statistics;
>
> typedef struct {
> - u32 coeff[((EMMH32_MSGLEN_MAX)+3)>>2];
> + __be32 coeff[((EMMH32_MSGLEN_MAX)+3)>>2];
> u64 accum; // accumulated mic, reduced to u32 in final()
> int position; // current position (byte offset) in message
> union {
> @@ -1216,7 +1219,7 @@ struct airo_info {
> struct iw_spy_data spy_data;
> struct iw_public_data wireless_data;
> /* MIC stuff */
> - struct crypto_cipher *tfm;
> + struct crypto_sync_skcipher *tfm;
> mic_module mod[2];
> mic_statistics micstats;
> HostRxDesc rxfids[MPI_MAX_FIDS]; // rx/tx/config MPI350 descriptors
> @@ -1291,14 +1294,14 @@ static int flashrestart(struct airo_info *ai,struct net_device *dev);
> static int RxSeqValid (struct airo_info *ai,miccntx *context,int mcast,u32 micSeq);
> static void MoveWindow(miccntx *context, u32 micSeq);
> static void emmh32_setseed(emmh32_context *context, u8 *pkey, int keylen,
> - struct crypto_cipher *tfm);
> + struct crypto_sync_skcipher *tfm);
> static void emmh32_init(emmh32_context *context);
> static void emmh32_update(emmh32_context *context, u8 *pOctets, int len);
> static void emmh32_final(emmh32_context *context, u8 digest[4]);
> static int flashpchar(struct airo_info *ai,int byte,int dwelltime);
>
> static void age_mic_context(miccntx *cur, miccntx *old, u8 *key, int key_len,
> - struct crypto_cipher *tfm)
> + struct crypto_sync_skcipher *tfm)
> {
> /* If the current MIC context is valid and its key is the same as
> * the MIC register, there's nothing to do.
> @@ -1359,7 +1362,7 @@ static int micsetup(struct airo_info *ai) {
> int i;
>
> if (ai->tfm == NULL)
> - ai->tfm = crypto_alloc_cipher("aes", 0, 0);
> + ai->tfm = crypto_alloc_sync_skcipher("ctr(aes)", 0, 0);
>
> if (IS_ERR(ai->tfm)) {
> airo_print_err(ai->dev->name, "failed to load transform for AES");
> @@ -1624,37 +1627,31 @@ static void MoveWindow(miccntx *context, u32 micSeq)
>
> /* mic accumulate */
> #define MIC_ACCUM(val) \
> - context->accum += (u64)(val) * context->coeff[coeff_position++];
> -
> -static unsigned char aes_counter[16];
> + context->accum += (u64)(val) * be32_to_cpu(context->coeff[coeff_position++]);
>
> /* expand the key to fill the MMH coefficient array */
> static void emmh32_setseed(emmh32_context *context, u8 *pkey, int keylen,
> - struct crypto_cipher *tfm)
> + struct crypto_sync_skcipher *tfm)
> {
> /* take the keying material, expand if necessary, truncate at 16-bytes */
> /* run through AES counter mode to generate context->coeff[] */
>
> - int i,j;
> - u32 counter;
> - u8 *cipher, plain[16];
> -
> - crypto_cipher_setkey(tfm, pkey, 16);
> - counter = 0;
> - for (i = 0; i < ARRAY_SIZE(context->coeff); ) {
> - aes_counter[15] = (u8)(counter >> 0);
> - aes_counter[14] = (u8)(counter >> 8);
> - aes_counter[13] = (u8)(counter >> 16);
> - aes_counter[12] = (u8)(counter >> 24);
> - counter++;
> - memcpy (plain, aes_counter, 16);
> - crypto_cipher_encrypt_one(tfm, plain, plain);
> - cipher = plain;
> - for (j = 0; (j < 16) && (i < ARRAY_SIZE(context->coeff)); ) {
> - context->coeff[i++] = ntohl(*(__be32 *)&cipher[j]);
> - j += 4;
> - }
> - }
> + SYNC_SKCIPHER_REQUEST_ON_STACK(req, tfm);
> + struct scatterlist dst, src;
> + u8 iv[AES_BLOCK_SIZE] = {};
> + int ret;
> +
> + crypto_sync_skcipher_setkey(tfm, pkey, 16);
> +
> + sg_init_one(&dst, context->coeff, sizeof(context->coeff));
> + sg_init_one(&src, page_address(ZERO_PAGE(0)), sizeof(context->coeff));
> +
> + skcipher_request_set_sync_tfm(req, tfm);
> + skcipher_request_set_callback(req, 0, NULL, NULL);
> + skcipher_request_set_crypt(req, &src, &dst, sizeof(context->coeff), iv);
> +
> + ret = crypto_skcipher_encrypt(req);
> + WARN_ON_ONCE(ret);
> }
>
> /* prepare for calculation of a new mic */
> @@ -2415,7 +2412,7 @@ void stop_airo_card( struct net_device *dev, int freeres )
> ai->shared, ai->shared_dma);
> }
> }
> - crypto_free_cipher(ai->tfm);
> + crypto_free_sync_skcipher(ai->tfm);
> del_airo_dev(ai);
> free_netdev( dev );
> }
> --
> 2.20.1
>
