Re: [PATCH 05/13] crypto: caam/qi - handle large number of S/Gs case

2017-07-09 Thread Horia Geantă
On 7/7/2017 4:06 PM, Horia Geantă wrote:
> 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

Looks like I forgot to add a check in one of the places -
ablkcipher_giv_edesc_alloc(). Will fix this in v2.

Horia


[PATCH 05/13] crypto: caam/qi - handle large number of S/Gs case

2017-07-07 Thread Horia Geantă
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 800021cb6003 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:
[] dump_backtrace+0x0/0x290
[] show_stack+0x14/0x1c
[] dump_stack+0xa4/0xc8
[] print_address_description+0x110/0x26c
[] kasan_report+0x1d0/0x2fc
[] __asan_store1+0x4c/0x54
[] ablkcipher_edesc_alloc+0x4ec/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

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 800021cb5e00
 which belongs to the cache caamqicache of size 512
The buggy address is located 3 bytes to the right of
 512-byte region [800021cb5e00, 800021cb6000)
The buggy address belongs to the page:
page:7e872d00 count:1 mapcount:0 mapping:  (null)
index:0x0 compound_mapcount: 0
flags: 0xfffc0008100(slab|head)
raw: 0fffc0008100   000180190019
raw: dead0100 dead0200 800931268200 
page dumped because: kasan: bad access detected

Memory state around the buggy address:
 800021cb5f00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 800021cb5f80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>800021cb6000: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
   ^
 800021cb6080: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
 800021cb6100: 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ă 
---
 drivers/crypto/caam/caamalg_qi.c | 24 +++-
 drivers/crypto/caam/qi.c |  3 ---
 drivers/crypto/caam/qi.h |  3 +++
 3 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/drivers/crypto/caam/caamalg_qi.c b/drivers/crypto/caam/caamalg_qi.c
index a5d634e0aef3..4ec8823ccc1a 100644
--- 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_alloc(struct 
aead_request *req,
 */
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);
+