From 7e92e1717e3eaf6b322c252947c696b3059f05be Mon Sep 17 00:00:00 2001 From: Christian Lamparter Date: Mon, 22 Apr 2019 13:25:59 +0200 Subject: crypto: crypto4xx - fix cfb and ofb "overran dst buffer" issues Currently, crypto4xx CFB and OFB AES ciphers are failing testmgr's test vectors. |cfb-aes-ppc4xx encryption overran dst buffer on test vector 3, cfg="in-place" |ofb-aes-ppc4xx encryption overran dst buffer on test vector 1, cfg="in-place" This is because of a very subtile "bug" in the hardware that gets indirectly mentioned in 18.1.3.5 Encryption/Decryption of the hardware spec: the OFB and CFB modes for AES are listed there as operation modes for >>> "Block ciphers" <<<. Which kind of makes sense, but we would like them to be considered as stream ciphers just like the CTR mode. To workaround this issue and stop the hardware from causing "overran dst buffer" on crypttexts that are not a multiple of 16 (AES_BLOCK_SIZE), we force the driver to use the scatter buffers as the go-between. As a bonus this patch also kills redundant pd_uinfo->num_gd and pd_uinfo->num_sd setters since the value has already been set before. Cc: stable@vger.kernel.org Fixes: f2a13e7cba9e ("crypto: crypto4xx - enable AES RFC3686, ECB, CFB and OFB offloads") Signed-off-by: Christian Lamparter Signed-off-by: Herbert Xu --- drivers/crypto/amcc/crypto4xx_core.c | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) (limited to 'drivers/crypto/amcc/crypto4xx_core.c') diff --git a/drivers/crypto/amcc/crypto4xx_core.c b/drivers/crypto/amcc/crypto4xx_core.c index 06574a884715..920bd5e720b2 100644 --- a/drivers/crypto/amcc/crypto4xx_core.c +++ b/drivers/crypto/amcc/crypto4xx_core.c @@ -714,7 +714,23 @@ int crypto4xx_build_pd(struct crypto_async_request *req, size_t offset_to_sr_ptr; u32 gd_idx = 0; int tmp; - bool is_busy; + bool is_busy, force_sd; + + /* + * There's a very subtile/disguised "bug" in the hardware that + * gets indirectly mentioned in 18.1.3.5 Encryption/Decryption + * of the hardware spec: + * *drum roll* the AES/(T)DES OFB and CFB modes are listed as + * operation modes for >>> "Block ciphers" <<<. + * + * To workaround this issue and stop the hardware from causing + * "overran dst buffer" on crypttexts that are not a multiple + * of 16 (AES_BLOCK_SIZE), we force the driver to use the + * scatter buffers. + */ + force_sd = (req_sa->sa_command_1.bf.crypto_mode9_8 == CRYPTO_MODE_CFB + || req_sa->sa_command_1.bf.crypto_mode9_8 == CRYPTO_MODE_OFB) + && (datalen % AES_BLOCK_SIZE); /* figure how many gd are needed */ tmp = sg_nents_for_len(src, assoclen + datalen); @@ -732,7 +748,7 @@ int crypto4xx_build_pd(struct crypto_async_request *req, } /* figure how many sd are needed */ - if (sg_is_last(dst)) { + if (sg_is_last(dst) && force_sd == false) { num_sd = 0; } else { if (datalen > PPC4XX_SD_BUFFER_SIZE) { @@ -807,9 +823,10 @@ int crypto4xx_build_pd(struct crypto_async_request *req, pd->sa_len = sa_len; pd_uinfo = &dev->pdr_uinfo[pd_entry]; - pd_uinfo->async_req = req; pd_uinfo->num_gd = num_gd; pd_uinfo->num_sd = num_sd; + pd_uinfo->dest_va = dst; + pd_uinfo->async_req = req; if (iv_len) memcpy(pd_uinfo->sr_va->save_iv, iv, iv_len); @@ -828,7 +845,6 @@ int crypto4xx_build_pd(struct crypto_async_request *req, /* get first gd we are going to use */ gd_idx = fst_gd; pd_uinfo->first_gd = fst_gd; - pd_uinfo->num_gd = num_gd; gd = crypto4xx_get_gdp(dev, &gd_dma, gd_idx); pd->src = gd_dma; /* enable gather */ @@ -865,17 +881,14 @@ int crypto4xx_build_pd(struct crypto_async_request *req, * Indicate gather array is not used */ pd_uinfo->first_gd = 0xffffffff; - pd_uinfo->num_gd = 0; } - if (sg_is_last(dst)) { + if (!num_sd) { /* * we know application give us dst a whole piece of memory * no need to use scatter ring. */ pd_uinfo->using_sd = 0; pd_uinfo->first_sd = 0xffffffff; - pd_uinfo->num_sd = 0; - pd_uinfo->dest_va = dst; sa->sa_command_0.bf.scatter = 0; pd->dest = (u32)dma_map_page(dev->core_dev->device, sg_page(dst), dst->offset, @@ -889,9 +902,7 @@ int crypto4xx_build_pd(struct crypto_async_request *req, nbytes = datalen; sa->sa_command_0.bf.scatter = 1; pd_uinfo->using_sd = 1; - pd_uinfo->dest_va = dst; pd_uinfo->first_sd = fst_sd; - pd_uinfo->num_sd = num_sd; sd = crypto4xx_get_sdp(dev, &sd_dma, sd_idx); pd->dest = sd_dma; /* setup scatter descriptor */ -- cgit From 9848e4c873c1cae019bbef4ab3a500a05ca59fe6 Mon Sep 17 00:00:00 2001 From: Christian Lamparter Date: Mon, 22 Apr 2019 13:26:00 +0200 Subject: crypto: crypto4xx - use sync skcipher for fallback This replaces struct crypto_skcipher and the extra request size with struct crypto_sync_skcipher and SYNC_SKCIPHER_REQUEST_ON_STACK(), which uses a fixed stack size. Signed-off-by: Christian Lamparter Signed-off-by: Herbert Xu --- drivers/crypto/amcc/crypto4xx_core.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) (limited to 'drivers/crypto/amcc/crypto4xx_core.c') diff --git a/drivers/crypto/amcc/crypto4xx_core.c b/drivers/crypto/amcc/crypto4xx_core.c index 920bd5e720b2..3e7d24ff3fa6 100644 --- a/drivers/crypto/amcc/crypto4xx_core.c +++ b/drivers/crypto/amcc/crypto4xx_core.c @@ -965,15 +965,10 @@ static int crypto4xx_sk_init(struct crypto_skcipher *sk) if (alg->base.cra_flags & CRYPTO_ALG_NEED_FALLBACK) { ctx->sw_cipher.cipher = - crypto_alloc_skcipher(alg->base.cra_name, 0, - CRYPTO_ALG_NEED_FALLBACK | - CRYPTO_ALG_ASYNC); + crypto_alloc_sync_skcipher(alg->base.cra_name, 0, + CRYPTO_ALG_NEED_FALLBACK); if (IS_ERR(ctx->sw_cipher.cipher)) return PTR_ERR(ctx->sw_cipher.cipher); - - crypto_skcipher_set_reqsize(sk, - sizeof(struct skcipher_request) + 32 + - crypto_skcipher_reqsize(ctx->sw_cipher.cipher)); } amcc_alg = container_of(alg, struct crypto4xx_alg, alg.u.cipher); @@ -992,7 +987,7 @@ static void crypto4xx_sk_exit(struct crypto_skcipher *sk) crypto4xx_common_exit(ctx); if (ctx->sw_cipher.cipher) - crypto_free_skcipher(ctx->sw_cipher.cipher); + crypto_free_sync_skcipher(ctx->sw_cipher.cipher); } static int crypto4xx_aead_init(struct crypto_aead *tfm) -- cgit From 38cf5533d7a876f75088bacc1277046f30005f28 Mon Sep 17 00:00:00 2001 From: Christian Lamparter Date: Mon, 22 Apr 2019 13:26:01 +0200 Subject: crypto: crypto4xx - get rid of redundant using_sd variable using_sd is used as a stand-in for sa_command_0.bf.scatter that we need to set anyway, so we might as well just prevent double-accounting. Signed-off-by: Christian Lamparter Signed-off-by: Herbert Xu --- drivers/crypto/amcc/crypto4xx_core.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'drivers/crypto/amcc/crypto4xx_core.c') diff --git a/drivers/crypto/amcc/crypto4xx_core.c b/drivers/crypto/amcc/crypto4xx_core.c index 3e7d24ff3fa6..3934c2523762 100644 --- a/drivers/crypto/amcc/crypto4xx_core.c +++ b/drivers/crypto/amcc/crypto4xx_core.c @@ -539,7 +539,7 @@ static void crypto4xx_cipher_done(struct crypto4xx_device *dev, req = skcipher_request_cast(pd_uinfo->async_req); - if (pd_uinfo->using_sd) { + if (pd_uinfo->sa_va->sa_command_0.bf.scatter) { crypto4xx_copy_pkt_to_dst(dev, pd, pd_uinfo, req->cryptlen, req->dst); } else { @@ -593,7 +593,7 @@ static void crypto4xx_aead_done(struct crypto4xx_device *dev, u32 icv[AES_BLOCK_SIZE]; int err = 0; - if (pd_uinfo->using_sd) { + if (pd_uinfo->sa_va->sa_command_0.bf.scatter) { crypto4xx_copy_pkt_to_dst(dev, pd, pd_uinfo, pd->pd_ctl_len.bf.pkt_len, dst); @@ -887,7 +887,6 @@ int crypto4xx_build_pd(struct crypto_async_request *req, * we know application give us dst a whole piece of memory * no need to use scatter ring. */ - pd_uinfo->using_sd = 0; pd_uinfo->first_sd = 0xffffffff; sa->sa_command_0.bf.scatter = 0; pd->dest = (u32)dma_map_page(dev->core_dev->device, @@ -901,7 +900,6 @@ int crypto4xx_build_pd(struct crypto_async_request *req, u32 sd_idx = fst_sd; nbytes = datalen; sa->sa_command_0.bf.scatter = 1; - pd_uinfo->using_sd = 1; pd_uinfo->first_sd = fst_sd; sd = crypto4xx_get_sdp(dev, &sd_dma, sd_idx); pd->dest = sd_dma; -- cgit