From 607150bd1fbf6a497a6326e57047e11bac35cfe7 Mon Sep 17 00:00:00 2001 From: Tudor-Dan Ambarus Date: Fri, 29 Sep 2017 12:21:04 +0300 Subject: crypto: dh - return unsigned int for dh_data_size() p->key_size, p->p_size, p->g_size are all of unsigned int type. Signed-off-by: Tudor Ambarus Signed-off-by: Herbert Xu --- crypto/dh_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'crypto/dh_helper.c') diff --git a/crypto/dh_helper.c b/crypto/dh_helper.c index 8ba8a3f8..69869dad 100644 --- a/crypto/dh_helper.c +++ b/crypto/dh_helper.c @@ -28,7 +28,7 @@ static inline const u8 *dh_unpack_data(void *dst, const void *src, size_t size) return src + size; } -static inline int dh_data_size(const struct dh *p) +static inline unsigned int dh_data_size(const struct dh *p) { return p->key_size + p->p_size + p->g_size; } -- cgit v1.2.3 From 42b792b9a8e523e82c13996149f527a4b27436c4 Mon Sep 17 00:00:00 2001 From: Tudor-Dan Ambarus Date: Fri, 29 Sep 2017 12:21:05 +0300 Subject: crypto: dh - return unsigned value for crypto_dh_key_len() DH_KPP_SECRET_MIN_SIZE and dh_data_size() are both returning unsigned values. Signed-off-by: Tudor Ambarus Signed-off-by: Herbert Xu --- crypto/dh_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'crypto/dh_helper.c') diff --git a/crypto/dh_helper.c b/crypto/dh_helper.c index 69869dad..a413b311 100644 --- a/crypto/dh_helper.c +++ b/crypto/dh_helper.c @@ -33,7 +33,7 @@ static inline unsigned int dh_data_size(const struct dh *p) return p->key_size + p->p_size + p->g_size; } -int crypto_dh_key_len(const struct dh *p) +unsigned int crypto_dh_key_len(const struct dh *p) { return DH_KPP_SECRET_MIN_SIZE + dh_data_size(p); } -- cgit v1.2.3 From 50f1c397040be62b563445de89dc1c563ce95c90 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Sun, 5 Nov 2017 18:30:45 -0800 Subject: crypto: dh - Don't permit 'p' to be 0 If 'p' is 0 for the software Diffie-Hellman implementation, then dh_max_size() returns 0. In the case of KEYCTL_DH_COMPUTE, this causes ZERO_SIZE_PTR to be passed to sg_init_one(), which with CONFIG_DEBUG_SG=y triggers the 'BUG_ON(!virt_addr_valid(buf));' in sg_set_buf(). Fix this by making crypto_dh_decode_key() reject 0 for 'p'. p=0 makes no sense for any DH implementation because 'p' is supposed to be a prime number. Moreover, 'mod 0' is not mathematically defined. Bug report: kernel BUG at ./include/linux/scatterlist.h:140! invalid opcode: 0000 [#1] SMP KASAN CPU: 0 PID: 27112 Comm: syz-executor2 Not tainted 4.14.0-rc7-00010-gf5dbb5d0ce32-dirty #7 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.3-20171021_125229-anatol 04/01/2014 task: ffff88006caac0c0 task.stack: ffff88006c7c8000 RIP: 0010:sg_set_buf include/linux/scatterlist.h:140 [inline] RIP: 0010:sg_init_one+0x1b3/0x240 lib/scatterlist.c:156 RSP: 0018:ffff88006c7cfb08 EFLAGS: 00010216 RAX: 0000000000010000 RBX: ffff88006c7cfe30 RCX: 00000000000064ee RDX: ffffffff81cf64c3 RSI: ffffc90000d72000 RDI: ffffffff92e937e0 RBP: ffff88006c7cfb30 R08: ffffed000d8f9fab R09: ffff88006c7cfd30 R10: 0000000000000005 R11: ffffed000d8f9faa R12: ffff88006c7cfd30 R13: 0000000000000000 R14: 0000000000000010 R15: ffff88006c7cfc50 FS: 00007fce190fa700(0000) GS:ffff88003ea00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007fffc6b33db8 CR3: 000000003cf64000 CR4: 00000000000006f0 Call Trace: __keyctl_dh_compute+0xa95/0x19b0 security/keys/dh.c:360 keyctl_dh_compute+0xac/0x100 security/keys/dh.c:434 SYSC_keyctl security/keys/keyctl.c:1745 [inline] SyS_keyctl+0x72/0x2c0 security/keys/keyctl.c:1641 entry_SYSCALL_64_fastpath+0x1f/0xbe RIP: 0033:0x4585c9 RSP: 002b:00007fce190f9bd8 EFLAGS: 00000216 ORIG_RAX: 00000000000000fa RAX: ffffffffffffffda RBX: 0000000000738020 RCX: 00000000004585c9 RDX: 000000002000d000 RSI: 0000000020000ff4 RDI: 0000000000000017 RBP: 0000000000000046 R08: 0000000020008000 R09: 0000000000000000 R10: 0000000000000000 R11: 0000000000000216 R12: 00007fff6e610cde R13: 00007fff6e610cdf R14: 00007fce190fa700 R15: 0000000000000000 Code: 03 0f b6 14 02 48 89 f8 83 e0 07 83 c0 03 38 d0 7c 04 84 d2 75 33 5b 45 89 6c 24 14 41 5c 41 5d 41 5e 41 5f 5d c3 e8 fd 8f 68 ff <0f> 0b e8 f6 8f 68 ff 0f 0b e8 ef 8f 68 ff 0f 0b e8 e8 8f 68 ff 20 RIP: sg_set_buf include/linux/scatterlist.h:140 [inline] RSP: ffff88006c7cfb08 RIP: sg_init_one+0x1b3/0x240 lib/scatterlist.c:156 RSP: ffff88006c7cfb08 Fixes: 2c3e38eb72d5 ("crypto: dh - Add DH software implementation") Cc: # v4.8+ Reviewed-by: Tudor Ambarus Signed-off-by: Eric Biggers Signed-off-by: Herbert Xu --- crypto/dh_helper.c | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'crypto/dh_helper.c') diff --git a/crypto/dh_helper.c b/crypto/dh_helper.c index a413b311..788a9e57 100644 --- a/crypto/dh_helper.c +++ b/crypto/dh_helper.c @@ -90,6 +90,14 @@ int crypto_dh_decode_key(const char *buf, unsigned int len, struct dh *params) params->p = (void *)(ptr + params->key_size); params->g = (void *)(ptr + params->key_size + params->p_size); + /* + * Don't permit 'p' to be 0. It's not a prime number, and it's subject + * to corner cases such as 'mod 0' being undefined or + * crypto_kpp_maxsize() returning 0. + */ + if (memchr_inv(params->p, 0, params->p_size) == NULL) + return -EINVAL; + return 0; } EXPORT_SYMBOL_GPL(crypto_dh_decode_key); -- cgit v1.2.3 From 246c7e6f3567aba7edb45a90cc662382c4d0acca Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Sun, 5 Nov 2017 18:30:46 -0800 Subject: crypto: dh - Don't permit 'key' or 'g' size longer than 'p' The "qat-dh" DH implementation assumes that 'key' and 'g' can be copied into a buffer with size 'p_size'. However it was never checked that that was actually the case, which most likely allowed users to cause a buffer underflow via KEYCTL_DH_COMPUTE. Fix this by updating crypto_dh_decode_key() to verify this precondition for all DH implementations. Fixes: c9839143ebbf ("crypto: qat - Add DH support") Cc: # v4.8+ Signed-off-by: Eric Biggers Reviewed-by: Tudor Ambarus Signed-off-by: Herbert Xu --- crypto/dh_helper.c | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'crypto/dh_helper.c') diff --git a/crypto/dh_helper.c b/crypto/dh_helper.c index 788a9e57..24fdb2ec 100644 --- a/crypto/dh_helper.c +++ b/crypto/dh_helper.c @@ -83,6 +83,14 @@ int crypto_dh_decode_key(const char *buf, unsigned int len, struct dh *params) if (secret.len != crypto_dh_key_len(params)) return -EINVAL; + /* + * Don't permit the buffer for 'key' or 'g' to be larger than 'p', since + * some drivers assume otherwise. + */ + if (params->key_size > params->p_size || + params->g_size > params->p_size) + return -EINVAL; + /* Don't allocate memory. Set pointers to data within * the given buffer */ -- cgit v1.2.3