Blob Blame History Raw
From eb9ba37dc15a6e6b6140eb6d62785ba99b7179d7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Horia=20Geant=C4=83?= <horia.geanta@nxp.com>
Date: Mon, 10 Jul 2017 08:40:31 +0300
Subject: [PATCH] crypto: caam/qi - handle large number of S/Gs case
Mime-version: 1.0
Content-type: text/plain; charset=UTF-8
Content-transfer-encoding: 8bit
Git-commit: eb9ba37dc15a6e6b6140eb6d62785ba99b7179d7
Patch-mainline: v4.14-rc1
References: bsc#1051510

For more than 16 S/G entries, driver currently corrupts memory
on ARMv8, see below KASAN log.
Note: this does not reproduce on PowerPC due to different (smaller)
cache line size - 64 bytes on PPC vs. 128 bytes on ARMv8.

One such use case is one of the cbc(aes) test vectors - with 8 S/G
entries and src != dst. Driver needs 1 (IV) + 2 x 8 = 17 entries,
which goes over the 16 S/G entries limit:
(CAAM_QI_MEMCACHE_SIZE - offsetof(struct ablkcipher_edesc, sgt)) /
sizeof(struct qm_sg_entry) = 256 / 16 = 16 S/Gs

Fix this by:
-increasing object size in caamqicache pool from 512 to 768; this means
the maximum number of S/G entries grows from (at least) 16 to 32
(again, for ARMv8 case of 128-byte cache line)
-add checks in the driver to fail gracefully (ENOMEM) in case the 32 S/G
entries limit is exceeded

==================================================================
Bug: KASAN: slab-out-of-bounds in ablkcipher_edesc_alloc+0x4ec/0xf60
Write of size 1 at addr ffff800021cb6003 by task cryptomgr_test/1394

Cpu: 3 PID: 1394 Comm: cryptomgr_test Not tainted 4.12.0-rc7-next-20170703-00023-g72badbcc1ea7-dirty #26
Hardware name: LS1046A RDB Board (DT)
Call trace:
[<ffff20000808ac6c>] dump_backtrace+0x0/0x290
[<ffff20000808b014>] show_stack+0x14/0x1c
[<ffff200008d62c00>] dump_stack+0xa4/0xc8
[<ffff200008264e40>] print_address_description+0x110/0x26c
[<ffff200008265224>] kasan_report+0x1d0/0x2fc
[<ffff2000082637b8>] __asan_store1+0x4c/0x54
[<ffff200008b4884c>] ablkcipher_edesc_alloc+0x4ec/0xf60
[<ffff200008b49304>] ablkcipher_encrypt+0x44/0xcc
[<ffff20000848a61c>] skcipher_encrypt_ablkcipher+0x120/0x138
[<ffff200008495014>] __test_skcipher+0xaec/0xe30
[<ffff200008497088>] test_skcipher+0x6c/0xd8
[<ffff200008497154>] alg_test_skcipher+0x60/0xe4
[<ffff2000084974c4>] alg_test.part.13+0x130/0x304
[<ffff2000084976d4>] alg_test+0x3c/0x68
[<ffff2000084938ac>] cryptomgr_test+0x54/0x5c
[<ffff20000810276c>] kthread+0x188/0x1c8
[<ffff2000080836c0>] ret_from_fork+0x10/0x50

Allocated by task 1394:
 save_stack_trace_tsk+0x0/0x1ac
 save_stack_trace+0x18/0x20
 kasan_kmalloc.part.5+0x48/0x110
 kasan_kmalloc+0x84/0xa0
 kasan_slab_alloc+0x14/0x1c
 kmem_cache_alloc+0x124/0x1e8
 qi_cache_alloc+0x28/0x58
 ablkcipher_edesc_alloc+0x244/0xf60
 ablkcipher_encrypt+0x44/0xcc
 skcipher_encrypt_ablkcipher+0x120/0x138
 __test_skcipher+0xaec/0xe30
 test_skcipher+0x6c/0xd8
 alg_test_skcipher+0x60/0xe4
 alg_test.part.13+0x130/0x304
 alg_test+0x3c/0x68
 cryptomgr_test+0x54/0x5c
 kthread+0x188/0x1c8
 ret_from_fork+0x10/0x50

Freed by task 0:
(stack is not available)

The buggy address belongs to the object at ffff800021cb5e00
 which belongs to the cache caamqicache of size 512
The buggy address is located 3 bytes to the right of
 512-byte region [ffff800021cb5e00, ffff800021cb6000)
The buggy address belongs to the page:
page:ffff7e0000872d00 count:1 mapcount:0 mapping:          (null)
index:0x0 compound_mapcount: 0
Flags: 0xfffc00000008100(slab|head)
Raw: 0fffc00000008100 0000000000000000 0000000000000000 0000000180190019
Raw: dead000000000100 dead000000000200 ffff800931268200 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
 ffff800021cb5f00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 ffff800021cb5f80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>ffff800021cb6000: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc                   ^ ffff800021cb6080: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc ffff800021cb6100: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
==================================================================

Fixes: b189817cf789 ("crypto: caam/qi - add ablkcipher and authenc algorithms")
Signed-off-by: Horia Geantă <horia.geanta@nxp.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Acked-by: Takashi Iwai <tiwai@suse.de>

---
 drivers/crypto/caam/caamalg_qi.c |   32 +++++++++++++++++++++++++++++++-
 drivers/crypto/caam/qi.c         |    3 ---
 drivers/crypto/caam/qi.h         |    3 +++
 3 files changed, 34 insertions(+), 4 deletions(-)

