Re: [PATCH 2/4] crypto: dh - don't permit 'p' to be 0

2017-11-02 Thread Tudor Ambarus



On 11/02/2017 12:25 AM, Eric Biggers wrote:

From: Eric Biggers 

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_POINTER 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:  [#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: 88006caac0c0 task.stack: 88006c7c8000
 RIP: 0010:sg_set_buf include/linux/scatterlist.h:140 [inline]
 RIP: 0010:sg_init_one+0x1b3/0x240 lib/scatterlist.c:156
 RSP: 0018:88006c7cfb08 EFLAGS: 00010216
 RAX: 0001 RBX: 88006c7cfe30 RCX: 64ee
 RDX: 81cf64c3 RSI: c9d72000 RDI: 92e937e0
 RBP: 88006c7cfb30 R08: ed000d8f9fab R09: 88006c7cfd30
 R10: 0005 R11: ed000d8f9faa R12: 88006c7cfd30
 R13:  R14: 0010 R15: 88006c7cfc50
 FS:  7fce190fa700() GS:88003ea0() 
knlGS:
 CS:  0010 DS:  ES:  CR0: 80050033
 CR2: 7fffc6b33db8 CR3: 3cf64000 CR4: 06f0
 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:7fce190f9bd8 EFLAGS: 0216 ORIG_RAX: 00fa
 RAX: ffda RBX: 00738020 RCX: 004585c9
 RDX: 2000d000 RSI: 2ff4 RDI: 0017
 RBP: 0046 R08: 20008000 R09: 
 R10:  R11: 0216 R12: 7fff6e610cde
 R13: 7fff6e610cdf R14: 7fce190fa700 R15: 
 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: 
88006c7cfb08
 RIP: sg_init_one+0x1b3/0x240 lib/scatterlist.c:156 RSP: 88006c7cfb08

Fixes: 802c7f1c84e4 ("crypto: dh - Add DH software implementation")
Cc:  # v4.8+
Signed-off-by: Eric Biggers 


Reviewed-by: Tudor Ambarus 


Re: [PATCH 2/4] crypto: dh - don't permit 'p' to be 0

2017-11-02 Thread Eric Biggers
On Thu, Nov 02, 2017 at 01:40:51PM +0200, Tudor Ambarus wrote:
> Hi, Eric,
> 
> On 11/02/2017 12:25 AM, Eric Biggers wrote:
> >If 'p' is 0 for the software Diffie-Hellman implementation, then
> >dh_max_size() returns 0.
> 
> dh_set_secret() returns -EINVAL if p_len < 1536, see
> dh_check_params_length(). What am I missing?
> 
> Cheers,
> ta

You pass a buffer containing 0's, not a buffer of length 0.  The buffer is
interpreted as an arbitrary precision integer, so any length buffer filled with
0's has the mathematical value 0.

Eric


Re: [PATCH 2/4] crypto: dh - don't permit 'p' to be 0

2017-11-02 Thread Tudor Ambarus

Hi, Eric,

On 11/02/2017 12:25 AM, Eric Biggers wrote:

If 'p' is 0 for the software Diffie-Hellman implementation, then
dh_max_size() returns 0.


dh_set_secret() returns -EINVAL if p_len < 1536, see
dh_check_params_length(). What am I missing?

Cheers,
ta