On 9/18/23 18:53, José Pekkarinen wrote:
> Kasan reported the following in my system:
>
> [ 3935.321003]
> ==================================================================
> [ 3935.321022] BUG: KASAN: slab-use-after-free in
> drm_atomic_helper_wait_for_vblanks.part.0+0x116/0x450 [drm_kms_helper]
> [ 3935.321124] Read of size 1 at addr ffff88818a6f8009 by task
> kworker/u16:3/5268
>
> [ 3935.321124] CPU: 7 PID: 5268 Comm: kworker/u16:3 Not tainted 6.6.0-rc2+ #1
> [ 3935.321124] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 0.0.0
> 02/06/2015
> [ 3935.321124] Workqueue: events_unbound commit_work [drm_kms_helper]
> [ 3935.321124] Call Trace:
> [ 3935.321124] <TASK>
> [ 3935.321124] dump_stack_lvl+0x43/0x60
> [ 3935.321124] print_report+0xcf/0x660
> [ 3935.321124] ? remove_entity_load_avg+0xdc/0x100
> [ 3935.321124] ? __virt_addr_valid+0xd9/0x160
> [ 3935.321124] ? drm_atomic_helper_wait_for_vblanks.part.0+0x116/0x450
> [drm_kms_helper]
> [ 3935.321124] kasan_report+0xda/0x110
> [ 3935.321124] ? drm_atomic_helper_wait_for_vblanks.part.0+0x116/0x450
> [drm_kms_helper]
> [ 3935.321124] drm_atomic_helper_wait_for_vblanks.part.0+0x116/0x450
> [drm_kms_helper]
> [ 3935.321124] ? __pfx_drm_atomic_helper_wait_for_vblanks.part.0+0x10/0x10
> [drm_kms_helper]
> [ 3935.321124] ? complete_all+0x48/0x100
> [ 3935.321124] ? _raw_spin_unlock_irqrestore+0x19/0x40
> [ 3935.321124] ? preempt_count_sub+0x14/0xc0
> [ 3935.321124] ? _raw_spin_unlock_irqrestore+0x23/0x40
> [ 3935.321124] ? drm_atomic_helper_commit_hw_done+0x1ac/0x240
> [drm_kms_helper]
> [ 3935.321124] drm_atomic_helper_commit_tail+0x82/0x90 [drm_kms_helper]
> [ 3935.321124] commit_tail+0x15c/0x1d0 [drm_kms_helper]
> [ 3935.323185] process_one_work+0x31a/0x610
> [ 3935.323185] worker_thread+0x38e/0x5f0
> [ 3935.323185] ? __pfx_worker_thread+0x10/0x10
> [ 3935.323185] kthread+0x184/0x1c0
> [ 3935.323185] ? __pfx_kthread+0x10/0x10
> [ 3935.323185] ret_from_fork+0x30/0x50
> [ 3935.323185] ? __pfx_kthread+0x10/0x10
> [ 3935.323185] ret_from_fork_asm+0x1b/0x30
> [ 3935.323185] </TASK>
>
> [ 3935.323185] Allocated by task 3751:
> [ 3935.323185] kasan_save_stack+0x2f/0x50
> [ 3935.323185] kasan_set_track+0x21/0x30
> [ 3935.323185] __kasan_kmalloc+0xa6/0xb0
> [ 3935.323185] drm_atomic_helper_crtc_duplicate_state+0x42/0x70
> [drm_kms_helper]
> [ 3935.323185] drm_atomic_get_crtc_state+0xc3/0x1e0 [drm]
> [ 3935.323185] page_flip_common+0x42/0x160 [drm_kms_helper]
> [ 3935.323185] drm_atomic_helper_page_flip+0x6b/0xf0 [drm_kms_helper]
> [ 3935.323185] drm_mode_page_flip_ioctl+0x8ad/0x900 [drm]
> [ 3935.323185] drm_ioctl_kernel+0x169/0x240 [drm]
> [ 3935.323185] drm_ioctl+0x399/0x6b0 [drm]
> [ 3935.324772] __x64_sys_ioctl+0xc5/0x100
> [ 3935.324772] do_syscall_64+0x5b/0xc0
> [ 3935.324772] entry_SYSCALL_64_after_hwframe+0x6e/0xd8
>
> [ 3935.324772] Freed by task 3751:
> [ 3935.324772] kasan_save_stack+0x2f/0x50
> [ 3935.324772] kasan_set_track+0x21/0x30
> [ 3935.324772] kasan_save_free_info+0x27/0x40
> [ 3935.324772] ____kasan_slab_free+0x166/0x1c0
> [ 3935.324772] slab_free_freelist_hook+0x9f/0x1e0
> [ 3935.324772] __kmem_cache_free+0x187/0x2d0
> [ 3935.324772] drm_atomic_state_default_clear+0x226/0x5e0 [drm]
> [ 3935.324772] __drm_atomic_state_free+0xc8/0x130 [drm]
> [ 3935.324772] drm_atomic_helper_update_plane+0x17d/0x1b0 [drm_kms_helper]
> [ 3935.324772] drm_mode_cursor_universal+0x2a4/0x4d0 [drm]
> [ 3935.324772] drm_mode_cursor_common+0x1cf/0x430 [drm]
> [ 3935.324772] drm_mode_cursor_ioctl+0xc6/0x100 [drm]
> [ 3935.326167] drm_ioctl_kernel+0x169/0x240 [drm]
> [ 3935.326167] drm_ioctl+0x399/0x6b0 [drm]
> [ 3935.326614] __x64_sys_ioctl+0xc5/0x100
> [ 3935.326614] do_syscall_64+0x5b/0xc0
> [ 3935.326614] entry_SYSCALL_64_after_hwframe+0x6e/0xd8
>
> [ 3935.326614] The buggy address belongs to the object at ffff88818a6f8000
> which belongs to the cache kmalloc-512 of size 512
> [ 3935.326614] The buggy address is located 9 bytes inside of
> freed 512-byte region [ffff88818a6f8000, ffff88818a6f8200)
>
> [ 3935.326614] The buggy address belongs to the physical page:
> [ 3935.326614] page:00000000b0fb0816 refcount:1 mapcount:0
> mapping:0000000000000000 index:0x0 pfn:0x18a6f8
> [ 3935.326614] head:00000000b0fb0816 order:3 entire_mapcount:0
> nr_pages_mapped:0 pincount:0
> [ 3935.326614] anon flags:
> 0x17ffffc0000840(slab|head|node=0|zone=2|lastcpupid=0x1fffff)
> [ 3935.326614] page_type: 0xffffffff()
> [ 3935.326614] raw: 0017ffffc0000840 ffff888100042c80 0000000000000000
> dead000000000001
> [ 3935.326614] raw: 0000000000000000 0000000080200020 00000001ffffffff
> 0000000000000000
> [ 3935.326614] page dumped because: kasan: bad access detected
>
> [ 3935.326614] Memory state around the buggy address:
> [ 3935.326614] ffff88818a6f7f00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> fc fc
> [ 3935.326614] ffff88818a6f7f80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> fc fc
> [ 3935.326614] >ffff88818a6f8000: fa fb fb fb fb fb fb fb fb fb fb fb fb fb
> fb fb
> [ 3935.326772] ^
> [ 3935.326772] ffff88818a6f8080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> fb fb
> [ 3935.326772] ffff88818a6f8100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> fb fb
> [ 3935.326772]
> ==================================================================
>
> This suggest there may be some situation where a
> struct drm_crtc_state is referenced after already
> being freed by drm_atomic_state_default_clear. This
> patch will check the new_crtc_state is not null before
> using it.
>
> Signed-off-by: José Pekkarinen <jose.pekkari...@foxhound.fi>
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 292e38eb6218..cc75d387a542 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1647,7 +1647,7 @@ drm_atomic_helper_wait_for_vblanks(struct drm_device
> *dev,
> return;
>
> for_each_oldnew_crtc_in_state(old_state, crtc, old_crtc_state,
> new_crtc_state, i) {
> - if (!new_crtc_state->active)
> + if (new_crtc_state && !new_crtc_state->active)
> continue;
>
> ret = drm_crtc_vblank_get(crtc);
I'm not quite seeing the connection between this change and the KASAN report.
If new_crtc_state was NULL, I would have expected a normal NULL pointer
dereference oops, not a KASAN report.
The KASAN report instead indicates that new_crtc_state isn't NULL, but points
to memory which has already been freed. This could be e.g. due to incorrect
reference counting somewhere else.
--
Earthling Michel Dänzer | https://redhat.com
Libre software enthusiast | Mesa and Xwayland developer