--- a/drivers/crypto/caam/caamalg_qi.c
+++ b/drivers/crypto/caam/caamalg_qi.c
@@ -411,6 +411,9 @@ struct aead_edesc {
 	dma_addr_t qm_sg_dma;
 	dma_addr_t assoclen_dma;
 	struct caam_drv_req drv_req;
+#define CAAM_QI_MAX_AEAD_SG						\
+	((CAAM_QI_MEMCACHE_SIZE - offsetof(struct aead_edesc, sgt)) /	\
+	 sizeof(struct qm_sg_entry))
 	struct qm_sg_entry sgt[0];
 };
 
@@ -431,6 +434,9 @@ struct ablkcipher_edesc {
 	int qm_sg_bytes;
 	dma_addr_t qm_sg_dma;
 	struct caam_drv_req drv_req;
+#define CAAM_QI_MAX_ABLKCIPHER_SG					    \
+	((CAAM_QI_MEMCACHE_SIZE - offsetof(struct ablkcipher_edesc, sgt)) / \
+	 sizeof(struct qm_sg_entry))
 	struct qm_sg_entry sgt[0];
 };
 
@@ -660,6 +666,14 @@ static struct aead_edesc *aead_edesc_all
 	 */
 	qm_sg_ents = 1 + !!ivsize + mapped_src_nents +
 		     (mapped_dst_nents > 1 ? mapped_dst_nents : 0);
+	if (unlikely(qm_sg_ents > CAAM_QI_MAX_AEAD_SG)) {
+		dev_err(qidev, "Insufficient S/G entries: %d > %lu\n",
+			qm_sg_ents, CAAM_QI_MAX_AEAD_SG);
+		caam_unmap(qidev, req->src, req->dst, src_nents, dst_nents,
+			   iv_dma, ivsize, op_type, 0, 0);
+		qi_cache_free(edesc);
+		return ERR_PTR(-ENOMEM);
+	}
 	sg_table = &edesc->sgt[0];
 	qm_sg_bytes = qm_sg_ents * sizeof(*sg_table);
 
@@ -888,6 +902,15 @@ static struct ablkcipher_edesc *ablkciph
 	}
 	dst_sg_idx = qm_sg_ents;
 
+	qm_sg_ents += mapped_dst_nents > 1 ? mapped_dst_nents : 0;
+	if (unlikely(qm_sg_ents > CAAM_QI_MAX_ABLKCIPHER_SG)) {
+		dev_err(qidev, "Insufficient S/G entries: %d > %lu\n",
+			qm_sg_ents, CAAM_QI_MAX_ABLKCIPHER_SG);
+		caam_unmap(qidev, req->src, req->dst, src_nents, dst_nents,
+			   iv_dma, ivsize, op_type, 0, 0);
+		return ERR_PTR(-ENOMEM);
+	}
+
 	/* allocate space for base edesc and link tables */
 	edesc = qi_cache_alloc(GFP_DMA | flags);
 	if (unlikely(!edesc)) {
@@ -900,7 +923,6 @@ static struct ablkcipher_edesc *ablkciph
 	edesc->src_nents = src_nents;
 	edesc->dst_nents = dst_nents;
 	edesc->iv_dma = iv_dma;
-	qm_sg_ents += mapped_dst_nents > 1 ? mapped_dst_nents : 0;
 	sg_table = &edesc->sgt[0];
 	edesc->qm_sg_bytes = qm_sg_ents * sizeof(*sg_table);
 	edesc->drv_req.app_ctx = req;
@@ -1035,6 +1057,14 @@ static struct ablkcipher_edesc *ablkciph
 		qm_sg_ents += 1 + mapped_dst_nents;
 	}
 
+	if (unlikely(qm_sg_ents > CAAM_QI_MAX_ABLKCIPHER_SG)) {
+		dev_err(qidev, "Insufficient S/G entries: %d > %lu\n",
+			qm_sg_ents, CAAM_QI_MAX_ABLKCIPHER_SG);
+		caam_unmap(qidev, req->src, req->dst, src_nents, dst_nents,
+			   iv_dma, ivsize, GIVENCRYPT, 0, 0);
+		return ERR_PTR(-ENOMEM);
+	}
+
 	/* allocate space for base edesc and link tables */
 	edesc = qi_cache_alloc(GFP_DMA | flags);
 	if (!edesc) {
--- a/drivers/crypto/caam/qi.c
+++ b/drivers/crypto/caam/qi.c
@@ -24,9 +24,6 @@
  */
 #define MAX_RSP_FQ_BACKLOG_PER_CPU	256
 
-/* Length of a single buffer in the QI driver memory cache */
-#define CAAM_QI_MEMCACHE_SIZE	512
-
 #define CAAM_QI_ENQUEUE_RETRIES	10000
 
 #define CAAM_NAPI_WEIGHT	63
--- a/drivers/crypto/caam/qi.h
+++ b/drivers/crypto/caam/qi.h
@@ -39,6 +39,9 @@
  */
 #define MAX_SDLEN	((CAAM_DESC_BYTES_MAX - DESC_JOB_IO_LEN) / CAAM_CMD_SZ)
 
+/* Length of a single buffer in the QI driver memory cache */
+#define CAAM_QI_MEMCACHE_SIZE	768
+
 extern bool caam_congested __read_mostly;
 
 /*