[Intel-gfx] [PATCH] drm/atomic: Fix double kfree on crtc_state->event
Daniel Vetter
daniel.vetter at ffwll.ch
Mon Jan 30 17:53:39 UTC 2017
This is a bug Maarten reported, with the following slab debug backtrace:
[IGT] kms_rotation_crc: starting subtest primary-rotation-180
=============================================================================
BUG kmalloc-128 (Tainted: G U ): Object already free
-----------------------------------------------------------------------------
Disabling lock debugging due to kernel taint
INFO: Allocated in drm_atomic_helper_setup_commit+0x285/0x2f0 [drm_kms_helper] age=0 cpu=3 pid=1529
___slab_alloc+0x308/0x3b0
__slab_alloc+0xd/0x20
kmem_cache_alloc_trace+0x92/0x1c0
drm_atomic_helper_setup_commit+0x285/0x2f0 [drm_kms_helper]
intel_atomic_commit+0x35/0x4f0 [i915]
drm_atomic_commit+0x46/0x50 [drm]
drm_mode_atomic_ioctl+0x7d4/0xab0 [drm]
drm_ioctl+0x2b3/0x490 [drm]
do_vfs_ioctl+0x69c/0x700
SyS_ioctl+0x4e/0x80
entry_SYSCALL_64_fastpath+0x13/0x94
INFO: Freed in drm_event_cancel_free+0xa3/0xb0 [drm] age=0 cpu=3 pid=1529
__slab_free+0x48/0x2e0
kfree+0x159/0x1a0
drm_event_cancel_free+0xa3/0xb0 [drm]
drm_mode_atomic_ioctl+0x86d/0xab0 [drm]
drm_ioctl+0x2b3/0x490 [drm]
do_vfs_ioctl+0x69c/0x700
SyS_ioctl+0x4e/0x80
entry_SYSCALL_64_fastpath+0x13/0x94
INFO: Slab 0xffffde1f0997b080 objects=17 used=2 fp=0xffff92fb65ec2578 flags=0x200000000008101
INFO: Object 0xffff92fb65ec2578 @offset=1400 fp=0xffff92fb65ec2ae8
Redzone ffff92fb65ec2570: bb bb bb bb bb bb bb bb ........
Object ffff92fb65ec2578: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ffff92fb65ec2588: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ffff92fb65ec2598: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ffff92fb65ec25a8: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ffff92fb65ec25b8: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ffff92fb65ec25c8: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ffff92fb65ec25d8: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b kkkkkkkkkkkkkkkk
Object ffff92fb65ec25e8: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b a5 kkkkkkkkkkkkkkk.
Redzone ffff92fb65ec25f8: bb bb bb bb bb bb bb bb ........
Padding ffff92fb65ec2738: 5a 5a 5a 5a 5a 5a 5a 5a ZZZZZZZZ
CPU: 3 PID: 180 Comm: kworker/3:2 Tainted: G BU 4.10.0-rc6-patser+ #5039
Hardware name: /NUC5PPYB, BIOS PYBSWCEL.86A.0031.2015.0601.1712 06/01/2015
Workqueue: events intel_atomic_helper_free_state [i915]
Call Trace:
dump_stack+0x4d/0x6d
print_trailer+0x20c/0x220
free_debug_processing+0x1c6/0x330
? drm_atomic_state_default_clear+0xf7/0x1c0 [drm]
__slab_free+0x48/0x2e0
? drm_atomic_state_default_clear+0xf7/0x1c0 [drm]
kfree+0x159/0x1a0
drm_atomic_state_default_clear+0xf7/0x1c0 [drm]
? drm_atomic_state_clear+0x30/0x30 [drm]
intel_atomic_state_clear+0xd/0x20 [i915]
drm_atomic_state_clear+0x1a/0x30 [drm]
__drm_atomic_state_free+0x13/0x60 [drm]
intel_atomic_helper_free_state+0x5d/0x70 [i915]
process_one_work+0x260/0x4a0
worker_thread+0x2d1/0x4f0
kthread+0x127/0x130
? process_one_work+0x4a0/0x4a0
? kthread_stop+0x120/0x120
ret_from_fork+0x29/0x40
FIX kmalloc-128: Object at 0xffff92fb65ec2578 not freed
Reported-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
---
drivers/gpu/drm/drm_atomic.c | 24 ++++++++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 026be94a7d15..366b4bf88206 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -159,15 +159,29 @@ void drm_atomic_state_default_clear(struct drm_atomic_state *state)
if (!crtc)
continue;
- crtc->funcs->atomic_destroy_state(crtc,
- state->crtcs[i].state);
-
if (state->crtcs[i].commit) {
- kfree(state->crtcs[i].commit->event);
+ /*
+ * We need to make sure we don't double-free, which we
+ * do by checking for state->event, implicitly since
+ * kfree can handle a NULL state->event. We also need to
+ * make sure we only kfree the event if it's one created
+ * for internal commit tracking (and hence won't be
+ * cleared by the caller, like the atomic IOCTL or a
+ * legacy pageflip. This is done by checking
+ * commit->event.
+ *
+ * This only works if everyone else sets state->event to
+ * NULL when they take it away.
+ */
+ if (state->crtcs[i].commit->event)
+ kfree(state->crtcs[i].commit->event);
state->crtcs[i].commit->event = NULL;
drm_crtc_commit_put(state->crtcs[i].commit);
}
+ crtc->funcs->atomic_destroy_state(crtc,
+ state->crtcs[i].state);
+
state->crtcs[i].commit = NULL;
state->crtcs[i].ptr = NULL;
state->crtcs[i].state = NULL;
@@ -2034,6 +2048,8 @@ static void complete_crtc_signaling(struct drm_device *dev,
*/
if (crtc_state->event)
drm_event_cancel_free(dev, &crtc_state->event->base);
+
+ crtc_state->event = NULL;
}
if (!fence_state)
--
2.11.0
More information about the Intel-gfx
mailing list