Re: "Revisit iommu_insert_resv_region() implementation" causes use-after-free

[Auger Eric]

Hi,

On 11/22/19 4:06 PM, Qian Cai wrote:
> Read files under /sys/kernel/iommu_groups/ triggers an use-after-free. Reverted
> the commit 4dbd258ff63e ("iommu: Revisit iommu_insert_resv_region()
> implementation") fixed the issue.

Thanks for testing and reporting this.

I've worked on this during the WE but until now I am not able to
reproduce myself or find the issue by review. Since the beginning I have
been testing the algo with a char device driver, injecting various
regions to insert and test whether they are correctly collected and merged.

About the test itself did you try to read the file at specific moments
or with specific concurrency constraints?

About KASAN, did you use the INLINE or OUTLINE config, if it were to
change anything?
> 
> /* no merge needed on elements of different types than @nr */
> if (iter->type != nr->type) {
> 	list_move_tail(&iter->list, &stack);
I guess this is the place where the use of dangling pointer happens?
> 	continue;>
> [ÂÂ160.156964][ T3100] BUG: KASAN: use-after-free in
> iommu_insert_resv_region+0x34b/0x520
> [ÂÂ160.197758][ T3100] Read of size 4 at addr ffff8887aba78464 by task cat/3100
> [ÂÂ160.230645][ T3100]Â
> [ÂÂ160.240907][ T3100] CPU: 14 PID: 3100 Comm: cat Not tainted 5.4.0-rc8-next-
> 20191122+ #11
> [ÂÂ160.278671][ T3100] Hardware name: HP ProLiant XL420 Gen9/ProLiant XL420
> Gen9, BIOS U19 12/27/2015
> [ÂÂ160.320589][ T3100] Call Trace:
> [ÂÂ160.335229][ T3100]ÂÂdump_stack+0xa0/0xea
> [ÂÂ160.354011][ T3100]ÂÂprint_address_description.constprop.5.cold.7+0x9/0x384
> [ÂÂ160.386569][ T3100]ÂÂ__kasan_report.cold.8+0x7a/0xc0
> [ÂÂ160.409811][ T3100]ÂÂ? iommu_insert_resv_region+0x34b/0x520
> [ÂÂ160.435668][ T3100]ÂÂkasan_report+0x12/0x20
> [ÂÂ160.455387][ T3100]ÂÂ__asan_load4+0x95/0xa0
> [ÂÂ160.474808][ T3100]ÂÂiommu_insert_resv_region+0x34b/0x520
> [ÂÂ160.500228][ T3100]ÂÂ? iommu_bus_notifier+0xe0/0xe0
> [ÂÂ160.522904][ T3100]ÂÂ? intel_iommu_get_resv_regions+0x348/0x400
> [ÂÂ160.550461][ T3100]ÂÂiommu_get_group_resv_regions+0x16d/0x2f0
> [ÂÂ160.577611][ T3100]ÂÂ? iommu_insert_resv_region+0x520/0x520
> [ÂÂ160.603756][ T3100]ÂÂ? register_lock_class+0x940/0x940
> [ÂÂ160.628265][ T3100]ÂÂiommu_group_show_resv_regions+0x8d/0x1f0
> [ÂÂ160.655370][ T3100]ÂÂ? iommu_get_group_resv_regions+0x2f0/0x2f0
> [ÂÂ160.684168][ T3100]ÂÂiommu_group_attr_show+0x34/0x50
> [ÂÂ160.708395][ T3100]ÂÂsysfs_kf_seq_show+0x11c/0x220
> [ÂÂ160.731758][ T3100]ÂÂ? iommu_default_passthrough+0x20/0x20
> [ÂÂ160.756898][ T3100]ÂÂkernfs_seq_show+0xa4/0xb0
> [ÂÂ160.777097][ T3100]ÂÂseq_read+0x27e/0x710
> [ÂÂ160.795195][ T3100]ÂÂkernfs_fop_read+0x7d/0x2c0
> [ÂÂ160.815349][ T3100]ÂÂ__vfs_read+0x50/0xa0
> [ÂÂ160.834154][ T3100]ÂÂvfs_read+0xcb/0x1e0
> [ÂÂ160.852332][ T3100]ÂÂksys_read+0xc6/0x160
> [ÂÂ160.871028][ T3100]ÂÂ? kernel_write+0xc0/0xc0
> [ÂÂ160.891307][ T3100]ÂÂ? do_syscall_64+0x79/0xaec
> [ÂÂ160.912446][ T3100]ÂÂ? do_syscall_64+0x79/0xaec
> [ÂÂ160.933640][ T3100]ÂÂ__x64_sys_read+0x43/0x50
> [ÂÂ160.953957][ T3100]ÂÂdo_syscall_64+0xcc/0xaec
> [ÂÂ160.974322][ T3100]ÂÂ? trace_hardirqs_on_thunk+0x1a/0x1c
> [ÂÂ160.999130][ T3100]ÂÂ? syscall_return_slowpath+0x580/0x580
> [ÂÂ161.024753][ T3100]ÂÂ? entry_SYSCALL_64_after_hwframe+0x3e/0xbe
> [ÂÂ161.052416][ T3100]ÂÂ? trace_hardirqs_off_caller+0x3a/0x150
> [ÂÂ161.078400][ T3100]ÂÂ? trace_hardirqs_off_thunk+0x1a/0x1c
> [ÂÂ161.103711][ T3100]ÂÂentry_SYSCALL_64_after_hwframe+0x49/0xbe
> [ÂÂ161.130793][ T3100] RIP: 0033:0x7f33e0d89d75
> [ÂÂ161.150732][ T3100] Code: fe ff ff 50 48 8d 3d 4a dc 09 00 e8 25 0e 02 00 0f
> 1f 44 00 00 f3 0f 1e fa 48 8d 05 a5 59 2d 00 8b 00 85 c0 75 0f 31 c0 0f 05 <48>
> 3d 00 f0 ff ff 77 53 c3 66 90 41 54 49 89 d4 55 48 89 f5 53 89
> [ÂÂ161.245503][ T3100] RSP: 002b:00007fff88f0db88 EFLAGS: 00000246 ORIG_RAX:
> 0000000000000000
> [ÂÂ161.284547][ T3100] RAX: ffffffffffffffda RBX: 0000000000020000 RCX:
> 00007f33e0d89d75
> [ÂÂ161.321123][ T3100] RDX: 0000000000020000 RSI: 00007f33e1201000 RDI:
> 0000000000000003
> [ÂÂ161.357617][ T3100] RBP: 00007f33e1201000 R08: 00000000ffffffff R09:
> 0000000000000000
> [ÂÂ161.394173][ T3100] R10: 0000000000000022 R11: 0000000000000246 R12:
> 00007f33e1201000
> [ÂÂ161.430736][ T3100] R13: 0000000000000003 R14: 0000000000000fff R15:
> 0000000000020000
> [ÂÂ161.467337][ T3100]Â
> [ÂÂ161.477529][ T3100] Allocated by task 3100:
> [ÂÂ161.497133][ T3100]ÂÂsave_stack+0x21/0x90
> [ÂÂ161.515777][ T3100]ÂÂ__kasan_kmalloc.constprop.13+0xc1/0xd0
> [ÂÂ161.541743][ T3100]ÂÂkasan_kmalloc+0x9/0x10
> [ÂÂ161.561330][ T3100]ÂÂkmem_cache_alloc_trace+0x1f8/0x470
> [ÂÂ161.585949][ T3100]ÂÂiommu_insert_resv_region+0xeb/0x520
> [ÂÂ161.610876][ T3100]ÂÂiommu_get_group_resv_regions+0x16d/0x2f0
> [ÂÂ161.638318][ T3100]ÂÂiommu_group_show_resv_regions+0x8d/0x1f0
> [ÂÂ161.665322][ T3100]ÂÂiommu_group_attr_show+0x34/0x50
> [ÂÂ161.688526][ T3100]ÂÂsysfs_kf_seq_show+0x11c/0x220
> [ÂÂ161.711992][ T3100]ÂÂkernfs_seq_show+0xa4/0xb0
> [ÂÂ161.734252][ T3100]ÂÂseq_read+0x27e/0x710
> [ÂÂ161.754412][ T3100]ÂÂkernfs_fop_read+0x7d/0x2c0
> [ÂÂ161.775493][ T3100]ÂÂ__vfs_read+0x50/0xa0
> [ÂÂ161.794328][ T3100]ÂÂvfs_read+0xcb/0x1e0
> [ÂÂ161.812559][ T3100]ÂÂksys_read+0xc6/0x160
> [ÂÂ161.831554][ T3100]ÂÂ__x64_sys_read+0x43/0x50
> [ÂÂ161.851772][ T3100]ÂÂdo_syscall_64+0xcc/0xaec
> [ÂÂ161.872098][ T3100]ÂÂentry_SYSCALL_64_after_hwframe+0x49/0xbe
> [ÂÂ161.898919][ T3100]Â
> [ÂÂ161.909113][ T3100] Freed by task 3100:
> [ÂÂ161.927070][ T3100]ÂÂsave_stack+0x21/0x90
> [ÂÂ161.945711][ T3100]ÂÂ__kasan_slab_free+0x11c/0x170
> [ÂÂ161.968112][ T3100]ÂÂkasan_slab_free+0xe/0x10
> [ÂÂ161.988601][ T3100]ÂÂslab_free_freelist_hook+0x5f/0x1d0
> [ÂÂ162.012918][ T3100]ÂÂkfree+0xe9/0x410
Do I understand correctly that the use after free happens in the same
execution of iommu_insert_resv_region and kfree is done in the
check_overlap part?

> [ÂÂ162.029454][ T3100]ÂÂiommu_insert_resv_region+0x47d/0x520
> [ÂÂ162.053701][ T3100]ÂÂiommu_get_group_resv_regions+0x16d/0x2f0
> [ÂÂ162.079671][ T3100]ÂÂiommu_group_show_resv_regions+0x8d/0x1f0
> [ÂÂ162.105484][ T3100]ÂÂiommu_group_attr_show+0x34/0x50
> [ÂÂ162.127709][ T3100]ÂÂsysfs_kf_seq_show+0x11c/0x220
> [ÂÂ162.149250][ T3100]ÂÂkernfs_seq_show+0xa4/0xb0
> [ÂÂ162.169085][ T3100]ÂÂseq_read+0x27e/0x710
> [ÂÂ162.187038][ T3100]ÂÂkernfs_fop_read+0x7d/0x2c0
> [ÂÂ162.207391][ T3100]ÂÂ__vfs_read+0x50/0xa0
> [ÂÂ162.227829][ T3100]ÂÂvfs_read+0xcb/0x1e0
> [ÂÂ162.247788][ T3100]ÂÂksys_read+0xc6/0x160
> [ÂÂ162.265471][ T3100]ÂÂ__x64_sys_read+0x43/0x50
> [ÂÂ162.285041][ T3100]ÂÂdo_syscall_64+0xcc/0xaec
> [ÂÂ162.304627][ T3100]ÂÂentry_SYSCALL_64_after_hwframe+0x49/0xbe
> [ÂÂ162.330429][ T3100]Â
> [ÂÂ162.340199][ T3100] The buggy address belongs to the object at
> ffff8887aba78440
> [ÂÂ162.340199][ T3100]ÂÂwhich belongs to the cache kmalloc-64 of size 64
> [ÂÂ162.402050][ T3100] The buggy address is located 36 bytes inside of
> [ÂÂ162.402050][ T3100]ÂÂ64-byte region [ffff8887aba78440, ffff8887aba78480)
> [ÂÂ162.460127][ T3100] The buggy address belongs to the page:
> [ÂÂ162.484696][ T3100] page:ffffea001eae9e00 refcount:1 mapcount:0
> mapping:ffff888207c02ac0 index:0xffff8887aba78e40
> [ÂÂ162.531045][ T3100] raw: 015fffe000000200 ffff888487c00740 ffff888487c00740
> ffff888207c02ac0
> [ÂÂ162.569455][ T3100] raw: ffff8887aba78e40 0000000000080003 00000001ffffffff
> 0000000000000000
> [ÂÂ162.607801][ T3100] page dumped because: kasan: bad access detected
> [ÂÂ162.636603][ T3100] page_owner tracks the page as allocated
> [ÂÂ162.661634][ T3100] page last allocated via order 0, migratetype Unmovable,
> gfp_mask 0x12cc0(GFP_KERNEL|__GFP_NOWARN|__GFP_NORETRY)
> [ÂÂ162.716310][ T3100]ÂÂprep_new_page+0x2ed/0x310
> [ÂÂ162.739158][ T3100]ÂÂget_page_from_freelist+0x20bb/0x3090
> [ÂÂ162.765017][ T3100]ÂÂ__alloc_pages_nodemask+0x2e4/0x720
> [ÂÂ162.788440][ T3100]ÂÂalloc_pages_current+0x9c/0x110
> [ÂÂ162.810324][ T3100]ÂÂalloc_slab_page+0xc9/0x4e0
> [ÂÂ162.831044][ T3100]ÂÂallocate_slab+0x70/0x5d0
> [ÂÂ162.851450][ T3100]ÂÂnew_slab+0x46/0x70
> [ÂÂ162.869326][ T3100]ÂÂ___slab_alloc+0x4ab/0x7b0
> [ÂÂ162.889554][ T3100]ÂÂ__slab_alloc+0x43/0x70
> [ÂÂ162.908430][ T3100]ÂÂkmem_cache_alloc_trace+0x2f1/0x470
> [ÂÂ162.932036][ T3100]ÂÂiommu_insert_resv_region+0xeb/0x520
> [ÂÂ162.956179][ T3100]ÂÂiommu_get_group_resv_regions+0x16d/0x2f0
> [ÂÂ162.982083][ T3100]ÂÂiommu_group_show_resv_regions+0x8d/0x1f0
> [ÂÂ163.008373][ T3100]ÂÂiommu_group_attr_show+0x34/0x50
> [ÂÂ163.030557][ T3100]ÂÂsysfs_kf_seq_show+0x11c/0x220
> [ÂÂ163.052038][ T3100]ÂÂkernfs_seq_show+0xa4/0xb0
> [ÂÂ163.072179][ T3100] page last free stack trace:
> [ÂÂ163.092612][ T3100]ÂÂ__free_pages_ok+0xa3e/0xb20
> [ÂÂ163.113361][ T3100]ÂÂ__free_pages+0x94/0xd0
> [ÂÂ163.132198][ T3100]ÂÂ__free_slab+0x177/0x520
> [ÂÂ163.150922][ T3100]ÂÂdiscard_slab+0x41/0x80
> [ÂÂ163.169708][ T3100]ÂÂ__slab_free+0x4b7/0x520
> [ÂÂ163.188856][ T3100]ÂÂ___cache_free+0xc3/0x120
> [ÂÂ163.208452][ T3100]ÂÂqlist_free_all+0x44/0xa0
> [ÂÂ163.228335][ T3100]ÂÂquarantine_reduce+0x1b0/0x240
> [ÂÂ163.253120][ T3100]ÂÂ__kasan_kmalloc.constprop.13+0x98/0xd0
> [ÂÂ163.279865][ T3100]ÂÂkasan_slab_alloc+0x11/0x20
> [ÂÂ163.300175][ T3100]ÂÂkmem_cache_alloc+0x17a/0x450
> [ÂÂ163.321373][ T3100]ÂÂptlock_alloc+0x20/0x50
> [ÂÂ163.340168][ T3100]ÂÂpte_alloc_one+0x40/0xf0
> [ÂÂ163.359310][ T3100]ÂÂ__handle_mm_fault+0x1257/0x1300
> [ÂÂ163.381603][ T3100]ÂÂhandle_mm_fault+0x205/0x4c0
> [ÂÂ163.402312][ T3100]ÂÂ__do_page_fault+0x29c/0x640
> [ÂÂ163.423082][ T3100]Â
> [ÂÂ163.432556][ T3100] Memory state around the buggy address:
> [ÂÂ163.457292][ T3100]ÂÂffff8887aba78300: fc fc fc fc fc fc fc fc fc fc fc fc fc
> fc fc fc
> [ÂÂ163.492994][ T3100]ÂÂffff8887aba78380: fc fc fc fc fc fc fc fc fc fc fc fc fc
> fc fc fc
> [ÂÂ163.528530][ T3100] >ffff8887aba78400: fc fc fc fc fc fc fc fc fb fb fb fb fb
> fb fb fb
> [ÂÂ163.565023][ T3100]ÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂ^
> [ÂÂ163.598027][ T3100]ÂÂffff8887aba78480: fc fc fc fc fc fc fc fc fc fc fc fc fc
> fc fc fc
> [ÂÂ163.633747][ T3100]ÂÂffff8887aba78500: fc fc fc fc fc fc fc fc fc fc fc fc fc
> fc fc fc
> [ÂÂ163.669828][ T3100]
> ==================================================================
> [ÂÂ163.705417][ T3100] Disabling lock debugging due to kernel taint
> 
Thanks

Eric