[Devel,RHEL7,COMMIT] ms/crypto: user - lock crypto_alg_list on alg dump

Submitted by Konstantin Khorenko on Oct. 24, 2016, 12:34 p.m.

Details

Message ID 201610241234.u9OCY6JQ024479@finist_cl7.x64_64.work.ct
State New
Series "ms/crypto: user - lock crypto_alg_list on alg dump"
Headers show

Commit Message

Konstantin Khorenko Oct. 24, 2016, 12:34 p.m.
The commit is pushed to "branch-rh7-3.10.0-327.36.1.vz7.19.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
after rh7-3.10.0-327.36.1.vz7.19.3
------>
commit a68e55a7e300d6cc3213d226718d6332334a227d
Author: Mathias Krause <minipli@googlemail.com>
Date:   Mon Oct 24 16:34:06 2016 +0400

    ms/crypto: user - lock crypto_alg_list on alg dump
    
    We miss to take the crypto_alg_sem semaphore when traversing the
    crypto_alg_list for CRYPTO_MSG_GETALG dumps. This allows a race with
    crypto_unregister_alg() removing algorithms from the list while we're
    still traversing it, thereby leading to a use-after-free as show below:
    
    [ 3482.071639] general protection fault: 0000 [#1] SMP
    [ 3482.075639] Modules linked in: aes_x86_64 glue_helper lrw ablk_helper cryptd gf128mul ipv6 pcspkr serio_raw virtio_net microcode virtio_pci virtio_ring virtio sr_mod cdrom [last unloaded: aesni_intel]
    [ 3482.075639] CPU: 1 PID: 11065 Comm: crconf Not tainted 4.3.4-grsec+ #126
    [ 3482.075639] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.7.5-20140531_083030-gandalf 04/01/2014
    [ 3482.075639] task: ffff88001cd41a40 ti: ffff88001cd422c8 task.ti: ffff88001cd422c8
    [ 3482.075639] RIP: 0010:[<ffffffff93722bd3>]  [<ffffffff93722bd3>] strncpy+0x13/0x30
    [ 3482.075639] RSP: 0018:ffff88001f713b60  EFLAGS: 00010202
    [ 3482.075639] RAX: ffff88001f6c4430 RBX: ffff88001f6c43a0 RCX: ffff88001f6c4430
    [ 3482.075639] RDX: 0000000000000040 RSI: fefefefefefeff16 RDI: ffff88001f6c4430
    [ 3482.075639] RBP: ffff88001f713b60 R08: ffff88001f6c4470 R09: ffff88001f6c4480
    [ 3482.075639] R10: 0000000000000002 R11: 0000000000000246 R12: ffff88001ce2aa28
    [ 3482.075639] R13: ffff880000093700 R14: ffff88001f5e4bf8 R15: 0000000000003b20
    [ 3482.075639] FS:  0000033826fa2700(0000) GS:ffff88001e900000(0000) knlGS:0000000000000000
    [ 3482.075639] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
    [ 3482.075639] CR2: ffffffffff600400 CR3: 00000000139ec000 CR4: 00000000001606f0
    [ 3482.075639] Stack:
    [ 3482.075639]  ffff88001f713bd8 ffffffff936ccd00 ffff88001e5c4200 ffff880000093700
    [ 3482.075639]  ffff88001f713bd0 ffffffff938ef4bf 0000000000000000 0000000000003b20
    [ 3482.075639]  ffff88001f5e4bf8 ffff88001f5e4848 0000000000000000 0000000000003b20
    [ 3482.075639] Call Trace:
    [ 3482.075639]  [<ffffffff936ccd00>] crypto_report_alg+0xc0/0x3e0
    [ 3482.075639]  [<ffffffff938ef4bf>] ? __alloc_skb+0x16f/0x300
    [ 3482.075639]  [<ffffffff936cd08a>] crypto_dump_report+0x6a/0x90
    [ 3482.075639]  [<ffffffff93935707>] netlink_dump+0x147/0x2e0
    [ 3482.075639]  [<ffffffff93935f99>] __netlink_dump_start+0x159/0x190
    [ 3482.075639]  [<ffffffff936ccb13>] crypto_user_rcv_msg+0xc3/0x130
    [ 3482.075639]  [<ffffffff936cd020>] ? crypto_report_alg+0x3e0/0x3e0
    [ 3482.075639]  [<ffffffff936cc4b0>] ? alg_test_crc32c+0x120/0x120
    [ 3482.075639]  [<ffffffff93933145>] ? __netlink_lookup+0xd5/0x120
    [ 3482.075639]  [<ffffffff936cca50>] ? crypto_add_alg+0x1d0/0x1d0
    [ 3482.075639]  [<ffffffff93938141>] netlink_rcv_skb+0xe1/0x130
    [ 3482.075639]  [<ffffffff936cc4f8>] crypto_netlink_rcv+0x28/0x40
    [ 3482.075639]  [<ffffffff939375a8>] netlink_unicast+0x108/0x180
    [ 3482.075639]  [<ffffffff93937c21>] netlink_sendmsg+0x541/0x770
    [ 3482.075639]  [<ffffffff938e31e1>] sock_sendmsg+0x21/0x40
    [ 3482.075639]  [<ffffffff938e4763>] SyS_sendto+0xf3/0x130
    [ 3482.075639]  [<ffffffff93444203>] ? bad_area_nosemaphore+0x13/0x20
    [ 3482.075639]  [<ffffffff93444470>] ? __do_page_fault+0x80/0x3a0
    [ 3482.075639]  [<ffffffff939d80cb>] entry_SYSCALL_64_fastpath+0x12/0x6e
    [ 3482.075639] Code: 88 4a ff 75 ed 5d 48 0f ba 2c 24 3f c3 66 66 2e 0f 1f 84 00 00 00 00 00 55 48 85 d2 48 89 f8 48 89 f9 4c 8d 04 17 48 89 e5 74 15 <0f> b6 16 80 fa 01 88 11 48 83 de ff 48 83 c1 01 4c 39 c1 75 eb
    [ 3482.075639] RIP  [<ffffffff93722bd3>] strncpy+0x13/0x30
    
    To trigger the race run the following loops simultaneously for a while:
      $ while : ; do modprobe aesni-intel; rmmod aesni-intel; done
      $ while : ; do crconf show all > /dev/null; done
    
    Fix the race by taking the crypto_alg_sem read lock, thereby preventing
    crypto_unregister_alg() from modifying the algorithm list during the
    dump.
    
    This bug has been detected by the PaX memory sanitize feature.
    
    Cc: stable@vger.kernel.org
    Signed-off-by: Mathias Krause <minipli@googlemail.com>
    Cc: Steffen Klassert <steffen.klassert@secunet.com>
    Cc: PaX Team <pageexec@freemail.hu>
    Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
    (cherry picked from commit 63e41ebc6630f39422d87f8a4bade1e793f37a01)
    Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
---
 crypto/crypto_user.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/crypto/crypto_user.c b/crypto/crypto_user.c
index 43665d0..619023e 100644
--- a/crypto/crypto_user.c
+++ b/crypto/crypto_user.c
@@ -477,6 +477,7 @@  static int crypto_user_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 		if (link->dump == NULL)
 			return -EINVAL;
 
+		down_read(&crypto_alg_sem);
 		list_for_each_entry(alg, &crypto_alg_list, cra_list)
 			dump_alloc += CRYPTO_REPORT_MAXSIZE;
 
@@ -486,8 +487,11 @@  static int crypto_user_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 				.done = link->done,
 				.min_dump_alloc = dump_alloc,
 			};
-			return netlink_dump_start(crypto_nlsk, skb, nlh, &c);
+			err = netlink_dump_start(crypto_nlsk, skb, nlh, &c);
 		}
+		up_read(&crypto_alg_sem);
+
+		return err;
 	}
 
 	err = nlmsg_parse(nlh, crypto_msg_min[type], attrs, CRYPTOCFGA_MAX,