use-after-free in hash_sock_destruct

[Dmitry Vyukov]

Hello,

The following program causes use-after-free in hash_sock_destruct:

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

struct sockaddr_alg {
unsigned short salg_family;
char salg_type[14];
unsigned salg_feat;
unsigned salg_mask;
char salg_name[64];
};

int fd;

void *thr0(void *arg)
{
struct sockaddr_alg sa = {};
sa.salg_family = 0x26;
memcpy(sa.salg_type,
"\x68\x61\x73\x68\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00", 14);
sa.salg_feat = 0xf;
sa.salg_mask = 0x100;
memcpy(sa.salg_name,
"\x6d\x64\x34\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",
64);
bind(fd, (struct sockaddr*)&sa, sizeof(sa));
return 0;
}

void *thr1(void *arg)
{
accept4(fd, 0, 0, 0x800ul);
return 0;
}

int main()
{
fd = socket(PF_ALG, SOCK_SEQPACKET, 0);
pthread_t th[4];
pthread_create(&th[0], 0, thr0, 0);
pthread_create(&th[1], 0, thr1, 0);
pthread_create(&th[2], 0, thr0, 0);
pthread_create(&th[3], 0, thr1, 0);
pthread_join(th[0], 0);
pthread_join(th[1], 0);
pthread_join(th[2], 0);
pthread_join(th[3], 0);
return 0;
}



==================================================================
BUG: KASAN: use-after-free in hash_sock_destruct+0x1de/0x260 at addr
ffff88005f057090
Read of size 8 by task a.out/6844
=============================================================================
BUG kmalloc-192 (Not tainted): kasan: bad access detected
-----------------------------------------------------------------------------

Disabling lock debugging due to kernel taint
INFO: Allocated in crypto_create_tfm+0x87/0x2a0 age=3 cpu=2 pid=6845
[< none >] ___slab_alloc+0x648/0x8c0 mm/slub.c:2438
[< none >] __slab_alloc+0x4c/0x90 mm/slub.c:2467
[< inline >] slab_alloc_node mm/slub.c:2530
[< inline >] slab_alloc mm/slub.c:2572
[< none >] __kmalloc+0x2d9/0x480 mm/slub.c:3532
[< inline >] kmalloc include/linux/slab.h:463
[< inline >] kzalloc include/linux/slab.h:602
[< none >] crypto_create_tfm+0x87/0x2a0 crypto/api.c:470
[< none >] crypto_alloc_tfm+0x138/0x3f0 crypto/api.c:555
[< none >] crypto_alloc_ahash+0x2c/0x40 crypto/ahash.c:538
[< none >] hash_bind+0x25/0x30 crypto/algif_hash.c:240
[< none >] alg_bind+0x1a9/0x410 crypto/af_alg.c:155
[< none >] SYSC_bind+0x20a/0x2c0 net/socket.c:1383
[< none >] SyS_bind+0x24/0x30 net/socket.c:1369
[< none >] entry_SYSCALL_64_fastpath+0x16/0x7a
arch/x86/entry/entry_64.S:185

INFO: Freed in kzfree+0x28/0x30 age=2 cpu=1 pid=6847
[< none >] __slab_free+0x21e/0x3e0 mm/slub.c:2648
[< inline >] slab_free mm/slub.c:2803
[< none >] kfree+0x26f/0x3e0 mm/slub.c:3632
[< none >] kzfree+0x28/0x30 mm/slab_common.c:1270
[< none >] crypto_destroy_tfm+0xdd/0x1e0 crypto/api.c:596
[< inline >] crypto_free_ahash include/crypto/hash.h:258
[< none >] hash_release+0x19/0x20 crypto/algif_hash.c:245
[< inline >] alg_do_release crypto/af_alg.c:116
[< none >] alg_bind+0x26a/0x410 crypto/af_alg.c:170
[< none >] SYSC_bind+0x20a/0x2c0 net/socket.c:1383
[< none >] SyS_bind+0x24/0x30 net/socket.c:1369
[< none >] entry_SYSCALL_64_fastpath+0x16/0x7a
arch/x86/entry/entry_64.S:185

INFO: Slab 0xffffea00017c1580 objects=16 used=15 fp=0xffff88005f057000
flags=0x5fffc0000004080
INFO: Object 0xffff88005f057000 @offset=4096 fp=0x (null)
CPU: 1 PID: 6844 Comm: a.out Tainted: G B 4.4.0-rc3+ #151
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
0000000000000001 ffff8800379779e0 ffffffff82e0f4b8 0000000041b58ab3
ffffffff87a9a265 ffffffff82e0f406 ffff88003d8c0000 ffffffff87abb3aa
ffff88003e804a00 0000000000000008 ffff88005f057000 ffff8800379779e0

