Re: KASAN: use-after-free Read in drm_gem_object_release

Re: KASAN: use-after-free Read in drm_gem_object_release

[Dmitry Vyukov]

On Thu, Oct 25, 2018 at 9:18 PM, syzbot
<syzbot+e73f2fb5ed5a5df36d33@syzkaller.appspotmail.com> wrote:
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit:    bd6bf7c10484 Merge tag 'pci-v4.20-changes' of git://git.ke..
> git tree:       upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=1448a683400000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=2dd8629d56664133
> dashboard link: https://syzkaller.appspot.com/bug?extid=e73f2fb5ed5a5df36d33
> compiler:       gcc (GCC) 8.0.1 20180413 (experimental)
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=11331de5400000
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=1334e64d400000

+drm maintainers

> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+e73f2fb5ed5a5df36d33@syzkaller.appspotmail.com
>
>  drm_mode_create_dumb+0x28d/0x310 drivers/gpu/drm/drm_dumb_buffers.c:92
>  drm_mode_create_dumb_ioctl+0x25/0x30 drivers/gpu/drm/drm_dumb_buffers.c:98
>  drm_ioctl_kernel+0x245/0x2f0 drivers/gpu/drm/drm_ioctl.c:751
>  drm_ioctl+0x57a/0xb20 drivers/gpu/drm/drm_ioctl.c:847
> ==================================================================
> BUG: KASAN: use-after-free in drm_gem_object_release+0xf1/0x110
> drivers/gpu/drm/drm_gem.c:813
> Read of size 8 at addr ffff8801d83d3410 by task syz-executor977/6742
>
>  vfs_ioctl fs/ioctl.c:46 [inline]
>  file_ioctl fs/ioctl.c:501 [inline]
>  do_vfs_ioctl+0x1de/0x1720 fs/ioctl.c:685
>  ksys_ioctl+0xa9/0xd0 fs/ioctl.c:702
>  __do_sys_ioctl fs/ioctl.c:709 [inline]
>  __se_sys_ioctl fs/ioctl.c:707 [inline]
>  __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:707
>  do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
>  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x445989
> Code: e8 4c e8 ff ff 48 83 c4 18 c3 0f 1f 80 00 00 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
> 0f 83 6b c8 fb ff c3 66 2e 0f 1f 84 00 00 00 00
> RSP: 002b:00007ffcf076f4e8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
> RAX: ffffffffffffffda RBX: 00007ffcf076f500 RCX: 0000000000445989
> RDX: 0000000020000000 RSI: ffffffffffffffb2 RDI: 0000000000000003
> RBP: 0000000000000004 R08: 0000000000000001 R09: 0000000000000100
> R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
> R13: ffffffffffffffff R14: 0000000000000000 R15: 0000000000000000
> CPU: 0 PID: 6742 Comm: syz-executor977 Not tainted 4.19.0+ #80
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 01/01/2011
> Call Trace:
>  __dump_stack lib/dump_stack.c:77 [inline]
>  dump_stack+0x244/0x39d lib/dump_stack.c:113
>  print_address_description.cold.7+0x9/0x1ff mm/kasan/report.c:256
>  kasan_report_error mm/kasan/report.c:354 [inline]
>  kasan_report.cold.8+0x242/0x309 mm/kasan/report.c:412
>  __asan_report_load8_noabort+0x14/0x20 mm/kasan/report.c:433
>  drm_gem_object_release+0xf1/0x110 drivers/gpu/drm/drm_gem.c:813
>  __vgem_gem_destroy drivers/gpu/drm/vgem/vgem_drv.c:175 [inline]
>  vgem_gem_create drivers/gpu/drm/vgem/vgem_drv.c:199 [inline]
>  vgem_gem_dumb_create+0x1f8/0x260 drivers/gpu/drm/vgem/vgem_drv.c:214
>  drm_mode_create_dumb+0x28d/0x310 drivers/gpu/drm/drm_dumb_buffers.c:92
>  drm_mode_create_dumb_ioctl+0x25/0x30 drivers/gpu/drm/drm_dumb_buffers.c:98
>  drm_ioctl_kernel+0x245/0x2f0 drivers/gpu/drm/drm_ioctl.c:751
>  drm_ioctl+0x57a/0xb20 drivers/gpu/drm/drm_ioctl.c:847
>  vfs_ioctl fs/ioctl.c:46 [inline]
>  file_ioctl fs/ioctl.c:501 [inline]
>  do_vfs_ioctl+0x1de/0x1720 fs/ioctl.c:685
>  ksys_ioctl+0xa9/0xd0 fs/ioctl.c:702
>  __do_sys_ioctl fs/ioctl.c:709 [inline]
>  __se_sys_ioctl fs/ioctl.c:707 [inline]
>  __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:707
>  do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
>  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x445989
> Code: e8 4c e8 ff ff 48 83 c4 18 c3 0f 1f 80 00 00 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
> 0f 83 6b c8 fb ff c3 66 2e 0f 1f 84 00 00 00 00
> RSP: 002b:00007ffcf076f4e8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
> RAX: ffffffffffffffda RBX: 00007ffcf076f500 RCX: 0000000000445989
> RDX: 0000000020000000 RSI: ffffffffffffffb2 RDI: 0000000000000003
> RBP: 0000000000000004 R08: 0000000000000001 R09: 0000000000000100
> R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
> R13: ffffffffffffffff R14: 0000000000000000 R15: 0000000000000000
>
> Allocated by task 6742:
>  save_stack+0x43/0xd0 mm/kasan/kasan.c:448
>  set_track mm/kasan/kasan.c:460 [inline]
>  kasan_kmalloc+0xc7/0xe0 mm/kasan/kasan.c:553
>  kmem_cache_alloc_trace+0x152/0x750 mm/slab.c:3620
>  kmalloc include/linux/slab.h:513 [inline]
>  kzalloc include/linux/slab.h:707 [inline]
>  __vgem_gem_create+0x4c/0x100 drivers/gpu/drm/vgem/vgem_drv.c:158
>  vgem_gem_create drivers/gpu/drm/vgem/vgem_drv.c:187 [inline]
>  vgem_gem_dumb_create+0xce/0x260 drivers/gpu/drm/vgem/vgem_drv.c:214
>  drm_mode_create_dumb+0x28d/0x310 drivers/gpu/drm/drm_dumb_buffers.c:92
>  drm_mode_create_dumb_ioctl+0x25/0x30 drivers/gpu/drm/drm_dumb_buffers.c:98
>  drm_ioctl_kernel+0x245/0x2f0 drivers/gpu/drm/drm_ioctl.c:751
>  drm_ioctl+0x57a/0xb20 drivers/gpu/drm/drm_ioctl.c:847
>  vfs_ioctl fs/ioctl.c:46 [inline]
>  file_ioctl fs/ioctl.c:501 [inline]
>  do_vfs_ioctl+0x1de/0x1720 fs/ioctl.c:685
>  ksys_ioctl+0xa9/0xd0 fs/ioctl.c:702
>  __do_sys_ioctl fs/ioctl.c:709 [inline]
>  __se_sys_ioctl fs/ioctl.c:707 [inline]
>  __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:707
>  do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
>  entry_SYSCALL_64_after_hwframe+0x49/0xbe
>
> Freed by task 6742:
>  save_stack+0x43/0xd0 mm/kasan/kasan.c:448
>  set_track mm/kasan/kasan.c:460 [inline]
>  __kasan_slab_free+0x102/0x150 mm/kasan/kasan.c:521
>  kasan_slab_free+0xe/0x10 mm/kasan/kasan.c:528
>  __cache_free mm/slab.c:3498 [inline]
>  kfree+0xcf/0x230 mm/slab.c:3813
>  vgem_gem_free_object+0xb6/0xe0 drivers/gpu/drm/vgem/vgem_drv.c:61
>  drm_gem_object_free+0xf1/0x2b0 drivers/gpu/drm/drm_gem.c:839
>  kref_put include/linux/kref.h:70 [inline]
>  drm_gem_object_put_unlocked+0x14c/0x180 drivers/gpu/drm/drm_gem.c:895
>  vgem_gem_create drivers/gpu/drm/vgem/vgem_drv.c:192 [inline]
>  vgem_gem_dumb_create+0x120/0x260 drivers/gpu/drm/vgem/vgem_drv.c:214
>  drm_mode_create_dumb+0x28d/0x310 drivers/gpu/drm/drm_dumb_buffers.c:92
>  drm_mode_create_dumb_ioctl+0x25/0x30 drivers/gpu/drm/drm_dumb_buffers.c:98
>  drm_ioctl_kernel+0x245/0x2f0 drivers/gpu/drm/drm_ioctl.c:751
>  drm_ioctl+0x57a/0xb20 drivers/gpu/drm/drm_ioctl.c:847
>  vfs_ioctl fs/ioctl.c:46 [inline]
>  file_ioctl fs/ioctl.c:501 [inline]
>  do_vfs_ioctl+0x1de/0x1720 fs/ioctl.c:685
>  ksys_ioctl+0xa9/0xd0 fs/ioctl.c:702
>  __do_sys_ioctl fs/ioctl.c:709 [inline]
>  __se_sys_ioctl fs/ioctl.c:707 [inline]
>  __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:707
>  do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
>  entry_SYSCALL_64_after_hwframe+0x49/0xbe
>
> The buggy address belongs to the object at ffff8801d83d3300
>  which belongs to the cache kmalloc-512 of size 512
> The buggy address is located 272 bytes inside of
>  512-byte region [ffff8801d83d3300, ffff8801d83d3500)
> The buggy address belongs to the page:
> page:ffffea000760f4c0 count:1 mapcount:0 mapping:ffff8801da800940 index:0x0
> flags: 0x2fffc0000000100(slab)
> raw: 02fffc0000000100 ffffea0007473948 ffffea0007471dc8 ffff8801da800940
> raw: 0000000000000000 ffff8801d83d3080 0000000100000006 0000000000000000
> page dumped because: kasan: bad access detected
>
> Memory state around the buggy address:
>  ffff8801d83d3300: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>  ffff8801d83d3380: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>>
>> ffff8801d83d3400: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>
>                          ^
>  ffff8801d83d3480: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>  ffff8801d83d3500: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> ==================================================================
> ==================================================================
> CPU: 0 PID: 6778 Comm: syz-executor977 Tainted: G    B             4.19.0+
> #80
> BUG: KASAN: double-free or invalid-free in __vgem_gem_destroy
> drivers/gpu/drm/vgem/vgem_drv.c:176 [inline]
> BUG: KASAN: double-free or invalid-free in vgem_gem_create
> drivers/gpu/drm/vgem/vgem_drv.c:199 [inline]
> BUG: KASAN: double-free or invalid-free in vgem_gem_dumb_create+0x203/0x260
> drivers/gpu/drm/vgem/vgem_drv.c:214
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 01/01/2011
>
> Call Trace:
>  __dump_stack lib/dump_stack.c:77 [inline]
>  dump_stack+0x244/0x39d lib/dump_stack.c:113
>  fail_dump lib/fault-inject.c:51 [inline]
>  should_fail.cold.4+0xa/0x17 lib/fault-inject.c:149
>  __should_failslab+0x124/0x180 mm/failslab.c:32
>  should_failslab+0x9/0x14 mm/slab_common.c:1557
>  slab_pre_alloc_hook mm/slab.h:423 [inline]
>  slab_alloc mm/slab.c:3378 [inline]
>  kmem_cache_alloc_trace+0x2d7/0x750 mm/slab.c:3618
>  kmalloc include/linux/slab.h:513 [inline]
>  drm_vma_node_allow+0x5f/0x290 drivers/gpu/drm/drm_vma_manager.c:277
>  drm_gem_handle_create_tail+0x233/0x440 drivers/gpu/drm/drm_gem.c:409
>  drm_gem_handle_create+0x52/0x60 drivers/gpu/drm/drm_gem.c:452
>  vgem_gem_create drivers/gpu/drm/vgem/vgem_drv.c:191 [inline]
>  vgem_gem_dumb_create+0x115/0x260 drivers/gpu/drm/vgem/vgem_drv.c:214
>  drm_mode_create_dumb+0x28d/0x310 drivers/gpu/drm/drm_dumb_buffers.c:92
>  drm_mode_create_dumb_ioctl+0x25/0x30 drivers/gpu/drm/drm_dumb_buffers.c:98
>  drm_ioctl_kernel+0x245/0x2f0 drivers/gpu/drm/drm_ioctl.c:751
>  drm_ioctl+0x57a/0xb20 drivers/gpu/drm/drm_ioctl.c:847
>  vfs_ioctl fs/ioctl.c:46 [inline]
>  file_ioctl fs/ioctl.c:501 [inline]
>  do_vfs_ioctl+0x1de/0x1720 fs/ioctl.c:685
>  ksys_ioctl+0xa9/0xd0 fs/ioctl.c:702
>  __do_sys_ioctl fs/ioctl.c:709 [inline]
>  __se_sys_ioctl fs/ioctl.c:707 [inline]
>  __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:707
>  do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
>  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x445989
> Code: e8 4c e8 ff ff 48 83 c4 18 c3 0f 1f 80 00 00 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
> 0f 83 6b c8 fb ff c3 66 2e 0f 1f 84 00 00 00 00
> RSP: 002b:00007ffcf076f4e8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
> RAX: ffffffffffffffda RBX: 00007ffcf076f500 RCX: 0000000000445989
> RDX: 0000000020000000 RSI: ffffffffffffffb2 RDI: 0000000000000003
> RBP: 0000000000000004 R08: 0000000000000001 R09: 0000000000000100
> R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
> R13: ffffffffffffffff R14: 0000000000000000 R15: 0000000000000000
> CPU: 1 PID: 6767 Comm: syz-executor977 Tainted: G    B             4.19.0+
> #80
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 01/01/2011
> Call Trace:
>  __dump_stack lib/dump_stack.c:77 [inline]
>  dump_stack+0x244/0x39d lib/dump_stack.c:113
>  print_address_description.cold.7+0x9/0x1ff mm/kasan/report.c:256
>  kasan_report_invalid_free+0x64/0xa0 mm/kasan/report.c:336
>  __kasan_slab_free+0x13a/0x150 mm/kasan/kasan.c:501
>  kasan_slab_free+0xe/0x10 mm/kasan/kasan.c:528
>  __cache_free mm/slab.c:3498 [inline]
>  kfree+0xcf/0x230 mm/slab.c:3813
>  __vgem_gem_destroy drivers/gpu/drm/vgem/vgem_drv.c:176 [inline]
>  vgem_gem_create drivers/gpu/drm/vgem/vgem_drv.c:199 [inline]
>  vgem_gem_dumb_create+0x203/0x260 drivers/gpu/drm/vgem/vgem_drv.c:214
>  drm_mode_create_dumb+0x28d/0x310 drivers/gpu/drm/drm_dumb_buffers.c:92
>  drm_mode_create_dumb_ioctl+0x25/0x30 drivers/gpu/drm/drm_dumb_buffers.c:98
>  drm_ioctl_kernel+0x245/0x2f0 drivers/gpu/drm/drm_ioctl.c:751
>  drm_ioctl+0x57a/0xb20 drivers/gpu/drm/drm_ioctl.c:847
>  vfs_ioctl fs/ioctl.c:46 [inline]
>  file_ioctl fs/ioctl.c:501 [inline]
>  do_vfs_ioctl+0x1de/0x1720 fs/ioctl.c:685
>  ksys_ioctl+0xa9/0xd0 fs/ioctl.c:702
>  __do_sys_ioctl fs/ioctl.c:709 [inline]
>  __se_sys_ioctl fs/ioctl.c:707 [inline]
>  __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:707
>  do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
>  entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x445989
> Code: e8 4c e8 ff ff 48 83 c4 18 c3 0f 1f 80 00 00 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
> 0f 83 6b c8 fb ff c3 66 2e 0f 1f 84 00 00 00 00
> RSP: 002b:00007ffcf076f4e8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
> RAX: ffffffffffffffda RBX: 00007ffcf076f500 RCX: 0000000000445989
> RDX: 0000000020000000 RSI: ffffffffffffffb2 RDI: 0000000000000003
> RBP: 0000000000000004 R08: 0000000000000001 R09: 0000000000000100
> R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
> R13: ffffffffffffffff R14: 0000000000000000 R15: 0000000000000000
>
> Allocated by task 6767:
>  save_stack+0x43/0xd0 mm/kasan/kasan.c:448
>  set_track mm/kasan/kasan.c:460 [inline]
>  kasan_kmalloc+0xc7/0xe0 mm/kasan/kasan.c:553
>  kmem_cache_alloc_trace+0x152/0x750 mm/slab.c:3620
>  kmalloc include/linux/slab.h:513 [inline]
>  kzalloc include/linux/slab.h:707 [inline]
>  __vgem_gem_create+0x4c/0x100 drivers/gpu/drm/vgem/vgem_drv.c:158
>  vgem_gem_create drivers/gpu/drm/vgem/vgem_drv.c:187 [inline]
>  vgem_gem_dumb_create+0xce/0x260 drivers/gpu/drm/vgem/vgem_drv.c:214
>  drm_mode_create_dumb+0x28d/0x310 drivers/gpu/drm/drm_dumb_buffers.c:92
>  drm_mode_create_dumb_ioctl+0x25/0x30 drivers/gpu/drm/drm_dumb_buffers.c:98
>  drm_ioctl_kernel+0x245/0x2f0 drivers/gpu/drm/drm_ioctl.c:751
>  drm_ioctl+0x57a/0xb20 drivers/gpu/drm/drm_ioctl.c:847
>  vfs_ioctl fs/ioctl.c:46 [inline]
>  file_ioctl fs/ioctl.c:501 [inline]
>  do_vfs_ioctl+0x1de/0x1720 fs/ioctl.c:685
>  ksys_ioctl+0xa9/0xd0 fs/ioctl.c:702
>  __do_sys_ioctl fs/ioctl.c:709 [inline]
>  __se_sys_ioctl fs/ioctl.c:707 [inline]
>  __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:707
>  do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
>  entry_SYSCALL_64_after_hwframe+0x49/0xbe
>
> Freed by task 6767:
>  save_stack+0x43/0xd0 mm/kasan/kasan.c:448
>  set_track mm/kasan/kasan.c:460 [inline]
>  __kasan_slab_free+0x102/0x150 mm/kasan/kasan.c:521
>  kasan_slab_free+0xe/0x10 mm/kasan/kasan.c:528
>  __cache_free mm/slab.c:3498 [inline]
>  kfree+0xcf/0x230 mm/slab.c:3813
>  vgem_gem_free_object+0xb6/0xe0 drivers/gpu/drm/vgem/vgem_drv.c:61
>  drm_gem_object_free+0xf1/0x2b0 drivers/gpu/drm/drm_gem.c:839
>  kref_put include/linux/kref.h:70 [inline]
>  drm_gem_object_put_unlocked+0x14c/0x180 drivers/gpu/drm/drm_gem.c:895
>  vgem_gem_create drivers/gpu/drm/vgem/vgem_drv.c:192 [inline]
>  vgem_gem_dumb_create+0x120/0x260 drivers/gpu/drm/vgem/vgem_drv.c:214
>  drm_mode_create_dumb+0x28d/0x310 drivers/gpu/drm/drm_dumb_buffers.c:92
>  drm_mode_create_dumb_ioctl+0x25/0x30 drivers/gpu/drm/drm_dumb_buffers.c:98
>  drm_ioctl_kernel+0x245/0x2f0 drivers/gpu/drm/drm_ioctl.c:751
>  drm_ioctl+0x57a/0xb20 drivers/gpu/drm/drm_ioctl.c:847
>  vfs_ioctl fs/ioctl.c:46 [inline]
>  file_ioctl fs/ioctl.c:501 [inline]
>  do_vfs_ioctl+0x1de/0x1720 fs/ioctl.c:685
>  ksys_ioctl+0xa9/0xd0 fs/ioctl.c:702
>  __do_sys_ioctl fs/ioctl.c:709 [inline]
>  __se_sys_ioctl fs/ioctl.c:707 [inline]
>  __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:707
>  do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
>  entry_SYSCALL_64_after_hwframe+0x49/0xbe
>
> The buggy address belongs to the object at ffff8801d4974080
>  which belongs to the cache kmalloc-512 of size 512
> The buggy address is located 0 bytes inside of
>  512-byte region [ffff8801d4974080, ffff8801d4974280)
> The buggy address belongs to the page:
> page:ffffea0007525d00 count:1 mapcount:0 mapping:ffff8801da800940 index:0x0
> flags: 0x2fffc0000000100(slab)
> raw: 02fffc0000000100 ffffea0007558e48 ffffea00071d0508 ffff8801da800940
> raw: 0000000000000000 ffff8801d4974080 0000000100000006 0000000000000000
> page dumped because: kasan: bad access detected
>
> Memory state around the buggy address:
>  ffff8801d4973f80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>  ffff8801d4974000: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>>
>> ffff8801d4974080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>
>                    ^
>  ffff8801d4974100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>  ffff8801d4974180: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> ==================================================================
> ==================================================================
>
>
> ---
> This bug is generated by a bot. It may contain errors.
> See https://goo.gl/tpsmEJ for more information about syzbot.
> syzbot engineers can be reached at syzkaller@googlegroups.com.
>
> syzbot will keep track of this bug report. See:
> https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
> syzbot.
> syzbot can test patches for this bug, for details see:
> https://goo.gl/tpsmEJ#testing-patches
>
> --
> You received this message because you are subscribed to the Google Groups
> "syzkaller-bugs" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to syzkaller-bugs+unsubscribe@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/syzkaller-bugs/00000000000053fea105791276d8%40google.com.
> For more options, visit https://groups.google.com/d/optout.

