crypto: use-after-free in skcipher_sock_destruct

[Dmitry Vyukov]

Hello,

The following program triggers use-after-free in skcipher_sock_destruct.
This is on upstream commit 03891f9c853d5c4473224478a1e03ea00d70ff8d +
all pending patches from
git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6.git +
4 latest Herbert patches.

// autogenerated by syzkaller (http://github.com/google/syzkaller)
#include <unistd.h>
#include <sys/syscall.h>
#include <string.h>
#include <stdint.h>
#include <pthread.h>

long r[53];

void *thr(void *arg)
{
switch ((long)arg) {
case 0:
r[0] = syscall(SYS_mmap, 0x20000000ul, 0x35000ul,
0x3ul, 0x32ul, 0xfffffffffffffffful, 0x0ul);
break;
case 1:
r[1] = syscall(SYS_socket, 0x26ul, 0x5ul, 0x0ul, 0, 0, 0);
break;
case 2:
r[2] = syscall(SYS_mmap, 0x20035000ul, 0x1000ul,
0x3ul, 0x32ul, 0xfffffffffffffffful, 0x0ul);
break;
case 3:
r[3] = syscall(SYS_mmap, 0x20035000ul, 0x1000ul,
0x3ul, 0x32ul, 0xfffffffffffffffful, 0x0ul);
break;
case 4:
r[4] = syscall(SYS_mmap, 0x20035000ul, 0x1000ul,
0x3ul, 0x32ul, 0xfffffffffffffffful, 0x0ul);
break;
case 5:
*(uint32_t*)0x2003521a = (uint32_t)0x2;
*(uint64_t*)0x20035000 = (uint64_t)0x77359400;
*(uint64_t*)0x20035008 = (uint64_t)0x0;
*(uint32_t*)0x20035000 = (uint32_t)0x6;
r[9] = syscall(SYS_futex, 0x2003521aul, 0x9ul, 0x1ul,
0x20035000ul, 0x20035000ul, 0x8ul);
break;
case 6:
*(uint16_t*)0x20032fa8 = (uint16_t)0x26;
memcpy((void*)0x20032faa,
"\x73\x6b\x63\x69\x70\x68\x65\x72\x00\x00\x00\x00\x00\x00", 14);
*(uint32_t*)0x20032fb8 = (uint32_t)0x8;
*(uint32_t*)0x20032fbc = (uint32_t)0x6;
memcpy((void*)0x20032fc0,
"\x73\x61\x6c\x73\x61\x32\x30\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00",
64);
r[15] = syscall(SYS_bind, r[1], 0x20032fa8ul, 0x58ul, 0, 0, 0);
break;
case 7:
memcpy((void*)0x2002a000,
"\xdc\x4f\x77\x67\x54\xf2\x63\xd1\xbe\x5c\x6b\xac\xa6\x65\xeb\xc1\x0f\x2f\xbd\xea\x88\x2e",
22);
r[17] = syscall(SYS_setsockopt, r[1], 0x117ul, 0x1ul,
0x2002a000ul, 0x16ul, 0);
break;
case 8:
r[18] = syscall(SYS_accept4, r[1], 0x0ul,
0x2001f000ul, 0x80800ul, 0, 0);
break;
case 9:
memcpy((void*)0x20024000,
"\x50\xf0\x53\x1d\x72\x27\x23\x70\x47\x93\x30\xee\xa4\xef\xee\x21\xa5\xce\x3d\xed\x43\xcc\x9d\xfe\x1b\x50\x16\x5c\x76\x74\xd2\xee\x56\x2a\x07\xf0\x40\x6d\xa6\xbc\x23\x47\xe2\x7d\xb4\x89\x2b\x0c\xf6\x97\x8c\x00\xbb\x36\x5e\x67\xc2\x85\xf2\xc7\x41\x51\x7a\x51\x43\x9e\xe9\xf6\x27\xf4\x00\xc8\xda\x36\x87\x22\x1b\xa8\xeb\xfd\x6a\x46\x23\xf5\x8d\x51\x2b\xe0\xf8\x8d\x3a\x50\xd0\x4a\x05\x03\x14\x15\xd5\x20\xf4\xcb\xbe\xe5\x77\x37\x09",
107);
memcpy((void*)0x20033732,
"\x10\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00",
128);
r[21] = syscall(SYS_sendto, r[18], 0x20024000ul,
0x6bul, 0x8800ul, 0x20033732ul, 0x80ul);
break;
case 10:
*(uint64_t*)0x200349fb = (uint64_t)0x20034111;
*(uint32_t*)0x20034a03 = (uint32_t)0x80;
*(uint64_t*)0x20034a0b = (uint64_t)0x20034fb0;
*(uint64_t*)0x20034a13 = (uint64_t)0x5;
*(uint64_t*)0x20034a1b = (uint64_t)0x20034fdd;
*(uint64_t*)0x20034a23 = (uint64_t)0x23;
*(uint32_t*)0x20034a2b = (uint32_t)0x6;
*(uint64_t*)0x20034fb0 = (uint64_t)0x20034f56;
*(uint64_t*)0x20034fb8 = (uint64_t)0xaa;
*(uint64_t*)0x20034fc0 = (uint64_t)0x20034fd0;
*(uint64_t*)0x20034fc8 = (uint64_t)0xe9;
*(uint64_t*)0x20034fd0 = (uint64_t)0x2003450a;
*(uint64_t*)0x20034fd8 = (uint64_t)0xb7;
*(uint64_t*)0x20034fe0 = (uint64_t)0x20034f88;
*(uint64_t*)0x20034fe8 = (uint64_t)0xca;
*(uint64_t*)0x20034ff0 = (uint64_t)0x20034f6b;
*(uint64_t*)0x20034ff8 = (uint64_t)0x95;
r[39] = syscall(SYS_recvmsg, r[18], 0x200349fbul,
0x2060ul, 0, 0, 0);
break;
case 11:
r[40] = syscall(SYS_dup, r[1], 0, 0, 0, 0, 0);
break;
case 12:
r[41] = syscall(SYS_mmap, 0x20035000ul, 0x1000ul,
0x3ul, 0x32ul, 0xfffffffffffffffful, 0x0ul);
break;
case 13:
*(uint16_t*)0x20035000 = (uint16_t)0x1f;
*(uint8_t*)0x20035002 = (uint8_t)0x169;
*(uint8_t*)0x20035003 = (uint8_t)0xfffffffffffffffe;
*(uint8_t*)0x20035004 = (uint8_t)0x5;
*(uint8_t*)0x20035005 = (uint8_t)0x0;
*(uint8_t*)0x20035006 = (uint8_t)0x50df1ce5;
*(uint8_t*)0x20035007 = (uint8_t)0x57;
r[49] = syscall(SYS_bind, r[40], 0x20035000ul, 0x8ul, 0, 0, 0);
break;
case 14:
r[50] = syscall(SYS_mmap, 0x20035000ul, 0x1000ul,
0x3ul, 0x32ul, 0xfffffffffffffffful, 0x0ul);
break;
case 15:
r[51] = syscall(SYS_mmap, 0x20035000ul, 0x1000ul,
0x3ul, 0x32ul, 0xfffffffffffffffful, 0x0ul);
break;
case 16:
r[52] = syscall(SYS_getpeername, r[40], 0x20035000ul,
0x20035adbul, 0, 0, 0);
break;
}
return 0;
}

