Date:   Fri, 3 Mar 2017 11:05:43 +0100
From:   Dmitry Vyukov <dvyukov@...gle.com>
To:     mawilcox@...rosoft.com, Andrew Morton <akpm@...ux-foundation.org>,
        ross.zwisler@...ux.intel.com, Johannes Weiner <hannes@...xchg.org>,
        Jan Kara <jack@...e.cz>, LKML <linux-kernel@...r.kernel.org>,
        Tejun Heo <tj@...nel.org>
Cc:     syzkaller <syzkaller@...glegroups.com>
Subject: Re: idr: use-after-free write in ida_get_new_above

On Fri, Mar 3, 2017 at 10:51 AM, Dmitry Vyukov <dvyukov@...gle.com> wrote:
> Hello,
>
> I am hitting the following use-after-free reports while running
> syzkaller fuzzer on commit c82be9d2244aacea9851c86f4fb74694c99cd874:
>
> ==================================================================
> BUG: KASAN: use-after-free in ida_get_new_above+0x564/0x9b0
> lib/idr.c:295 at addr ffff880160ef3600
> Write of size 128 by task syz-executor1/1757
> CPU: 1 PID: 1757 Comm: syz-executor1 Not tainted 4.10.0+ #98
> Hardware name: Google Google Compute Engine/Google Compute Engine,
> BIOS Google 01/01/2011
> Call Trace:
>  __dump_stack lib/dump_stack.c:15 [inline]
>  dump_stack+0x2ee/0x3ef lib/dump_stack.c:51
>  kasan_object_err+0x1c/0x70 mm/kasan/report.c:162
>  print_address_description mm/kasan/report.c:200 [inline]
>  kasan_report_error mm/kasan/report.c:289 [inline]
>  kasan_report.part.2+0x1e5/0x4b0 mm/kasan/report.c:311
>  kasan_report+0x21/0x30 mm/kasan/report.c:298
>  check_memory_region_inline mm/kasan/kasan.c:315 [inline]
>  check_memory_region+0x139/0x190 mm/kasan/kasan.c:322
>  memset+0x23/0x40 mm/kasan/kasan.c:340
>  ida_get_new_above+0x564/0x9b0 lib/idr.c:295
>  ida_get_new include/linux/idr.h:209 [inline]
>  proc_alloc_inum+0x97/0x1f0 fs/proc/generic.c:201
>  proc_register+0x25/0x2f0 fs/proc/generic.c:338
>  proc_create_data+0x100/0x190 fs/proc/generic.c:509
>  proc_create include/linux/proc_fs.h:35 [inline]
>  ip_rt_do_proc_init+0x53/0x110 net/ipv4/route.c:382
>  ops_init+0x10a/0x530 net/core/net_namespace.c:115
>  setup_net+0x2ed/0x690 net/core/net_namespace.c:291
>  copy_net_ns+0x26c/0x530 net/core/net_namespace.c:396
>  create_new_namespaces+0x409/0x860 kernel/nsproxy.c:106
>  unshare_nsproxy_namespaces+0xae/0x1e0 kernel/nsproxy.c:205
>  SYSC_unshare kernel/fork.c:2281 [inline]
>  SyS_unshare+0x64e/0xfc0 kernel/fork.c:2231
>  entry_SYSCALL_64_fastpath+0x1f/0xc2
> RIP: 0033:0x44fb79
> RSP: 002b:00007f1b66930b58 EFLAGS: 00000212 ORIG_RAX: 0000000000000110
> RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 000000000044fb79
> RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000062020200
> RBP: 0000000062020200 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000212 R12: 00000000007082a0
> R13: 0000000000000004 R14: 0000000000000400 R15: 0000000000000000
> Object at ffff880160ef3600, in cache kmalloc-128 size: 128
> Allocated:
> PID = 1730
>  save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:57
>  save_stack+0x43/0xd0 mm/kasan/kasan.c:502
>  set_track mm/kasan/kasan.c:514 [inline]
>  kasan_kmalloc+0xaa/0xd0 mm/kasan/kasan.c:605
>  kmem_cache_alloc_trace+0x10b/0x670 mm/slab.c:3634
>  kmalloc include/linux/slab.h:490 [inline]
>  ida_pre_get+0xcd/0xe0 lib/radix-tree.c:2129
>  proc_alloc_inum+0xc5/0x1f0 fs/proc/generic.c:197
>  proc_register+0x25/0x2f0 fs/proc/generic.c:338
>  proc_create_data+0x100/0x190 fs/proc/generic.c:509
>  proc_create include/linux/proc_fs.h:35 [inline]
>  packet_net_init+0x8d/0xb0 net/packet/af_packet.c:4488
>  ops_init+0x10a/0x530 net/core/net_namespace.c:115
>  setup_net+0x2ed/0x690 net/core/net_namespace.c:291
>  copy_net_ns+0x26c/0x530 net/core/net_namespace.c:396
>  create_new_namespaces+0x409/0x860 kernel/nsproxy.c:106
>  unshare_nsproxy_namespaces+0xae/0x1e0 kernel/nsproxy.c:205
>  SYSC_unshare kernel/fork.c:2281 [inline]
>  SyS_unshare+0x64e/0xfc0 kernel/fork.c:2231
>  entry_SYSCALL_64_fastpath+0x1f/0xc2
> Freed:
> PID = 1648
>  save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:57
>  save_stack+0x43/0xd0 mm/kasan/kasan.c:502
>  set_track mm/kasan/kasan.c:514 [inline]
>  kasan_slab_free+0x6f/0xb0 mm/kasan/kasan.c:578
>  __cache_free mm/slab.c:3510 [inline]
>  kfree+0xd3/0x250 mm/slab.c:3827
>  ida_pre_get+0x8a/0xe0 lib/radix-tree.c:2133
>  ida_simple_get+0x168/0x320 lib/idr.c:443
>  __kernfs_new_node+0xfe/0x3c0 fs/kernfs/dir.c:633
>  kernfs_new_node+0x80/0xe0 fs/kernfs/dir.c:661
>  kernfs_create_dir_ns+0x41/0x140 fs/kernfs/dir.c:933
>  kernfs_create_dir include/linux/kernfs.h:477 [inline]
>  internal_create_group+0xf7/0x8f0 fs/sysfs/group.c:124
>  sysfs_create_group fs/sysfs/group.c:156 [inline]
>  sysfs_create_groups+0x97/0x130 fs/sysfs/group.c:183
>  device_add_groups drivers/base/core.c:1031 [inline]
>  device_add_attrs drivers/base/core.c:1058 [inline]
>  device_add+0xb2f/0x1650 drivers/base/core.c:1679
>  netdev_register_kobject+0x183/0x360 net/core/net-sysfs.c:1603
>  register_netdevice+0xa27/0xed0 net/core/dev.c:7228
>  tun_set_iff drivers/net/tun.c:1843 [inline]
>  __tun_chr_ioctl+0x1bd0/0x3b70 drivers/net/tun.c:2046
>  tun_chr_ioctl+0x2a/0x40 drivers/net/tun.c:2291
>  vfs_ioctl fs/ioctl.c:43 [inline]
>  do_vfs_ioctl+0x1bf/0x1790 fs/ioctl.c:683
>  SYSC_ioctl fs/ioctl.c:698 [inline]
>  SyS_ioctl+0x8f/0xc0 fs/ioctl.c:689
>  entry_SYSCALL_64_fastpath+0x1f/0xc2
> Memory state around the buggy address:
>  ffff880160ef3500: fc fc fc fc fc fc fc fc fb fb fb fb fb fb fb fb
>  ffff880160ef3580: fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
>>ffff880160ef3600: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>                    ^
>  ffff880160ef3680: fc fc fc fc fc fc fc fc 00 00 00 00 00 00 00 00
>  ffff880160ef3700: 00 00 00 00 00 00 00 00 fc fc fc fc fc fc fc fc
> ==================================================================
>
>
> I think we need:
>
> diff --git a/lib/radix-tree.c b/lib/radix-tree.c
> index 5ed506d648c4..d435916f6527 100644
> --- a/lib/radix-tree.c
> +++ b/lib/radix-tree.c
> @@ -2129,8 +2129,8 @@ int ida_pre_get(struct ida *ida, gfp_t gfp)
>                 struct ida_bitmap *bitmap = kmalloc(sizeof(*bitmap), gfp);
>                 if (!bitmap)
>                         return 0;
> -               bitmap = this_cpu_cmpxchg(ida_bitmap, NULL, bitmap);
> -               kfree(bitmap);
> +               if (!this_cpu_cmpxchg(ida_bitmap, NULL, bitmap))
> +                       kfree(bitmap);

I meant:

+               if (this_cpu_cmpxchg(ida_bitmap, NULL, bitmap))
+                       kfree(bitmap);


>         }
>
>
> Otherwise we both free the still installed old pointer and leak the new one.

Powered by blists - more mailing lists