Call Trace:
[< inline >] __dump_stack lib/dump_stack.c:15
[<ffffffff82e0f4b8>] dump_stack+0xb2/0xfa lib/dump_stack.c:50
[<ffffffff8183680c>] print_trailer+0x12c/0x210 mm/slub.c:652
[<ffffffff8184162f>] object_err+0x2f/0x40 mm/slub.c:659
[< inline >] print_address_description mm/kasan/report.c:138
[<ffffffff81845399>] kasan_report_error+0x5d9/0x860 mm/kasan/report.c:251
[< inline >] kasan_report mm/kasan/report.c:274
[<ffffffff818457c4>] __asan_report_load8_noabort+0x54/0x70
mm/kasan/report.c:295
[< inline >] crypto_ahash_digestsize include/crypto/hash.h:290
[<ffffffff82d2a7ae>] hash_sock_destruct+0x1de/0x260 crypto/algif_hash.c:259
[<ffffffff856c9255>] sk_destruct+0xa5/0x5f0 net/core/sock.c:1446
[<ffffffff856c982c>] __sk_free+0x8c/0x260 net/core/sock.c:1474
[<ffffffff856c9a30>] sk_free+0x30/0x40 net/core/sock.c:1485
[< inline >] sock_put include/net/sock.h:1625
[<ffffffff82d26c50>] af_alg_release+0x60/0x90 crypto/af_alg.c:123
[<ffffffff856b06b6>] sock_release+0x96/0x260 net/socket.c:571
[<ffffffff856b0896>] sock_close+0x16/0x20 net/socket.c:1022
[<ffffffff8189d9e4>] __fput+0x244/0x860 fs/file_table.c:208
[<ffffffff8189e095>] ____fput+0x15/0x20 fs/file_table.c:244
[<ffffffff813e34d0>] task_work_run+0x130/0x240 kernel/task_work.c:115
[< inline >] exit_task_work include/linux/task_work.h:21
[<ffffffff8137d8f5>] do_exit+0x885/0x3050 kernel/exit.c:750
[<ffffffff8138021c>] do_group_exit+0xec/0x390 kernel/exit.c:880
[< inline >] SYSC_exit_group kernel/exit.c:891
[<ffffffff813804dd>] SyS_exit_group+0x1d/0x20 kernel/exit.c:889
[<ffffffff86a89fb6>] entry_SYSCALL_64_fastpath+0x16/0x7a
arch/x86/entry/entry_64.S:185
==================================================================
BUG: unable to handle kernel paging request at fffffffffffffff8
IP: [<ffffffff82d2a6ae>] hash_sock_destruct+0xde/0x260 crypto/algif_hash.c:258
PGD 7c2f067 PUD 7c31067 PMD 0
Oops: 0000 [#1] SMP KASAN
Modules linked in:
CPU: 1 PID: 6844 Comm: a.out Tainted: G B 4.4.0-rc3+ #151
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
task: ffff88003d8c0000 ti: ffff880037970000 task.ti: ffff880037970000
RIP: 0010:[<ffffffff82d2a6ae>] [<ffffffff82d2a6ae>]
hash_sock_destruct+0xde/0x260
RSP: 0018:ffff880037977b30 EFLAGS: 00010282
RAX: 0000000000000000 RBX: ffff88005f94dc80 RCX: 0000000000000001
RDX: 1fffffffffffffff RSI: 0000000000000001 RDI: 0000000000000282
RBP: ffff880037977b58 R08: 0000000000000001 R09: 0000000000000000
R10: ffffed0006f2ef55 R11: 0000000000000000 R12: ffff88005f94e5c0
R13: 0000000000000000 R14: fffffffffffffff8 R15: ffffffff856b0880
FS: 0000000000000000(0000) GS:ffff88003ed00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
CR2: fffffffffffffff8 CR3: 0000000007c2e000 CR4: 00000000000006e0
Stack:
ffff880000000000 ffff88005f94e5c0 ffffffff82d2a5d0 1ffff10006f2ef6d
0000000000000000 ffff880037977bf0 ffffffff856c9255 0000000041b58ab3
ffffffff87a9a273 ffffffff856c91b0 ffff8800602c8610 ffff88003d8c0000
Call Trace:
[<ffffffff856c9255>] sk_destruct+0xa5/0x5f0 net/core/sock.c:1446
[<ffffffff856c982c>] __sk_free+0x8c/0x260 net/core/sock.c:1474
[<ffffffff856c9a30>] sk_free+0x30/0x40 net/core/sock.c:1485
[< inline >] sock_put include/net/sock.h:1625
[<ffffffff82d26c50>] af_alg_release+0x60/0x90 crypto/af_alg.c:123
[<ffffffff856b06b6>] sock_release+0x96/0x260 net/socket.c:571
[<ffffffff856b0896>] sock_close+0x16/0x20 net/socket.c:1022
[<ffffffff8189d9e4>] __fput+0x244/0x860 fs/file_table.c:208
[<ffffffff8189e095>] ____fput+0x15/0x20 fs/file_table.c:244
[<ffffffff813e34d0>] task_work_run+0x130/0x240 kernel/task_work.c:115
[< inline >] exit_task_work include/linux/task_work.h:21
[<ffffffff8137d8f5>] do_exit+0x885/0x3050 kernel/exit.c:750
[<ffffffff8138021c>] do_group_exit+0xec/0x390 kernel/exit.c:880
[< inline >] SYSC_exit_group kernel/exit.c:891
[<ffffffff813804dd>] SyS_exit_group+0x1d/0x20 kernel/exit.c:889
[<ffffffff86a89fb6>] entry_SYSCALL_64_fastpath+0x16/0x7a
arch/x86/entry/entry_64.S:185
Code: 08 0f 84 7c 01 00 00 4c 89 f2 48 b8 00 00 00 00 00 fc ff df 48
c1 ea 03 0f b6 04 02 84 c0 74 08 3c 03 0f 8e d4 00 00 00 48 85 db <41>
8b 55 f8 0f 84 35 01 00 00 48 8d bb 30 03 00 00 48 b8 00 00
RIP [<ffffffff82d2a6ae>] hash_sock_destruct+0xde/0x260 crypto/algif_hash.c:258
RSP <ffff880037977b30>
CR2: fffffffffffffff8
---[ end trace b367ebac5d601601 ]---
Fixing recursive fault but reboot is needed!


On upstream commit 31ade3b83e1821da5fbb2f11b5b3d4ab2ec39db8 (Nov 29).

[Herbert Xu]

On Thu, Dec 17, 2015 at 01:59:50PM +0100, Dmitry Vyukov wrote:
>
> The following program causes use-after-free in hash_sock_destruct:

This patch should fix the problem. AFAIK everything that you have
reported should now be fixed. If you still have issues please
resubmit them (and please cc me). Thanks!

---8<---
Subject: crypto: af_alg - Disallow bind/setkey/... after accept(2)

Each af_alg parent socket obtained by socket(2) corresponds to a
tfm object once bind(2) has succeeded. An accept(2) call on that
parent socket creates a context which then uses the tfm object.

Therefore as long as any child sockets created by accept(2) exist
the parent socket must not be modified or freed.

This patch guarantees this by using locks and a reference count
on the parent socket. Any attempt to modify the parent socket will
fail with EBUSY.

Cc: sta...@vger.kernel.org
Reported-by: Dmitry Vyukov <dvy...@google.com>
Signed-off-by: Herbert Xu <her...@gondor.apana.org.au>

diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index a8e7aa3..f5a2426 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -125,6 +125,23 @@ int af_alg_release(struct socket *sock)
}
EXPORT_SYMBOL_GPL(af_alg_release);