int main()
{
long i;
pthread_t th[17];

srand(getpid());
memset(r, -1, sizeof(r));
for (i = 0; i < 17; i++) {
pthread_create(&th[i], 0, thr, (void*)i);
usleep(1000);
}
for (i = 0; i < 17; i++) {
pthread_create(&th[i], 0, thr, (void*)i);
if (rand()%2)
usleep(rand()%1000);
}
usleep(100000);
return 0;
}

==================================================================
BUG: KASAN: use-after-free in skcipher_sock_destruct+0x2f6/0x310 at
addr ffff880061db0270
Read of size 4 by task syz-executor/30602
=============================================================================
BUG kmalloc-128 (Not tainted): kasan: bad access detected
-----------------------------------------------------------------------------

INFO: Allocated in crypto_create_tfm+0x7e/0x240 age=38 cpu=2 pid=30612
[< none >] ___slab_alloc+0x486/0x4e0 mm/slub.c:2468
[< none >] __slab_alloc+0x66/0xc0 mm/slub.c:2497
[< inline >] slab_alloc_node mm/slub.c:2560
[< inline >] slab_alloc mm/slub.c:2602
[< none >] __kmalloc+0x2b9/0x350 mm/slub.c:3562
[< inline >] kmalloc include/linux/slab.h:463
[< inline >] kzalloc include/linux/slab.h:602
[< none >] crypto_create_tfm+0x7e/0x240 crypto/api.c:470
[< none >] crypto_alloc_tfm+0x13c/0x370 crypto/api.c:555
[< none >] crypto_alloc_skcipher+0x2c/0x40 crypto/skcipher.c:242
[< none >] skcipher_bind+0x6c/0xe0 crypto/algif_skcipher.c:858
[< none >] alg_bind+0x18e/0x430 crypto/af_alg.c:178
[< none >] SYSC_bind+0x1ea/0x250 net/socket.c:1377
[< none >] SyS_bind+0x24/0x30 net/socket.c:1363
[< none >] entry_SYSCALL_64_fastpath+0x16/0x7a
arch/x86/entry/entry_64.S:185