[PATCH] drm/vgem: fix use-after-free when drm_gem_handle_create() fails

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

If drm_gem_handle_create() fails in vgem_gem_create(), then the
drm_vgem_gem_object is freed twice: once when the reference is dropped
by drm_gem_object_put_unlocked(), and again by __vgem_gem_destroy().

This was hit by syzkaller using fault injection.

Fix it by skipping the second free.

Reported-by: syzbot+e73f2fb5ed5a5df36d33@syzkaller.appspotmail.com
Fixes: 5ba6c9ff961a ("drm/vgem: Fix mmaping")
Cc: stable@vger.kernel.org
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 drivers/gpu/drm/vgem/vgem_drv.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
index 5930facd6d2d8..70646d9da1596 100644
--- a/drivers/gpu/drm/vgem/vgem_drv.c
+++ b/drivers/gpu/drm/vgem/vgem_drv.c
@@ -189,15 +189,13 @@ static struct drm_gem_object *vgem_gem_create(struct drm_device *dev,
 		return ERR_CAST(obj);
 
 	ret = drm_gem_handle_create(file, &obj->base, handle);
+
 	drm_gem_object_put_unlocked(&obj->base);
+
 	if (ret)
-		goto err;
+		return ERR_PTR(ret);
 
 	return &obj->base;
-
-err:
-	__vgem_gem_destroy(obj);
-	return ERR_PTR(ret);
 }
 
 static int vgem_gem_dumb_create(struct drm_file *file, struct drm_device *dev,
-- 
2.21.0.rc2.261.ga7da99ff1b-goog

Re: [PATCH] drm/vgem: fix use-after-free when drm_gem_handle_create() fails

[Chris Wilson]

Quoting Eric Biggers (2019-02-26 20:47:26)
> From: Eric Biggers <ebiggers@google.com>
> 
> If drm_gem_handle_create() fails in vgem_gem_create(), then the
> drm_vgem_gem_object is freed twice: once when the reference is dropped
> by drm_gem_object_put_unlocked(), and again by __vgem_gem_destroy().
> 
> This was hit by syzkaller using fault injection.
> 
> Fix it by skipping the second free.
> 
> Reported-by: syzbot+e73f2fb5ed5a5df36d33@syzkaller.appspotmail.com
> Fixes: 5ba6c9ff961a ("drm/vgem: Fix mmaping")

That's the wrong fixes line, it's
Fixes: af33a9190d02 ("drm/vgem: Enable dmabuf import interfaces")
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Laura Abbott <labbott@redhat.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>

Sadly I reviewed it so I'm still culpable, but the fix is correct as the
put purposely frees it on error.

> Cc: stable@vger.kernel.org
> Signed-off-by: Eric Biggers <ebiggers@google.com>

> ---
>  drivers/gpu/drm/vgem/vgem_drv.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
> index 5930facd6d2d8..70646d9da1596 100644
> --- a/drivers/gpu/drm/vgem/vgem_drv.c
> +++ b/drivers/gpu/drm/vgem/vgem_drv.c
> @@ -189,15 +189,13 @@ static struct drm_gem_object *vgem_gem_create(struct drm_device *dev,
>                 return ERR_CAST(obj);
>  
>         ret = drm_gem_handle_create(file, &obj->base, handle);
> +
>         drm_gem_object_put_unlocked(&obj->base);
> +

The pattern in the other GEM drivers is not to have these extra
newlines.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

>         if (ret)
> -               goto err;
> +               return ERR_PTR(ret);
>  
>         return &obj->base;
> -
> -err:
> -       __vgem_gem_destroy(obj);
> -       return ERR_PTR(ret);
>  }
>  
>  static int vgem_gem_dumb_create(struct drm_file *file, struct drm_device *dev,
> -- 
> 2.21.0.rc2.261.ga7da99ff1b-goog
> 

Re: [PATCH] drm/vgem: fix use-after-free when drm_gem_handle_create() fails

[Eric Biggers]

On Tue, Feb 26, 2019 at 09:01:29PM +0000, Chris Wilson wrote:
> Quoting Eric Biggers (2019-02-26 20:47:26)
> > From: Eric Biggers <ebiggers@google.com>
> > 
> > If drm_gem_handle_create() fails in vgem_gem_create(), then the
> > drm_vgem_gem_object is freed twice: once when the reference is dropped
> > by drm_gem_object_put_unlocked(), and again by __vgem_gem_destroy().
> > 
> > This was hit by syzkaller using fault injection.
> > 
> > Fix it by skipping the second free.
> > 
> > Reported-by: syzbot+e73f2fb5ed5a5df36d33@syzkaller.appspotmail.com
> > Fixes: 5ba6c9ff961a ("drm/vgem: Fix mmaping")
> 
> That's the wrong fixes line, it's
> Fixes: af33a9190d02 ("drm/vgem: Enable dmabuf import interfaces")
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Laura Abbott <labbott@redhat.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> Sadly I reviewed it so I'm still culpable, but the fix is correct as the
> put purposely frees it on error.
> 

You're right; I misread the code at that commit.
I'll resend with the correct tags.

> > Cc: stable@vger.kernel.org
> > Signed-off-by: Eric Biggers <ebiggers@google.com>
> 
> > ---
> >  drivers/gpu/drm/vgem/vgem_drv.c | 8 +++-----
> >  1 file changed, 3 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
> > index 5930facd6d2d8..70646d9da1596 100644
> > --- a/drivers/gpu/drm/vgem/vgem_drv.c
> > +++ b/drivers/gpu/drm/vgem/vgem_drv.c
> > @@ -189,15 +189,13 @@ static struct drm_gem_object *vgem_gem_create(struct drm_device *dev,
> >                 return ERR_CAST(obj);
> >  
> >         ret = drm_gem_handle_create(file, &obj->base, handle);
> > +
> >         drm_gem_object_put_unlocked(&obj->base);
> > +
> 
> The pattern in the other GEM drivers is not to have these extra
> newlines.
> 
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> >         if (ret)
> > -               goto err;
> > +               return ERR_PTR(ret);
> >  
> >         return &obj->base;
> > -
> > -err:
> > -       __vgem_gem_destroy(obj);
> > -       return ERR_PTR(ret);
> >  }
> >  
> >  static int vgem_gem_dumb_create(struct drm_file *file, struct drm_device *dev,
> > -- 
> > 2.21.0.rc2.261.ga7da99ff1b-goog
> > 

[PATCH v2] drm/vgem: fix use-after-free when drm_gem_handle_create() fails

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

If drm_gem_handle_create() fails in vgem_gem_create(), then the
drm_vgem_gem_object is freed twice: once when the reference is dropped
by drm_gem_object_put_unlocked(), and again by __vgem_gem_destroy().

This was hit by syzkaller using fault injection.

Fix it by skipping the second free.

Reported-by: syzbot+e73f2fb5ed5a5df36d33@syzkaller.appspotmail.com
Fixes: af33a9190d02 ("drm/vgem: Enable dmabuf import interfaces")
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Laura Abbott <labbott@redhat.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: stable@vger.kernel.org
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 drivers/gpu/drm/vgem/vgem_drv.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
index 5930facd6d2d8..11a8f99ba18c5 100644
--- a/drivers/gpu/drm/vgem/vgem_drv.c
+++ b/drivers/gpu/drm/vgem/vgem_drv.c
@@ -191,13 +191,9 @@ static struct drm_gem_object *vgem_gem_create(struct drm_device *dev,
 	ret = drm_gem_handle_create(file, &obj->base, handle);
 	drm_gem_object_put_unlocked(&obj->base);
 	if (ret)
-		goto err;
+		return ERR_PTR(ret);
 
 	return &obj->base;
-
-err:
-	__vgem_gem_destroy(obj);
-	return ERR_PTR(ret);
 }
 
 static int vgem_gem_dumb_create(struct drm_file *file, struct drm_device *dev,
-- 
2.21.0.rc2.261.ga7da99ff1b-goog

Re: [PATCH v2] drm/vgem: fix use-after-free when drm_gem_handle_create() fails

[Laura Abbott]

On 2/26/19 1:44 PM, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> If drm_gem_handle_create() fails in vgem_gem_create(), then the
> drm_vgem_gem_object is freed twice: once when the reference is dropped
> by drm_gem_object_put_unlocked(), and again by __vgem_gem_destroy().
> 
> This was hit by syzkaller using fault injection.
> 
> Fix it by skipping the second free.
> 
> Reported-by: syzbot+e73f2fb5ed5a5df36d33@syzkaller.appspotmail.com
> Fixes: af33a9190d02 ("drm/vgem: Enable dmabuf import interfaces")
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Laura Abbott <labbott@redhat.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: stable@vger.kernel.org
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
>   drivers/gpu/drm/vgem/vgem_drv.c | 6 +-----
>   1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
> index 5930facd6d2d8..11a8f99ba18c5 100644
> --- a/drivers/gpu/drm/vgem/vgem_drv.c
> +++ b/drivers/gpu/drm/vgem/vgem_drv.c
> @@ -191,13 +191,9 @@ static struct drm_gem_object *vgem_gem_create(struct drm_device *dev,
>   	ret = drm_gem_handle_create(file, &obj->base, handle);
>   	drm_gem_object_put_unlocked(&obj->base);
>   	if (ret)
> -		goto err;
> +		return ERR_PTR(ret);
>   
>   	return &obj->base;
> -
> -err:
> -	__vgem_gem_destroy(obj);
> -	return ERR_PTR(ret);
>   }
>   
>   static int vgem_gem_dumb_create(struct drm_file *file, struct drm_device *dev,
> 


Acked-by: Laura Abbott <labbott@redhat.com>

Re: [PATCH v2] drm/vgem: fix use-after-free when drm_gem_handle_create() fails

[Rodrigo Siqueira]

[-- Attachment #1: Type: text/plain, Size: 1909 bytes --]

On 02/26, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> If drm_gem_handle_create() fails in vgem_gem_create(), then the
> drm_vgem_gem_object is freed twice: once when the reference is dropped
> by drm_gem_object_put_unlocked(), and again by __vgem_gem_destroy().
> 
> This was hit by syzkaller using fault injection.
> 
> Fix it by skipping the second free.
> 
> Reported-by: syzbot+e73f2fb5ed5a5df36d33@syzkaller.appspotmail.com
> Fixes: af33a9190d02 ("drm/vgem: Enable dmabuf import interfaces")
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Laura Abbott <labbott@redhat.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: stable@vger.kernel.org
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
>  drivers/gpu/drm/vgem/vgem_drv.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
> index 5930facd6d2d8..11a8f99ba18c5 100644
> --- a/drivers/gpu/drm/vgem/vgem_drv.c
> +++ b/drivers/gpu/drm/vgem/vgem_drv.c
> @@ -191,13 +191,9 @@ static struct drm_gem_object *vgem_gem_create(struct drm_device *dev,
>  	ret = drm_gem_handle_create(file, &obj->base, handle);
>  	drm_gem_object_put_unlocked(&obj->base);
>  	if (ret)
> -		goto err;
> +		return ERR_PTR(ret);
>  
>  	return &obj->base;
> -
> -err:
> -	__vgem_gem_destroy(obj);
> -	return ERR_PTR(ret);
>  }
>  
>  static int vgem_gem_dumb_create(struct drm_file *file, struct drm_device *dev,
> -- 
> 2.21.0.rc2.261.ga7da99ff1b-goog
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Applied to drm-misc-fixes.

Thanks

-- 
Rodrigo Siqueira
https://siqueira.tech
Graduate Student
Department of Computer Science
University of São Paulo

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH] drm/vkms: fix use-after-free when drm_gem_handle_create() fails

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

If drm_gem_handle_create() fails in vkms_gem_create(), then the
vkms_gem_object is freed twice: once when the reference is dropped by
drm_gem_object_put_unlocked(), and again by the extra calls to
drm_gem_object_release() and kfree().

Fix it by skipping the second release and free.

This bug was originally found in the vgem driver by syzkaller using
fault injection, but I noticed it's also present in the vkms driver.

Fixes: 559e50fd34d1 ("drm/vkms: Add dumb operations")
Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
Cc: Haneen Mohammed <hamohammed.sa@gmail.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: stable@vger.kernel.org
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 drivers/gpu/drm/vkms/vkms_gem.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_gem.c b/drivers/gpu/drm/vkms/vkms_gem.c
index 138b0bb325cf9..69048e73377dc 100644
--- a/drivers/gpu/drm/vkms/vkms_gem.c
+++ b/drivers/gpu/drm/vkms/vkms_gem.c
@@ -111,11 +111,8 @@ struct drm_gem_object *vkms_gem_create(struct drm_device *dev,
 
 	ret = drm_gem_handle_create(file, &obj->gem, handle);
 	drm_gem_object_put_unlocked(&obj->gem);
-	if (ret) {
-		drm_gem_object_release(&obj->gem);
-		kfree(obj);
+	if (ret)
 		return ERR_PTR(ret);
-	}
 
 	return &obj->gem;
 }
-- 
2.21.0.rc2.261.ga7da99ff1b-goog

Re: [PATCH] drm/vkms: fix use-after-free when drm_gem_handle_create() fails

[Chris Wilson]

Quoting Eric Biggers (2019-02-26 22:08:58)
> From: Eric Biggers <ebiggers@google.com>
> 
> If drm_gem_handle_create() fails in vkms_gem_create(), then the
> vkms_gem_object is freed twice: once when the reference is dropped by
> drm_gem_object_put_unlocked(), and again by the extra calls to
> drm_gem_object_release() and kfree().
> 
> Fix it by skipping the second release and free.
> 
> This bug was originally found in the vgem driver by syzkaller using
> fault injection, but I noticed it's also present in the vkms driver.
> 
> Fixes: 559e50fd34d1 ("drm/vkms: Add dumb operations")
> Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> Cc: Haneen Mohammed <hamohammed.sa@gmail.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: stable@vger.kernel.org
> Signed-off-by: Eric Biggers <ebiggers@google.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

Re: [PATCH] drm/vkms: fix use-after-free when drm_gem_handle_create() fails

[Rodrigo Siqueira]

[-- Attachment #1: Type: text/plain, Size: 1920 bytes --]

On 02/26, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> If drm_gem_handle_create() fails in vkms_gem_create(), then the
> vkms_gem_object is freed twice: once when the reference is dropped by
> drm_gem_object_put_unlocked(), and again by the extra calls to
> drm_gem_object_release() and kfree().
> 
> Fix it by skipping the second release and free.
> 
> This bug was originally found in the vgem driver by syzkaller using
> fault injection, but I noticed it's also present in the vkms driver.
> 
> Fixes: 559e50fd34d1 ("drm/vkms: Add dumb operations")
> Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> Cc: Haneen Mohammed <hamohammed.sa@gmail.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: stable@vger.kernel.org
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
>  drivers/gpu/drm/vkms/vkms_gem.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vkms/vkms_gem.c b/drivers/gpu/drm/vkms/vkms_gem.c
> index 138b0bb325cf9..69048e73377dc 100644
> --- a/drivers/gpu/drm/vkms/vkms_gem.c
> +++ b/drivers/gpu/drm/vkms/vkms_gem.c
> @@ -111,11 +111,8 @@ struct drm_gem_object *vkms_gem_create(struct drm_device *dev,
>  
>  	ret = drm_gem_handle_create(file, &obj->gem, handle);
>  	drm_gem_object_put_unlocked(&obj->gem);
> -	if (ret) {
> -		drm_gem_object_release(&obj->gem);
> -		kfree(obj);
> +	if (ret)
>  		return ERR_PTR(ret);
> -	}
>  
>  	return &obj->gem;
>  }
> -- 
> 2.21.0.rc2.261.ga7da99ff1b-goog
> 

Hi,

Thanks for your patch! :)

The patch looks good for me. I also tested it under the IGT tests on my
local VM and everything was fine.

Reviewed-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>

-- 
Rodrigo Siqueira
https://siqueira.tech
Graduate Student
Department of Computer Science
University of São Paulo

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] drm/vkms: fix use-after-free when drm_gem_handle_create() fails

[Dmitry Vyukov]

On Thu, Feb 28, 2019 at 12:12 AM Rodrigo Siqueira
<rodrigosiqueiramelo@gmail.com> wrote:
>
> On 02/26, Eric Biggers wrote:
> > From: Eric Biggers <ebiggers@google.com>
> >
> > If drm_gem_handle_create() fails in vkms_gem_create(), then the
> > vkms_gem_object is freed twice: once when the reference is dropped by
> > drm_gem_object_put_unlocked(), and again by the extra calls to
> > drm_gem_object_release() and kfree().
> >
> > Fix it by skipping the second release and free.
> >
> > This bug was originally found in the vgem driver by syzkaller using
> > fault injection, but I noticed it's also present in the vkms driver.
> >
> > Fixes: 559e50fd34d1 ("drm/vkms: Add dumb operations")
> > Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> > Cc: Haneen Mohammed <hamohammed.sa@gmail.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Eric Biggers <ebiggers@google.com>
> > ---
> >  drivers/gpu/drm/vkms/vkms_gem.c | 5 +----
> >  1 file changed, 1 insertion(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/vkms/vkms_gem.c b/drivers/gpu/drm/vkms/vkms_gem.c
> > index 138b0bb325cf9..69048e73377dc 100644
> > --- a/drivers/gpu/drm/vkms/vkms_gem.c
> > +++ b/drivers/gpu/drm/vkms/vkms_gem.c
> > @@ -111,11 +111,8 @@ struct drm_gem_object *vkms_gem_create(struct drm_device *dev,
> >
> >       ret = drm_gem_handle_create(file, &obj->gem, handle);
> >       drm_gem_object_put_unlocked(&obj->gem);
> > -     if (ret) {
> > -             drm_gem_object_release(&obj->gem);
> > -             kfree(obj);
> > +     if (ret)
> >               return ERR_PTR(ret);
> > -     }
> >
> >       return &obj->gem;
> >  }
> > --
> > 2.21.0.rc2.261.ga7da99ff1b-goog
> >
>
> Hi,
>
> Thanks for your patch! :)
>
> The patch looks good for me. I also tested it under the IGT tests on my
> local VM and everything was fine.

Hi Rodrigo,

What are IGT tests? How can I run them?

>
> Reviewed-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
>
> --
> Rodrigo Siqueira
> https://siqueira.tech
> Graduate Student
> Department of Computer Science
> University of São Paulo
>
> --
> You received this message because you are subscribed to the Google Groups "syzkaller-bugs" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller-bugs+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/20190227231202.tycdbcqtk5ylwp4k%40smtp.gmail.com.
> For more options, visit https://groups.google.com/d/optout.

Re: [PATCH] drm/vkms: fix use-after-free when drm_gem_handle_create() fails

[Rodrigo Siqueira]

[-- Attachment #1: Type: text/plain, Size: 3470 bytes --]

On 02/28, Dmitry Vyukov wrote:
> On Thu, Feb 28, 2019 at 12:12 AM Rodrigo Siqueira
> <rodrigosiqueiramelo@gmail.com> wrote:
> >
> > On 02/26, Eric Biggers wrote:
> > > From: Eric Biggers <ebiggers@google.com>
> > >
> > > If drm_gem_handle_create() fails in vkms_gem_create(), then the
> > > vkms_gem_object is freed twice: once when the reference is dropped by
> > > drm_gem_object_put_unlocked(), and again by the extra calls to
> > > drm_gem_object_release() and kfree().
> > >
> > > Fix it by skipping the second release and free.
> > >
> > > This bug was originally found in the vgem driver by syzkaller using
> > > fault injection, but I noticed it's also present in the vkms driver.
> > >
> > > Fixes: 559e50fd34d1 ("drm/vkms: Add dumb operations")
> > > Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> > > Cc: Haneen Mohammed <hamohammed.sa@gmail.com>
> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Eric Biggers <ebiggers@google.com>
> > > ---
> > >  drivers/gpu/drm/vkms/vkms_gem.c | 5 +----
> > >  1 file changed, 1 insertion(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/vkms/vkms_gem.c b/drivers/gpu/drm/vkms/vkms_gem.c
> > > index 138b0bb325cf9..69048e73377dc 100644
> > > --- a/drivers/gpu/drm/vkms/vkms_gem.c
> > > +++ b/drivers/gpu/drm/vkms/vkms_gem.c
> > > @@ -111,11 +111,8 @@ struct drm_gem_object *vkms_gem_create(struct drm_device *dev,
> > >
> > >       ret = drm_gem_handle_create(file, &obj->gem, handle);
> > >       drm_gem_object_put_unlocked(&obj->gem);
> > > -     if (ret) {
> > > -             drm_gem_object_release(&obj->gem);
> > > -             kfree(obj);
> > > +     if (ret)
> > >               return ERR_PTR(ret);
> > > -     }
> > >
> > >       return &obj->gem;
> > >  }
> > > --
> > > 2.21.0.rc2.261.ga7da99ff1b-goog
> > >
> >
> > Hi,
> >
> > Thanks for your patch! :)
> >
> > The patch looks good for me. I also tested it under the IGT tests on my
> > local VM and everything was fine.

Hi,

Patch applied to drm-misc-fixes.
 
> Hi Rodrigo,
> 
> What are IGT tests? How can I run them?

Hi Dmitry,

IGT is a test suite focused on DRM drivers.

You can clone the project using the link below:

  https://gitlab.freedesktop.org/drm/igt-gpu-tools.git

In the README, you will find the software dependencies. After you
install all the required package, just use:

 mkdir build && meson build && cd build && ninja

Finally, if you want to test VKMS, I recommend you to do it inside a VM.

Best Regards
Rodrigo Siqueira

> >
> > Reviewed-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> >
> > --
> > Rodrigo Siqueira
> > https://siqueira.tech
> > Graduate Student
> > Department of Computer Science
> > University of São Paulo
> >
> > --
> > You received this message because you are subscribed to the Google Groups "syzkaller-bugs" group.
> > To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller-bugs+unsubscribe@googlegroups.com.
> > To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/20190227231202.tycdbcqtk5ylwp4k%40smtp.gmail.com.
> > For more options, visit https://groups.google.com/d/optout.

-- 
Rodrigo Siqueira
https://siqueira.tech
Graduate Student
Department of Computer Science
University of São Paulo

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] drm/vkms: fix use-after-free when drm_gem_handle_create() fails

[Dmitry Vyukov]

On Tue, Mar 5, 2019 at 12:23 AM Rodrigo Siqueira
<rodrigosiqueiramelo@gmail.com> wrote:
>
> On 02/28, Dmitry Vyukov wrote:
> > On Thu, Feb 28, 2019 at 12:12 AM Rodrigo Siqueira
> > <rodrigosiqueiramelo@gmail.com> wrote:
> > >
> > > On 02/26, Eric Biggers wrote:
> > > > From: Eric Biggers <ebiggers@google.com>
> > > >
> > > > If drm_gem_handle_create() fails in vkms_gem_create(), then the
> > > > vkms_gem_object is freed twice: once when the reference is dropped by
> > > > drm_gem_object_put_unlocked(), and again by the extra calls to
> > > > drm_gem_object_release() and kfree().
> > > >
> > > > Fix it by skipping the second release and free.
> > > >
> > > > This bug was originally found in the vgem driver by syzkaller using
> > > > fault injection, but I noticed it's also present in the vkms driver.
> > > >
> > > > Fixes: 559e50fd34d1 ("drm/vkms: Add dumb operations")
> > > > Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> > > > Cc: Haneen Mohammed <hamohammed.sa@gmail.com>
> > > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > > Cc: stable@vger.kernel.org
> > > > Signed-off-by: Eric Biggers <ebiggers@google.com>
> > > > ---
> > > >  drivers/gpu/drm/vkms/vkms_gem.c | 5 +----
> > > >  1 file changed, 1 insertion(+), 4 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/vkms/vkms_gem.c b/drivers/gpu/drm/vkms/vkms_gem.c
> > > > index 138b0bb325cf9..69048e73377dc 100644
> > > > --- a/drivers/gpu/drm/vkms/vkms_gem.c
> > > > +++ b/drivers/gpu/drm/vkms/vkms_gem.c
> > > > @@ -111,11 +111,8 @@ struct drm_gem_object *vkms_gem_create(struct drm_device *dev,
> > > >
> > > >       ret = drm_gem_handle_create(file, &obj->gem, handle);
> > > >       drm_gem_object_put_unlocked(&obj->gem);
> > > > -     if (ret) {
> > > > -             drm_gem_object_release(&obj->gem);
> > > > -             kfree(obj);
> > > > +     if (ret)
> > > >               return ERR_PTR(ret);
> > > > -     }
> > > >
> > > >       return &obj->gem;
> > > >  }
> > > > --
> > > > 2.21.0.rc2.261.ga7da99ff1b-goog
> > > >
> > >
> > > Hi,
> > >
> > > Thanks for your patch! :)
> > >
> > > The patch looks good for me. I also tested it under the IGT tests on my
> > > local VM and everything was fine.
>
> Hi,
>
> Patch applied to drm-misc-fixes.
>
> > Hi Rodrigo,
> >
> > What are IGT tests? How can I run them?
>
> Hi Dmitry,
>
> IGT is a test suite focused on DRM drivers.
>
> You can clone the project using the link below:
>
>   https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
>
> In the README, you will find the software dependencies. After you
> install all the required package, just use:
>
>  mkdir build && meson build && cd build && ninja

Hi Rodrigo,

Thanks for the info, but this did not work for me.
I installed all recommended packages (including libdw-dev), but then got:

igt-gpu-tools$ mkdir -p build && meson build && cd build && ninja
The Meson build system
Version: 0.46.1
Source dir: /src/igt-gpu-tools
Build dir: /src/igt-gpu-tools/build
Build type: native build
Project name: igt-gpu-tools
Native C compiler: ccache cc (gcc 7.3.0 "cc (Debian 7.3.0-5) 7.3.0")
Build machine cpu family: x86_64
Build machine cpu: x86_64
Compiler for C supports arguments -Wbad-function-cast: YES
Compiler for C supports arguments -Wdeclaration-after-statement: YES
Compiler for C supports arguments -Wformat=2: YES
Compiler for C supports arguments -Wimplicit-fallthrough=0: YES
Compiler for C supports arguments -Wlogical-op: YES
Compiler for C supports arguments -Wmissing-declarations: YES
Compiler for C supports arguments -Wmissing-format-attribute: YES
Compiler for C supports arguments -Wmissing-noreturn: YES
Compiler for C supports arguments -Wmissing-prototypes: YES
Compiler for C supports arguments -Wnested-externs: YES
Compiler for C supports arguments -Wold-style-definition: YES
Compiler for C supports arguments -Wpointer-arith: YES
Compiler for C supports arguments -Wredundant-decls: YES
Compiler for C supports arguments -Wshadow: YES
Compiler for C supports arguments -Wstrict-prototypes: YES
Compiler for C supports arguments -Wuninitialized: YES
Compiler for C supports arguments -Wunused: YES
Compiler for C supports arguments -Wno-clobbered -Wclobbered: YES
Compiler for C supports arguments -Wno-maybe-uninitialized
-Wmaybe-uninitialized: YES
Compiler for C supports arguments -Wno-missing-field-initializers
-Wmissing-field-initializers: YES
Compiler for C supports arguments -Wno-pointer-arith -Wpointer-arith: YES
Compiler for C supports arguments -Wno-sign-compare -Wsign-compare: YES
Compiler for C supports arguments -Wno-type-limits -Wtype-limits: YES
Compiler for C supports arguments -Wno-unused-parameter -Wunused-parameter: YES
Compiler for C supports arguments -Wno-unused-result -Wunused-result: YES
Compiler for C supports arguments -Werror=address: YES
Compiler for C supports arguments -Werror=array-bounds: YES
Compiler for C supports arguments -Werror=implicit: YES
Compiler for C supports arguments -Werror=init-self: YES
Compiler for C supports arguments -Werror=int-to-pointer-cast: YES
Compiler for C supports arguments -Werror=main: YES
Compiler for C supports arguments -Werror=missing-braces: YES
Compiler for C supports arguments -Werror=nonnull: YES
Compiler for C supports arguments -Werror=pointer-to-int-cast: YES
Compiler for C supports arguments -Werror=return-type: YES
Compiler for C supports arguments -Werror=sequence-point: YES
Compiler for C supports arguments -Werror=trigraphs: YES
Compiler for C supports arguments -Werror=write-strings: YES
Found pkg-config: /usr/bin/pkg-config (0.29)
Native dependency libdrm found: YES 2.4.91
Native dependency libdrm_intel found: YES 2.4.91
Native dependency libdrm_nouveau found: YES 2.4.91
Native dependency libdrm_amdgpu found: YES 2.4.91
Native dependency pciaccess found: YES 0.13.4
Native dependency libkmod found: YES 24
Native dependency libprocps found: YES 3.3.15
Native dependency libunwind found: YES 1.21

meson.build:151:0: ERROR:  Could not generate cargs for libdw:

A full log can be found at /src/igt-gpu-tools/build/meson-logs/meson-log.txt


and meson-log.txt ends with:

Compiler for C supports arguments -Werror=write-strings: YES
Found pkg-config: /usr/bin/pkg-config (0.29)
Determining dependency 'libdrm' with pkg-config executable '/usr/bin/pkg-config'
Native dependency libdrm found: YES 2.4.91
Determining dependency 'libdrm_intel' with pkg-config executable
'/usr/bin/pkg-config'
Native dependency libdrm_intel found: YES 2.4.91
Determining dependency 'libdrm_nouveau' with pkg-config executable
'/usr/bin/pkg-config'
Native dependency libdrm_nouveau found: YES 2.4.91
Determining dependency 'libdrm_amdgpu' with pkg-config executable
'/usr/bin/pkg-config'
Native dependency libdrm_amdgpu found: YES 2.4.91
Determining dependency 'pciaccess' with pkg-config executable
'/usr/bin/pkg-config'
Native dependency pciaccess found: YES 0.13.4
Determining dependency 'libkmod' with pkg-config executable
'/usr/bin/pkg-config'
Native dependency libkmod found: YES 24
Determining dependency 'libprocps' with pkg-config executable
'/usr/bin/pkg-config'
Native dependency libprocps found: YES 3.3.15
Determining dependency 'libunwind' with pkg-config executable
'/usr/bin/pkg-config'
Native dependency libunwind found: YES 1.21
Determining dependency 'libdw' with pkg-config executable '/usr/bin/pkg-config'

meson.build:151:0: ERROR:  Could not generate cargs for libdw:

Re: [PATCH] drm/vkms: fix use-after-free when drm_gem_handle_create() fails

[Rodrigo Siqueira]

[-- Attachment #1: Type: text/plain, Size: 8357 bytes --]

On 03/05, Dmitry Vyukov wrote:
> On Tue, Mar 5, 2019 at 12:23 AM Rodrigo Siqueira
> <rodrigosiqueiramelo@gmail.com> wrote:
> >
> > On 02/28, Dmitry Vyukov wrote:
> > > On Thu, Feb 28, 2019 at 12:12 AM Rodrigo Siqueira
> > > <rodrigosiqueiramelo@gmail.com> wrote:
> > > >
> > > > On 02/26, Eric Biggers wrote:
> > > > > From: Eric Biggers <ebiggers@google.com>
> > > > >
> > > > > If drm_gem_handle_create() fails in vkms_gem_create(), then the
> > > > > vkms_gem_object is freed twice: once when the reference is dropped by
> > > > > drm_gem_object_put_unlocked(), and again by the extra calls to
> > > > > drm_gem_object_release() and kfree().
> > > > >
> > > > > Fix it by skipping the second release and free.
> > > > >
> > > > > This bug was originally found in the vgem driver by syzkaller using
> > > > > fault injection, but I noticed it's also present in the vkms driver.
> > > > >
> > > > > Fixes: 559e50fd34d1 ("drm/vkms: Add dumb operations")
> > > > > Cc: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> > > > > Cc: Haneen Mohammed <hamohammed.sa@gmail.com>
> > > > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > > > > Cc: stable@vger.kernel.org
> > > > > Signed-off-by: Eric Biggers <ebiggers@google.com>
> > > > > ---
> > > > >  drivers/gpu/drm/vkms/vkms_gem.c | 5 +----
> > > > >  1 file changed, 1 insertion(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/vkms/vkms_gem.c b/drivers/gpu/drm/vkms/vkms_gem.c
> > > > > index 138b0bb325cf9..69048e73377dc 100644
> > > > > --- a/drivers/gpu/drm/vkms/vkms_gem.c
> > > > > +++ b/drivers/gpu/drm/vkms/vkms_gem.c
> > > > > @@ -111,11 +111,8 @@ struct drm_gem_object *vkms_gem_create(struct drm_device *dev,
> > > > >
> > > > >       ret = drm_gem_handle_create(file, &obj->gem, handle);
> > > > >       drm_gem_object_put_unlocked(&obj->gem);
> > > > > -     if (ret) {
> > > > > -             drm_gem_object_release(&obj->gem);
> > > > > -             kfree(obj);
> > > > > +     if (ret)
> > > > >               return ERR_PTR(ret);
> > > > > -     }
> > > > >
> > > > >       return &obj->gem;
> > > > >  }
> > > > > --
> > > > > 2.21.0.rc2.261.ga7da99ff1b-goog
> > > > >
> > > >
> > > > Hi,
> > > >
> > > > Thanks for your patch! :)
> > > >
> > > > The patch looks good for me. I also tested it under the IGT tests on my
> > > > local VM and everything was fine.
> >
> > Hi,
> >
> > Patch applied to drm-misc-fixes.
> >
> > > Hi Rodrigo,
> > >
> > > What are IGT tests? How can I run them?
> >
> > Hi Dmitry,
> >
> > IGT is a test suite focused on DRM drivers.
> >
> > You can clone the project using the link below:
> >
> >   https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
> >
> > In the README, you will find the software dependencies. After you
> > install all the required package, just use:
> >
> >  mkdir build && meson build && cd build && ninja
> 
> Hi Rodrigo,
> 
> Thanks for the info, but this did not work for me.
> I installed all recommended packages (including libdw-dev), but then got:

Hi Dmitry,

I would like to recommend you to join the dri-devel channel (Freenode).
There you can quickly get help from me and others ;)

My nick in the dri-devel is 'siqueira'.

Best Regards
 
> igt-gpu-tools$ mkdir -p build && meson build && cd build && ninja
> The Meson build system
> Version: 0.46.1
> Source dir: /src/igt-gpu-tools
> Build dir: /src/igt-gpu-tools/build
> Build type: native build
> Project name: igt-gpu-tools
> Native C compiler: ccache cc (gcc 7.3.0 "cc (Debian 7.3.0-5) 7.3.0")
> Build machine cpu family: x86_64
> Build machine cpu: x86_64
> Compiler for C supports arguments -Wbad-function-cast: YES
> Compiler for C supports arguments -Wdeclaration-after-statement: YES
> Compiler for C supports arguments -Wformat=2: YES
> Compiler for C supports arguments -Wimplicit-fallthrough=0: YES
> Compiler for C supports arguments -Wlogical-op: YES
> Compiler for C supports arguments -Wmissing-declarations: YES
> Compiler for C supports arguments -Wmissing-format-attribute: YES
> Compiler for C supports arguments -Wmissing-noreturn: YES
> Compiler for C supports arguments -Wmissing-prototypes: YES
> Compiler for C supports arguments -Wnested-externs: YES
> Compiler for C supports arguments -Wold-style-definition: YES
> Compiler for C supports arguments -Wpointer-arith: YES
> Compiler for C supports arguments -Wredundant-decls: YES
> Compiler for C supports arguments -Wshadow: YES
> Compiler for C supports arguments -Wstrict-prototypes: YES
> Compiler for C supports arguments -Wuninitialized: YES
> Compiler for C supports arguments -Wunused: YES
> Compiler for C supports arguments -Wno-clobbered -Wclobbered: YES
> Compiler for C supports arguments -Wno-maybe-uninitialized
> -Wmaybe-uninitialized: YES
> Compiler for C supports arguments -Wno-missing-field-initializers
> -Wmissing-field-initializers: YES
> Compiler for C supports arguments -Wno-pointer-arith -Wpointer-arith: YES
> Compiler for C supports arguments -Wno-sign-compare -Wsign-compare: YES
> Compiler for C supports arguments -Wno-type-limits -Wtype-limits: YES
> Compiler for C supports arguments -Wno-unused-parameter -Wunused-parameter: YES
> Compiler for C supports arguments -Wno-unused-result -Wunused-result: YES
> Compiler for C supports arguments -Werror=address: YES
> Compiler for C supports arguments -Werror=array-bounds: YES
> Compiler for C supports arguments -Werror=implicit: YES
> Compiler for C supports arguments -Werror=init-self: YES
> Compiler for C supports arguments -Werror=int-to-pointer-cast: YES
> Compiler for C supports arguments -Werror=main: YES
> Compiler for C supports arguments -Werror=missing-braces: YES
> Compiler for C supports arguments -Werror=nonnull: YES
> Compiler for C supports arguments -Werror=pointer-to-int-cast: YES
> Compiler for C supports arguments -Werror=return-type: YES
> Compiler for C supports arguments -Werror=sequence-point: YES
> Compiler for C supports arguments -Werror=trigraphs: YES
> Compiler for C supports arguments -Werror=write-strings: YES
> Found pkg-config: /usr/bin/pkg-config (0.29)
> Native dependency libdrm found: YES 2.4.91
> Native dependency libdrm_intel found: YES 2.4.91
> Native dependency libdrm_nouveau found: YES 2.4.91
> Native dependency libdrm_amdgpu found: YES 2.4.91
> Native dependency pciaccess found: YES 0.13.4
> Native dependency libkmod found: YES 24
> Native dependency libprocps found: YES 3.3.15
> Native dependency libunwind found: YES 1.21
> 
> meson.build:151:0: ERROR:  Could not generate cargs for libdw:
> 
> A full log can be found at /src/igt-gpu-tools/build/meson-logs/meson-log.txt
> 
> 
> and meson-log.txt ends with:
> 
> Compiler for C supports arguments -Werror=write-strings: YES
> Found pkg-config: /usr/bin/pkg-config (0.29)
> Determining dependency 'libdrm' with pkg-config executable '/usr/bin/pkg-config'
> Native dependency libdrm found: YES 2.4.91
> Determining dependency 'libdrm_intel' with pkg-config executable
> '/usr/bin/pkg-config'
> Native dependency libdrm_intel found: YES 2.4.91
> Determining dependency 'libdrm_nouveau' with pkg-config executable
> '/usr/bin/pkg-config'
> Native dependency libdrm_nouveau found: YES 2.4.91
> Determining dependency 'libdrm_amdgpu' with pkg-config executable
> '/usr/bin/pkg-config'
> Native dependency libdrm_amdgpu found: YES 2.4.91
> Determining dependency 'pciaccess' with pkg-config executable
> '/usr/bin/pkg-config'
> Native dependency pciaccess found: YES 0.13.4
> Determining dependency 'libkmod' with pkg-config executable
> '/usr/bin/pkg-config'
> Native dependency libkmod found: YES 24
> Determining dependency 'libprocps' with pkg-config executable
> '/usr/bin/pkg-config'
> Native dependency libprocps found: YES 3.3.15
> Determining dependency 'libunwind' with pkg-config executable
> '/usr/bin/pkg-config'
> Native dependency libunwind found: YES 1.21
> Determining dependency 'libdw' with pkg-config executable '/usr/bin/pkg-config'
> 
> meson.build:151:0: ERROR:  Could not generate cargs for libdw:

-- 
Rodrigo Siqueira
https://siqueira.tech
Graduate Student
Department of Computer Science
University of São Paulo

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] drm/vgem: fix use-after-free when drm_gem_handle_create() fails

[Sasha Levin]

Hi,

[This is an automated email]

This commit has been processed because it contains a "Fixes:" tag,
fixing commit: 5ba6c9ff961a drm/vgem: Fix mmaping.

The bot has tested the following trees: v4.20.12, v4.19.25, v4.14.103, v4.9.160.

v4.20.12: Build OK!
v4.19.25: Build OK!
v4.14.103: Build OK!
v4.9.160: Failed to apply! Possible dependencies:
    024b6a63138c ("gpu: drm: gma500: Use vma_pages()")
    1a29d85eb0f1 ("mm: use vmf->address instead of of vmf->virtual_address")
    82b0f8c39a38 ("mm: join struct fault_env and vm_fault")
    953c66c2b22a ("mm: THP page cache support for ppc64")
    af33a9190d02 ("drm/vgem: Enable dmabuf import interfaces")
    c54fd47aa5b8 ("drm/vgem: switch to drm_*_get(), drm_*_put() helpers")
    fd60775aea80 ("mm, thp: avoid unlikely branches for split_huge_pmd")


How should we proceed with this patch?

--
Thanks,
Sasha
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel