Re: [PATCH v2] security/keys: rewrite all of big_key crypto

From: Eric Biggers
Date: Wed Jun 07 2017 - 00:22:32 EST


Hi Jason,

On Tue, Jun 06, 2017 at 11:51:29PM +0200, Jason A. Donenfeld wrote:
> issue now. And, some error paths forgot to zero out sensitive material, so
> this patch changes a kfree into a kzfree.

There are other places in big_key.c that should be doing kzfree() instead of
kfree(). Sorry, I actually recently did a patchset that fixed this for major
parts of the keyrings API, but I skipped the big_key type because it was one of
the more obscure key types (and frankly I have no idea what, if anything,
actually uses it). Probably the switch to kzfree() should be its own patch
since it's a separate logical change.

> {
> int ret = -EINVAL;
> struct scatterlist sgio;
> - SKCIPHER_REQUEST_ON_STACK(req, big_key_skcipher);
> -
> - if (crypto_skcipher_setkey(big_key_skcipher, key, ENC_KEY_SIZE)) {
> + u8 req_on_stack[sizeof(struct aead_request) +
> + crypto_aead_reqsize(big_key_aead)];
> + struct aead_request *aead_req = (struct aead_request *)req_on_stack;
> +

The crypto API with CONFIG_VMAP_STACK=y and CONFIG_DEBUG_SG=y is unhappy with
using an aead_request on the stack, because it can't create a scatterlist from
it. It will need to be on the heap instead. (Or else the crypto API fixed.)

# cat .vimrc | keyctl padd big_key fred @s
[ 43.687347] ------------[ cut here ]------------
[ 43.692592] kernel BUG at ./include/linux/scatterlist.h:140!
[ 43.697527] invalid opcode: 0000 [#1] SMP
[ 43.700843] CPU: 0 PID: 219 Comm: keyctl Not tainted 4.12.0-rc4-next-20170606-xfstests-00003-gc6e36366c198 #120
[ 43.707361] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-20170228_101828-anatol 04/01/2014
[ 43.712167] task: ffff9ff6f97d8340 task.stack: ffffbd460049c000
[ 43.714259] RIP: 0010:crypto_gcm_init_common+0x234/0x260
[ 43.715786] RSP: 0018:ffffbd460049fac8 EFLAGS: 00010246
[ 43.717031] RAX: 0000000000000000 RBX: ffffbd460049fba8 RCX: 0000000000000028
[ 43.718688] RDX: 00001d4f4049fbb8 RSI: 000000000000001d RDI: ffffbd468049fbb8
[ 43.720245] RBP: ffffbd460049fb00 R08: ffffbd460049fbd8 R09: ffffbd460049fbd8
[ 43.721473] R10: 0000000000000000 R11: 0000000000000000 R12: ffffbd460049fbb8
[ 43.722704] R13: 000000000000092f R14: ffffbd460049fb58 R15: ffffbd460049fba8
[ 43.723927] FS: 00007f04df2f8700(0000) GS:ffff9ff6fe200000(0000) knlGS:0000000000000000
[ 43.725262] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 43.726065] CR2: 00007f04dec1a4c0 CR3: 000000003957e000 CR4: 00000000003406f0
[ 43.727059] Call Trace:
[ 43.727414] crypto_gcm_encrypt+0x37/0xe0
[ 43.727896] big_key_crypt+0x106/0x160
[ 43.728324] big_key_preparse+0xc0/0x1d0
[ 43.728784] ? down_read+0x68/0xa0
[ 43.729183] key_create_or_update+0x13f/0x440
[ 43.729688] SyS_add_key+0xa1/0x1d0
[ 43.730260] entry_SYSCALL_64_fastpath+0x1f/0xbe
[ 43.730851] RIP: 0033:0x7f04dec22f19
[ 43.731253] RSP: 002b:00007ffd622a0c88 EFLAGS: 00000206 ORIG_RAX: 00000000000000f8
[ 43.732321] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f04dec22f19
[ 43.733274] RDX: 0000000000606260 RSI: 00007ffd622a2866 RDI: 00007ffd622a285e
[ 43.734192] RBP: 0000000000000000 R08: 00000000fffffffd R09: 000000000000001e
[ 43.735026] R10: 000000000000092f R11: 0000000000000206 R12: 0000000000100000
[ 43.735855] R13: 00007ffd622a0ca8 R14: 00007ffd622a0df8 R15: 0000000000404606
[ 43.736690] Code: c8 01 48 89 83 d8 00 00 00 48 83 c4 10 5b 41 5c 41 5d 41 5e 41 5f 5d c3 48 c7 c0 00 00 00 80 48 2b 05 69 36 74 00 e9 43 ff ff ff <0f> 0b 0f 0b 0f 0b 0f 0b 0f 0b 0f 0b 0f 0b 0f 0b 48 c7 45 c8 01
[ 43.738740] RIP: crypto_gcm_init_common+0x234/0x260 RSP: ffffbd460049fac8
[ 43.739402] ---[ end trace 6df3f493a6e39d76 ]---
Segmentation fault