INFO: Freed in kzfree+0x28/0x30 age=16 cpu=0 pid=30602
[< none >] __slab_free+0x1fc/0x320 mm/slub.c:2678
[< inline >] slab_free mm/slub.c:2833
[< none >] kfree+0x2a8/0x2d0 mm/slub.c:3662
[< none >] kzfree+0x28/0x30 mm/slab_common.c:1270
[< none >] crypto_destroy_tfm+0xcb/0x190 crypto/api.c:596
[< inline >] crypto_free_skcipher include/crypto/skcipher.h:215
[< none >] skcipher_release+0x30/0x50 crypto/algif_skcipher.c:873
[< inline >] alg_do_release crypto/af_alg.c:118
[< none >] alg_sock_destruct+0x8c/0xe0 crypto/af_alg.c:358
[< none >] sk_destruct+0x4a/0x490 net/core/sock.c:1447
[< none >] __sk_free+0x57/0x200 net/core/sock.c:1475
[< none >] sk_free+0x30/0x40 net/core/sock.c:1486
[< inline >] sock_put include/net/sock.h:1627
[< none >] af_alg_release+0x5b/0x70 crypto/af_alg.c:125
[< none >] sock_release+0x8d/0x1d0 net/socket.c:572
[< none >] sock_close+0x16/0x20 net/socket.c:1023
[< none >] __fput+0x236/0x780 fs/file_table.c:208
[< none >] ____fput+0x15/0x20 fs/file_table.c:244
[< none >] task_work_run+0x16b/0x200 kernel/task_work.c:115
[< inline >] exit_task_work include/linux/task_work.h:21
[< none >] do_exit+0x8be/0x2b80 kernel/exit.c:750

INFO: Slab 0xffffea0001876c00 objects=27 used=26 fp=0xffff880061db20d0
flags=0x5fffc0000004080
INFO: Object 0xffff880061db0258 @offset=600 fp=0xffffffff82746af0
CPU: 0 PID: 30602 Comm: syz-executor Tainted: G B 4.4.0+ #238
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
00000000ffffffff ffff8800341df7b0 ffffffff8290ea2d ffff88003e807300
ffff880061db0258 ffff880061db0000 ffff8800341df7e0 ffffffff81730904
ffff88003e807300 ffffea0001876c00 ffff880061db0258 0000000000000000
Call Trace:
[<ffffffff81739e1e>] __asan_report_load4_noabort+0x3e/0x40
mm/kasan/report.c:294
[< inline >] crypto_skcipher_ivsize include/crypto/skcipher.h:246
[<ffffffff828511b6>] skcipher_sock_destruct+0x2f6/0x310
crypto/algif_skcipher.c:908
[<ffffffff84d652aa>] sk_destruct+0x4a/0x490 net/core/sock.c:1447
[<ffffffff84d65747>] __sk_free+0x57/0x200 net/core/sock.c:1475
[<ffffffff84d65920>] sk_free+0x30/0x40 net/core/sock.c:1486
[< inline >] sock_put include/net/sock.h:1627
[<ffffffff8284cddb>] af_alg_release+0x5b/0x70 crypto/af_alg.c:125
[<ffffffff84d4f25d>] sock_release+0x8d/0x1d0 net/socket.c:572
[<ffffffff84d4f3b6>] sock_close+0x16/0x20 net/socket.c:1023
[<ffffffff81780cf6>] __fput+0x236/0x780 fs/file_table.c:208
[<ffffffff817812c5>] ____fput+0x15/0x20 fs/file_table.c:244
[<ffffffff8139fdab>] task_work_run+0x16b/0x200 kernel/task_work.c:115
[< inline >] exit_task_work include/linux/task_work.h:21
[<ffffffff8134bb6e>] do_exit+0x8be/0x2b80 kernel/exit.c:750
[<ffffffff8134dfa8>] do_group_exit+0x108/0x330 kernel/exit.c:880
[<ffffffff81371214>] get_signal+0x5e4/0x1500 kernel/signal.c:2307
[<ffffffff81193db3>] do_signal+0x83/0x1c90 arch/x86/kernel/signal.c:712
[<ffffffff81006685>] exit_to_usermode_loop+0x1a5/0x210
arch/x86/entry/common.c:247
[< inline >] prepare_exit_to_usermode arch/x86/entry/common.c:282
[<ffffffff8100851a>] syscall_return_slowpath+0x2ba/0x340
arch/x86/entry/common.c:344
[<ffffffff85e8ece2>] int_ret_from_sys_call+0x25/0x9f
arch/x86/entry/entry_64.S:281
==================================================================

[Herbert Xu]

On Wed, Jan 13, 2016 at 12:58:34PM +0100, Dmitry Vyukov wrote:
>
> The following program triggers use-after-free in skcipher_sock_destruct.
> This is on upstream commit 03891f9c853d5c4473224478a1e03ea00d70ff8d +
> all pending patches from
> git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6.git +
> 4 latest Herbert patches.

OK, the check_key function is buggy in that it doesn't lock the
child socket so if you make two syscalls on the child socket at
the same time you can end up freeing the parent socket.

Please try these two patches.

Thanks,
--
Email: Herbert Xu <her...@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[Herbert Xu]

We need to lock the child socket in hash_check_key as otherwise
two simultaneous calls can cause the parent socket to be freed.

Signed-off-by: Herbert Xu <her...@gondor.apana.org.au>