+void af_alg_release_parent(struct sock *sk)
+{
+ struct alg_sock *ask = alg_sk(sk);
+ bool last;
+
+ sk = ask->parent;
+ ask = alg_sk(sk);
+
+ lock_sock(sk);
+ last = !--ask->refcnt;
+ release_sock(sk);
+
+ if (last)
+ sock_put(sk);
+}
+EXPORT_SYMBOL_GPL(af_alg_release_parent);
+
static int alg_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
{
const u32 forbidden = CRYPTO_ALG_INTERNAL;
@@ -133,6 +150,7 @@ static int alg_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
struct sockaddr_alg *sa = (void *)uaddr;
const struct af_alg_type *type;
void *private;
+ int err;

if (sock->state == SS_CONNECTED)
return -EINVAL;
@@ -160,16 +178,22 @@ static int alg_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
return PTR_ERR(private);
}

+ err = -EBUSY;
lock_sock(sk);
+ if (ask->refcnt)
+ goto unlock;

swap(ask->type, type);
swap(ask->private, private);

+ err = 0;
+
+unlock:
release_sock(sk);

alg_do_release(type, private);

- return 0;
+ return err;
}

static int alg_setkey(struct sock *sk, char __user *ukey,
@@ -188,14 +212,41 @@ static int alg_setkey(struct sock *sk, char __user *ukey,
if (copy_from_user(key, ukey, keylen))
goto out;

+ err = -EBUSY;
+ lock_sock(sk);
+ if (ask->refcnt)
+ goto unlock;
+
err = type->setkey(ask->private, key, keylen);

+unlock:
+ release_sock(sk);
+
out:
sock_kzfree_s(sk, key, keylen);

return err;
}

+static int alg_setauthsize(struct sock *sk, unsigned int size)
+{
+ int err;
+ struct alg_sock *ask = alg_sk(sk);
+ const struct af_alg_type *type = ask->type;
+
+ err = -EBUSY;
+ lock_sock(sk);
+ if (ask->refcnt)
+ goto unlock;
+
+ err = type->setauthsize(ask->private, size);
+
+unlock:
+ release_sock(sk);
+
+ return err;
+}
+
static int alg_setsockopt(struct socket *sock, int level, int optname,
char __user *optval, unsigned int optlen)
{
@@ -224,7 +275,7 @@ static int alg_setsockopt(struct socket *sock, int level, int optname,
goto unlock;
if (!type->setauthsize)
goto unlock;
- err = type->setauthsize(ask->private, optlen);
+ err = alg_setauthsize(sk, optlen);
}

unlock:
@@ -264,7 +315,8 @@ int af_alg_accept(struct sock *sk, struct socket *newsock)

sk2->sk_family = PF_ALG;

- sock_hold(sk);
+ if (!ask->refcnt++)
+ sock_hold(sk);
alg_sk(sk2)->parent = sk;
alg_sk(sk2)->type = type;

diff --git a/include/crypto/if_alg.h b/include/crypto/if_alg.h
index 018afb2..589716f 100644
--- a/include/crypto/if_alg.h
+++ b/include/crypto/if_alg.h
@@ -30,6 +30,8 @@ struct alg_sock {

struct sock *parent;

+ unsigned int refcnt;
+
const struct af_alg_type *type;
void *private;
};
@@ -67,6 +69,7 @@ int af_alg_register_type(const struct af_alg_type *type);
int af_alg_unregister_type(const struct af_alg_type *type);

int af_alg_release(struct socket *sock);
+void af_alg_release_parent(struct sock *sk);
int af_alg_accept(struct sock *sk, struct socket *newsock);

int af_alg_make_sg(struct af_alg_sgl *sgl, struct iov_iter *iter, int len);
@@ -83,11 +86,6 @@ static inline struct alg_sock *alg_sk(struct sock *sk)
return (struct alg_sock *)sk;
}

-static inline void af_alg_release_parent(struct sock *sk)
-{
- sock_put(alg_sk(sk)->parent);
-}
-
static inline void af_alg_init_completion(struct af_alg_completion *completion)
{
init_completion(&completion->completion);
--
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

[Dmitry Vyukov]

On Tue, Dec 29, 2015 at 3:40 PM, Herbert Xu <her...@gondor.apana.org.au> wrote:
> On Thu, Dec 17, 2015 at 01:59:50PM +0100, Dmitry Vyukov wrote:
>>
>> The following program causes use-after-free in hash_sock_destruct:
>
> This patch should fix the problem. AFAIK everything that you have
> reported should now be fixed. If you still have issues please
> resubmit them (and please cc me). Thanks!

Thanks Herbert!
I will do another round of crypto testing with this patch (on top of
the other two patches) and report if I see any bugs.

> --
> You received this message because you are subscribed to the Google Groups "syzkaller" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller+...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

[Dmitry Vyukov]

On Tue, Dec 29, 2015 at 4:28 PM, Dmitry Vyukov <dvy...@google.com> wrote:
> On Tue, Dec 29, 2015 at 3:40 PM, Herbert Xu <her...@gondor.apana.org.au> wrote:
>> On Thu, Dec 17, 2015 at 01:59:50PM +0100, Dmitry Vyukov wrote:
>>>
>>> The following program causes use-after-free in hash_sock_destruct:
>>
>> This patch should fix the problem. AFAIK everything that you have
>> reported should now be fixed. If you still have issues please
>> resubmit them (and please cc me). Thanks!
>
> Thanks Herbert!
> I will do another round of crypto testing with this patch (on top of
> the other two patches) and report if I see any bugs.

Doh! Now +Herbert