diff mbox series

[1/1] net: crypto set sk to NULL when af_alg_release.

Message ID 1550741312-27390-2-git-send-email-tyhicks@canonical.com
State New
Headers show
Series CVE-2019-8912 - AF_ALG use after free | expand

Commit Message

Tyler Hicks Feb. 21, 2019, 9:28 a.m. UTC 
From: Mao Wenan <maowenan@huawei.com>

KASAN has found use-after-free in sockfs_setattr.
The existed commit 6d8c50dcb029 ("socket: close race condition between sock_close()
and sockfs_setattr()") is to fix this simillar issue, but it seems to ignore
that crypto module forgets to set the sk to NULL after af_alg_release.

KASAN report details as below:
BUG: KASAN: use-after-free in sockfs_setattr+0x120/0x150
Write of size 4 at addr ffff88837b956128 by task syz-executor0/4186

CPU: 2 PID: 4186 Comm: syz-executor0 Not tainted xxx + #1
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
1.10.2-1ubuntu1 04/01/2014
Call Trace:
 dump_stack+0xca/0x13e
 print_address_description+0x79/0x330
 ? vprintk_func+0x5e/0xf0
 kasan_report+0x18a/0x2e0
 ? sockfs_setattr+0x120/0x150
 sockfs_setattr+0x120/0x150
 ? sock_register+0x2d0/0x2d0
 notify_change+0x90c/0xd40
 ? chown_common+0x2ef/0x510
 chown_common+0x2ef/0x510
 ? chmod_common+0x3b0/0x3b0
 ? __lock_is_held+0xbc/0x160
 ? __sb_start_write+0x13d/0x2b0
 ? __mnt_want_write+0x19a/0x250
 do_fchownat+0x15c/0x190
 ? __ia32_sys_chmod+0x80/0x80
 ? trace_hardirqs_on_thunk+0x1a/0x1c
 __x64_sys_fchownat+0xbf/0x160
 ? lockdep_hardirqs_on+0x39a/0x5e0
 do_syscall_64+0xc8/0x580
 entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x462589
Code: f7 d8 64 89 02 b8 ff ff ff ff c3 66 0f 1f 44 00 00 48 89 f8 48 89
f7 48 89 d6 48 89
ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3
48 c7 c1 bc ff ff
ff f7 d8 64 89 01 48
RSP: 002b:00007fb4b2c83c58 EFLAGS: 00000246 ORIG_RAX: 0000000000000104
RAX: ffffffffffffffda RBX: 000000000072bfa0 RCX: 0000000000462589
RDX: 0000000000000000 RSI: 00000000200000c0 RDI: 0000000000000007
RBP: 0000000000000005 R08: 0000000000001000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00007fb4b2c846bc
R13: 00000000004bc733 R14: 00000000006f5138 R15: 00000000ffffffff

Allocated by task 4185:
 kasan_kmalloc+0xa0/0xd0
 __kmalloc+0x14a/0x350
 sk_prot_alloc+0xf6/0x290
 sk_alloc+0x3d/0xc00
 af_alg_accept+0x9e/0x670
 hash_accept+0x4a3/0x650
 __sys_accept4+0x306/0x5c0
 __x64_sys_accept4+0x98/0x100
 do_syscall_64+0xc8/0x580
 entry_SYSCALL_64_after_hwframe+0x49/0xbe

Freed by task 4184:
 __kasan_slab_free+0x12e/0x180
 kfree+0xeb/0x2f0
 __sk_destruct+0x4e6/0x6a0
 sk_destruct+0x48/0x70
 __sk_free+0xa9/0x270
 sk_free+0x2a/0x30
 af_alg_release+0x5c/0x70
 __sock_release+0xd3/0x280
 sock_close+0x1a/0x20
 __fput+0x27f/0x7f0
 task_work_run+0x136/0x1b0
 exit_to_usermode_loop+0x1a7/0x1d0
 do_syscall_64+0x461/0x580
 entry_SYSCALL_64_after_hwframe+0x49/0xbe

Syzkaller reproducer:
r0 = perf_event_open(&(0x7f0000000000)={0x0, 0x70, 0x0, 0x0, 0x0, 0x0,
0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, @perf_config_ext}, 0x0, 0x0,
0xffffffffffffffff, 0x0)
r1 = socket$alg(0x26, 0x5, 0x0)
getrusage(0x0, 0x0)
bind(r1, &(0x7f00000001c0)=@alg={0x26, 'hash\x00', 0x0, 0x0,
'sha256-ssse3\x00'}, 0x80)
r2 = accept(r1, 0x0, 0x0)
r3 = accept4$unix(r2, 0x0, 0x0, 0x0)
r4 = dup3(r3, r0, 0x0)
fchownat(r4, &(0x7f00000000c0)='\x00', 0x0, 0x0, 0x1000)

Fixes: 6d8c50dcb029 ("socket: close race condition between sock_close() and sockfs_setattr()")
Signed-off-by: Mao Wenan <maowenan@huawei.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

CVE-2019-8912

(cherry picked from commit 9060cb719e61b685ec0102574e10337fa5f445ea)
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
---
 crypto/af_alg.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Colin Ian King Feb. 21, 2019, 9:30 a.m. UTC | #1 
On 21/02/2019 09:28, Tyler Hicks wrote:
> From: Mao Wenan <maowenan@huawei.com>
> 
> KASAN has found use-after-free in sockfs_setattr.
> The existed commit 6d8c50dcb029 ("socket: close race condition between sock_close()
> and sockfs_setattr()") is to fix this simillar issue, but it seems to ignore
> that crypto module forgets to set the sk to NULL after af_alg_release.
> 
> KASAN report details as below:
> BUG: KASAN: use-after-free in sockfs_setattr+0x120/0x150
> Write of size 4 at addr ffff88837b956128 by task syz-executor0/4186
> 
> CPU: 2 PID: 4186 Comm: syz-executor0 Not tainted xxx + #1
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> 1.10.2-1ubuntu1 04/01/2014
> Call Trace:
>  dump_stack+0xca/0x13e
>  print_address_description+0x79/0x330
>  ? vprintk_func+0x5e/0xf0
>  kasan_report+0x18a/0x2e0
>  ? sockfs_setattr+0x120/0x150
>  sockfs_setattr+0x120/0x150
>  ? sock_register+0x2d0/0x2d0
>  notify_change+0x90c/0xd40
>  ? chown_common+0x2ef/0x510
>  chown_common+0x2ef/0x510
>  ? chmod_common+0x3b0/0x3b0
>  ? __lock_is_held+0xbc/0x160
>  ? __sb_start_write+0x13d/0x2b0
>  ? __mnt_want_write+0x19a/0x250
>  do_fchownat+0x15c/0x190
>  ? __ia32_sys_chmod+0x80/0x80
>  ? trace_hardirqs_on_thunk+0x1a/0x1c
>  __x64_sys_fchownat+0xbf/0x160
>  ? lockdep_hardirqs_on+0x39a/0x5e0
>  do_syscall_64+0xc8/0x580
>  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x462589
> Code: f7 d8 64 89 02 b8 ff ff ff ff c3 66 0f 1f 44 00 00 48 89 f8 48 89
> f7 48 89 d6 48 89
> ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3
> 48 c7 c1 bc ff ff
> ff f7 d8 64 89 01 48
> RSP: 002b:00007fb4b2c83c58 EFLAGS: 00000246 ORIG_RAX: 0000000000000104
> RAX: ffffffffffffffda RBX: 000000000072bfa0 RCX: 0000000000462589
> RDX: 0000000000000000 RSI: 00000000200000c0 RDI: 0000000000000007
> RBP: 0000000000000005 R08: 0000000000001000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000246 R12: 00007fb4b2c846bc
> R13: 00000000004bc733 R14: 00000000006f5138 R15: 00000000ffffffff
> 
> Allocated by task 4185:
>  kasan_kmalloc+0xa0/0xd0
>  __kmalloc+0x14a/0x350
>  sk_prot_alloc+0xf6/0x290
>  sk_alloc+0x3d/0xc00
>  af_alg_accept+0x9e/0x670
>  hash_accept+0x4a3/0x650
>  __sys_accept4+0x306/0x5c0
>  __x64_sys_accept4+0x98/0x100
>  do_syscall_64+0xc8/0x580
>  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> 
> Freed by task 4184:
>  __kasan_slab_free+0x12e/0x180
>  kfree+0xeb/0x2f0
>  __sk_destruct+0x4e6/0x6a0
>  sk_destruct+0x48/0x70
>  __sk_free+0xa9/0x270
>  sk_free+0x2a/0x30
>  af_alg_release+0x5c/0x70
>  __sock_release+0xd3/0x280
>  sock_close+0x1a/0x20
>  __fput+0x27f/0x7f0
>  task_work_run+0x136/0x1b0
>  exit_to_usermode_loop+0x1a7/0x1d0
>  do_syscall_64+0x461/0x580
>  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> 
> Syzkaller reproducer:
> r0 = perf_event_open(&(0x7f0000000000)={0x0, 0x70, 0x0, 0x0, 0x0, 0x0,
> 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
> 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
> 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, @perf_config_ext}, 0x0, 0x0,
> 0xffffffffffffffff, 0x0)
> r1 = socket$alg(0x26, 0x5, 0x0)
> getrusage(0x0, 0x0)
> bind(r1, &(0x7f00000001c0)=@alg={0x26, 'hash\x00', 0x0, 0x0,
> 'sha256-ssse3\x00'}, 0x80)
> r2 = accept(r1, 0x0, 0x0)
> r3 = accept4$unix(r2, 0x0, 0x0, 0x0)
> r4 = dup3(r3, r0, 0x0)
> fchownat(r4, &(0x7f00000000c0)='\x00', 0x0, 0x0, 0x1000)
> 
> Fixes: 6d8c50dcb029 ("socket: close race condition between sock_close() and sockfs_setattr()")
> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> 
> CVE-2019-8912
> 
> (cherry picked from commit 9060cb719e61b685ec0102574e10337fa5f445ea)
> Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
> ---
>  crypto/af_alg.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/crypto/af_alg.c b/crypto/af_alg.c
> index 671e42ff21d4..493002afae73 100644
> --- a/crypto/af_alg.c
> +++ b/crypto/af_alg.c
> @@ -122,8 +122,10 @@ static void alg_do_release(const struct af_alg_type *type, void *private)
>  
>  int af_alg_release(struct socket *sock)
>  {
> -	if (sock->sk)
> +	if (sock->sk) {
>  		sock_put(sock->sk);
> +		sock->sk = NULL;
> +	}
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(af_alg_release);
> 

Clean cherry pick.

Acked-by: Colin Ian King <colin.king@canonical.com>
Kai-Heng Feng Feb. 22, 2019, 8:05 a.m. UTC | #2 
> On Feb 21, 2019, at 10:28 AM, Tyler Hicks <tyhicks@canonical.com> wrote:
> 
> From: Mao Wenan <maowenan@huawei.com>
> 
> KASAN has found use-after-free in sockfs_setattr.
> The existed commit 6d8c50dcb029 ("socket: close race condition between sock_close()
> and sockfs_setattr()") is to fix this simillar issue, but it seems to ignore
> that crypto module forgets to set the sk to NULL after af_alg_release.
> 
> KASAN report details as below:
> BUG: KASAN: use-after-free in sockfs_setattr+0x120/0x150
> Write of size 4 at addr ffff88837b956128 by task syz-executor0/4186
> 
> CPU: 2 PID: 4186 Comm: syz-executor0 Not tainted xxx + #1
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> 1.10.2-1ubuntu1 04/01/2014
> Call Trace:
> dump_stack+0xca/0x13e
> print_address_description+0x79/0x330
> ? vprintk_func+0x5e/0xf0
> kasan_report+0x18a/0x2e0
> ? sockfs_setattr+0x120/0x150
> sockfs_setattr+0x120/0x150
> ? sock_register+0x2d0/0x2d0
> notify_change+0x90c/0xd40
> ? chown_common+0x2ef/0x510
> chown_common+0x2ef/0x510
> ? chmod_common+0x3b0/0x3b0
> ? __lock_is_held+0xbc/0x160
> ? __sb_start_write+0x13d/0x2b0
> ? __mnt_want_write+0x19a/0x250
> do_fchownat+0x15c/0x190
> ? __ia32_sys_chmod+0x80/0x80
> ? trace_hardirqs_on_thunk+0x1a/0x1c
> __x64_sys_fchownat+0xbf/0x160
> ? lockdep_hardirqs_on+0x39a/0x5e0
> do_syscall_64+0xc8/0x580
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x462589
> Code: f7 d8 64 89 02 b8 ff ff ff ff c3 66 0f 1f 44 00 00 48 89 f8 48 89
> f7 48 89 d6 48 89
> ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3
> 48 c7 c1 bc ff ff
> ff f7 d8 64 89 01 48
> RSP: 002b:00007fb4b2c83c58 EFLAGS: 00000246 ORIG_RAX: 0000000000000104
> RAX: ffffffffffffffda RBX: 000000000072bfa0 RCX: 0000000000462589
> RDX: 0000000000000000 RSI: 00000000200000c0 RDI: 0000000000000007
> RBP: 0000000000000005 R08: 0000000000001000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000246 R12: 00007fb4b2c846bc
> R13: 00000000004bc733 R14: 00000000006f5138 R15: 00000000ffffffff
> 
> Allocated by task 4185:
> kasan_kmalloc+0xa0/0xd0
> __kmalloc+0x14a/0x350
> sk_prot_alloc+0xf6/0x290
> sk_alloc+0x3d/0xc00
> af_alg_accept+0x9e/0x670
> hash_accept+0x4a3/0x650
> __sys_accept4+0x306/0x5c0
> __x64_sys_accept4+0x98/0x100
> do_syscall_64+0xc8/0x580
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
> 
> Freed by task 4184:
> __kasan_slab_free+0x12e/0x180
> kfree+0xeb/0x2f0
> __sk_destruct+0x4e6/0x6a0
> sk_destruct+0x48/0x70
> __sk_free+0xa9/0x270
> sk_free+0x2a/0x30
> af_alg_release+0x5c/0x70
> __sock_release+0xd3/0x280
> sock_close+0x1a/0x20
> __fput+0x27f/0x7f0
> task_work_run+0x136/0x1b0
> exit_to_usermode_loop+0x1a7/0x1d0
> do_syscall_64+0x461/0x580
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
> 
> Syzkaller reproducer:
> r0 = perf_event_open(&(0x7f0000000000)={0x0, 0x70, 0x0, 0x0, 0x0, 0x0,
> 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
> 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
> 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, @perf_config_ext}, 0x0, 0x0,
> 0xffffffffffffffff, 0x0)
> r1 = socket$alg(0x26, 0x5, 0x0)
> getrusage(0x0, 0x0)
> bind(r1, &(0x7f00000001c0)=@alg={0x26, 'hash\x00', 0x0, 0x0,
> 'sha256-ssse3\x00'}, 0x80)
> r2 = accept(r1, 0x0, 0x0)
> r3 = accept4$unix(r2, 0x0, 0x0, 0x0)
> r4 = dup3(r3, r0, 0x0)
> fchownat(r4, &(0x7f00000000c0)='\x00', 0x0, 0x0, 0x1000)
> 
> Fixes: 6d8c50dcb029 ("socket: close race condition between sock_close() and sockfs_setattr()")
> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> 
> CVE-2019-8912
> 
> (cherry picked from commit 9060cb719e61b685ec0102574e10337fa5f445ea)
> Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
> ---
> crypto/af_alg.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/crypto/af_alg.c b/crypto/af_alg.c
> index 671e42ff21d4..493002afae73 100644
> --- a/crypto/af_alg.c
> +++ b/crypto/af_alg.c
> @@ -122,8 +122,10 @@ static void alg_do_release(const struct af_alg_type *type, void *private)
> 
> int af_alg_release(struct socket *sock)
> {
> -	if (sock->sk)
> +	if (sock->sk) {
> 		sock_put(sock->sk);
> +		sock->sk = NULL;
> +	}
> 	return 0;
> }
> EXPORT_SYMBOL_GPL(af_alg_release);

Clean cherry-pick, positive test result.
Acked-by: Kai-Heng Feng <kai.heng.feng@canonical.com>

> -- 
> 2.7.4
> 
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
diff mbox series

Patch

diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index 671e42ff21d4..493002afae73 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -122,8 +122,10 @@  static void alg_do_release(const struct af_alg_type *type, void *private)
 
 int af_alg_release(struct socket *sock)
 {
-	if (sock->sk)
+	if (sock->sk) {
 		sock_put(sock->sk);
+		sock->sk = NULL;
+	}
 	return 0;
 }
 EXPORT_SYMBOL_GPL(af_alg_release);