diff --git a/crypto/algif_hash.c b/crypto/algif_hash.c
index 46637be..598f141 100644
--- a/crypto/algif_hash.c
+++ b/crypto/algif_hash.c
@@ -242,15 +242,16 @@ static struct proto_ops algif_hash_ops = {

static int hash_check_key(struct socket *sock)
{
- int err;
+ int err = 0;
struct sock *psk;
struct alg_sock *pask;
struct algif_hash_tfm *tfm;
struct sock *sk = sock->sk;
struct alg_sock *ask = alg_sk(sk);

+ lock_sock(sk);
if (ask->refcnt)
- return 0;
+ goto unlock_child;

psk = ask->parent;
pask = alg_sk(ask->parent);
@@ -271,6 +272,8 @@ static int hash_check_key(struct socket *sock)

unlock:
release_sock(psk);
+unlock_child:
+ release_sock(sk);

return err;

[Herbert Xu]

We need to lock the child socket in skcipher_check_key as otherwise

two simultaneous calls can cause the parent socket to be freed.

Signed-off-by: Herbert Xu <her...@gondor.apana.org.au>

diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index 39df2a9..5772c0b 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -755,15 +755,16 @@ static struct proto_ops algif_skcipher_ops = {

static int skcipher_check_key(struct socket *sock)

{
- int err;
+ int err = 0;
struct sock *psk;
struct alg_sock *pask;

struct skcipher_tfm *tfm;

struct sock *sk = sock->sk;
struct alg_sock *ask = alg_sk(sk);

+ lock_sock(sk);
if (ask->refcnt)
- return 0;
+ goto unlock_child;

psk = ask->parent;
pask = alg_sk(ask->parent);

@@ -784,6 +785,8 @@ static int skcipher_check_key(struct socket *sock)

[Dmitry Vyukov]

On Thu, Jan 14, 2016 at 3:13 PM, Herbert Xu <her...@gondor.apana.org.au> wrote:
> On Wed, Jan 13, 2016 at 12:58:34PM +0100, Dmitry Vyukov wrote:
>>
>> The following program triggers use-after-free in skcipher_sock_destruct.
>> This is on upstream commit 03891f9c853d5c4473224478a1e03ea00d70ff8d +
>> all pending patches from
>> git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6.git +
>> 4 latest Herbert patches.
>
> OK, the check_key function is buggy in that it doesn't lock the
> child socket so if you make two syscalls on the child socket at
> the same time you can end up freeing the parent socket.
>
> Please try these two patches.

With these patches I see lots of:

[ INFO: possible recursive locking detected ]
4.4.0+ #250 Not tainted
---------------------------------------------
syz-executor/16742 is trying to acquire lock:
(sk_lock-AF_ALG){+.+.+.}, at: [< inline >] lock_sock
include/net/sock.h:1480
(sk_lock-AF_ALG){+.+.+.}, at: [<ffffffff828661d2>]
hash_check_key.isra.3+0xd2/0x210 crypto/algif_hash.c:261

but task is already holding lock:
(sk_lock-AF_ALG){+.+.+.}, at: [< inline >] lock_sock
include/net/sock.h:1480
(sk_lock-AF_ALG){+.+.+.}, at: [<ffffffff82866126>]
hash_check_key.isra.3+0x26/0x210 crypto/algif_hash.c:252

other info that might help us debug this:
Possible unsafe locking scenario:

CPU0
----
lock(sk_lock-AF_ALG);
lock(sk_lock-AF_ALG);

*** DEADLOCK ***

May be due to missing lock nesting notation

1 lock held by syz-executor/16742:
#0: (sk_lock-AF_ALG){+.+.+.}, at: [< inline >] lock_sock
include/net/sock.h:1480
#0: (sk_lock-AF_ALG){+.+.+.}, at: [<ffffffff82866126>]
hash_check_key.isra.3+0x26/0x210 crypto/algif_hash.c:252

stack backtrace:
CPU: 0 PID: 16742 Comm: syz-executor Not tainted 4.4.0+ #250

Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011

00000000ffffffff ffff880035e277b0 ffffffff82925f5d 0000000000000000
ffffffff88ec2570 ffffffff88ec2570 ffff880035e27938 ffffffff81454890
ffff880000008900 fffffbfff128d2c0 ffff880035e27890 ffffed0006959405
Call Trace:
[< inline >] __dump_stack lib/dump_stack.c:15
[<ffffffff82925f5d>] dump_stack+0x6f/0xa2 lib/dump_stack.c:50
[< inline >] print_deadlock_bug kernel/locking/lockdep.c:1752
[< inline >] check_deadlock kernel/locking/lockdep.c:1796
[< inline >] validate_chain kernel/locking/lockdep.c:2128
[<ffffffff81454890>] __lock_acquire+0x17e0/0x4700 kernel/locking/lockdep.c:3206
[<ffffffff81459bfc>] lock_acquire+0x1dc/0x430 kernel/locking/lockdep.c:3585
[<ffffffff8510caab>] lock_sock_nested+0xcb/0x120 net/core/sock.c:2462
[< inline >] lock_sock include/net/sock.h:1480
[<ffffffff828661d2>] hash_check_key.isra.3+0xd2/0x210 crypto/algif_hash.c:261
[<ffffffff8286646f>] hash_sendmsg_nokey+0x3f/0x80 crypto/algif_hash.c:286
[< inline >] sock_sendmsg_nosec net/socket.c:611
[<ffffffff8510415a>] sock_sendmsg+0xca/0x110 net/socket.c:621
[<ffffffff85105b79>] ___sys_sendmsg+0x309/0x840 net/socket.c:1947
[<ffffffff85108194>] __sys_sendmmsg+0x134/0x350 net/socket.c:2032
[< inline >] SYSC_sendmmsg net/socket.c:2061
[<ffffffff851083e5>] SyS_sendmmsg+0x35/0x60 net/socket.c:2056
[<ffffffff8626c3f6>] entry_SYSCALL_64_fastpath+0x16/0x7a
arch/x86/entry/entry_64.S:185

[Herbert Xu]

On Fri, Jan 15, 2016 at 10:06:11AM +0100, Dmitry Vyukov wrote:
>
> With these patches I see lots of:
>
> [ INFO: possible recursive locking detected ]
> 4.4.0+ #250 Not tainted
> ---------------------------------------------
> syz-executor/16742 is trying to acquire lock:
> (sk_lock-AF_ALG){+.+.+.}, at: [< inline >] lock_sock
> include/net/sock.h:1480
> (sk_lock-AF_ALG){+.+.+.}, at: [<ffffffff828661d2>]
> hash_check_key.isra.3+0xd2/0x210 crypto/algif_hash.c:261
>
> but task is already holding lock:
> (sk_lock-AF_ALG){+.+.+.}, at: [< inline >] lock_sock
> include/net/sock.h:1480
> (sk_lock-AF_ALG){+.+.+.}, at: [<ffffffff82866126>]
> hash_check_key.isra.3+0x26/0x210 crypto/algif_hash.c:252
>
> other info that might help us debug this:
> Possible unsafe locking scenario:
>
> CPU0
> ----
> lock(sk_lock-AF_ALG);
> lock(sk_lock-AF_ALG);
>
> *** DEADLOCK ***
>
> May be due to missing lock nesting notation

Indeed. Here is an updated version.

[Herbert Xu]

We need to lock the child socket in hash_check_key as otherwise

two simultaneous calls can cause the parent socket to be freed.

Signed-off-by: Herbert Xu <her...@gondor.apana.org.au>

diff --git a/crypto/algif_hash.c b/crypto/algif_hash.c
index 46637be..0557e62 100644
--- a/crypto/algif_hash.c
+++ b/crypto/algif_hash.c
@@ -242,22 +242,23 @@ static struct proto_ops algif_hash_ops = {

static int hash_check_key(struct socket *sock)

{
- int err;
+ int err = 0;
struct sock *psk;
struct alg_sock *pask;

struct algif_hash_tfm *tfm;

struct sock *sk = sock->sk;
struct alg_sock *ask = alg_sk(sk);

+ lock_sock(sk);
if (ask->refcnt)
- return 0;
+ goto unlock_child;

psk = ask->parent;
pask = alg_sk(ask->parent);

tfm = pask->private;

err = -ENOKEY;
- lock_sock(psk);
+ lock_sock_nested(psk, SINGLE_DEPTH_NESTING);
if (!tfm->has_key)
goto unlock;

@@ -271,6 +272,8 @@ static int hash_check_key(struct socket *sock)

unlock:
release_sock(psk);
+unlock_child:
+ release_sock(sk);

return err;
}

[Herbert Xu]

We need to lock the child socket in skcipher_check_key as otherwise

two simultaneous calls can cause the parent socket to be freed.

Signed-off-by: Herbert Xu <her...@gondor.apana.org.au>

diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c
index 39df2a9..dc7d91b 100644
--- a/crypto/algif_skcipher.c
+++ b/crypto/algif_skcipher.c
@@ -755,22 +755,23 @@ static struct proto_ops algif_skcipher_ops = {

static int skcipher_check_key(struct socket *sock)

{
- int err;
+ int err = 0;
struct sock *psk;
struct alg_sock *pask;

struct skcipher_tfm *tfm;

struct sock *sk = sock->sk;
struct alg_sock *ask = alg_sk(sk);

+ lock_sock(sk);
if (ask->refcnt)
- return 0;
+ goto unlock_child;

psk = ask->parent;
pask = alg_sk(ask->parent);
tfm = pask->private;

err = -ENOKEY;
- lock_sock(psk);
+ lock_sock_nested(psk, SINGLE_DEPTH_NESTING);
if (!tfm->has_key)
goto unlock;

@@ -784,6 +785,8 @@ static int skcipher_check_key(struct socket *sock)

unlock:
release_sock(psk);
+unlock_child:
+ release_sock(sk);

return err;
}

[Dmitry Vyukov]

With these patches the original bug is fixed and don't see any new.

Tested-by: Dmitry Vyukov <dvy...@google.com>