From eed7bf427cdcacd7443c9f886b76217b8032b237 Mon Sep 17 00:00:00 2001 From: Rajesh Kemisetti Date: Tue, 20 Aug 2019 14:18:55 +0530 Subject: [PATCH] msm: kgsl: Fix race condition between cmdbatch and context destroy kgsl_cmdbatch_destroy() tries to cancel all pending sync events by taking local copy of pending list. In case of sync point timestamp event, it goes ahead and accesses context's events list assuming that event's context would be alive. But at the same time, if the other context, which is of interest for these sync point events, can be destroyed by cancelling all events in its group. This leads to use-after-free in kgsl_cmdbatch_destroy() path. Fix is to give the responsibility of putting the context's ref count to the thread which clears the pending mask. Change-Id: I8d08ef6ddb38ca917f75088071c04727bced11d2 Signed-off-by: Rajesh Kemisetti Signed-off-by: Archana Sriram --- drivers/gpu/msm/kgsl_cmdbatch.c | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/msm/kgsl_cmdbatch.c b/drivers/gpu/msm/kgsl_cmdbatch.c index 065686a2a490..4f772514ee67 100644 --- a/drivers/gpu/msm/kgsl_cmdbatch.c +++ b/drivers/gpu/msm/kgsl_cmdbatch.c @@ -1,4 +1,4 @@ -/* Copyright (c) 2008-2017, The Linux Foundation. All rights reserved. +/* Copyright (c) 2008-2017,2019, The Linux Foundation. All rights reserved. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 and @@ -180,8 +180,12 @@ EXPORT_SYMBOL(kgsl_cmdbatch_destroy_object); /* * a generic function to retire a pending sync event and (possibly) * kick the dispatcher + * Returns false if the event was already marked for cancellation in another + * thread. This function should return true if this thread is responsible for + * freeing up the memory, and the event will not be cancelled. */ -static void kgsl_cmdbatch_sync_expire(struct kgsl_device *device, + +static bool kgsl_cmdbatch_sync_expire(struct kgsl_device *device, struct kgsl_cmdbatch_sync_event *event) { struct kgsl_cmdbatch_sync_event *e, *tmp; @@ -208,6 +212,11 @@ static void kgsl_cmdbatch_sync_expire(struct kgsl_device *device, } } + if (removed == 0) { + spin_unlock_irqrestore(&event->cmdbatch->lock, flags); + return false; + } + event->handle = NULL; sched = list_empty(&event->cmdbatch->synclist) ? 1 : 0; spin_unlock_irqrestore(&event->cmdbatch->lock, flags); @@ -227,6 +236,8 @@ static void kgsl_cmdbatch_sync_expire(struct kgsl_device *device, /* Put events that have been removed from the synclist */ if (removed) kgsl_cmdbatch_sync_event_put(event); + + return true; } /* @@ -241,8 +252,13 @@ static void kgsl_cmdbatch_sync_func(struct kgsl_device *device, trace_syncpoint_timestamp_expire(event->cmdbatch, event->context, event->timestamp); - kgsl_cmdbatch_sync_expire(device, event); - kgsl_context_put(event->context); + /* + * Put down the context ref count only if + * this thread successfully clears the pending bit mask. + */ + if (kgsl_cmdbatch_sync_expire(device, event)) + kgsl_context_put(event->context); + /* Put events that have signaled */ kgsl_cmdbatch_sync_event_put(event); } @@ -306,6 +322,12 @@ void kgsl_cmdbatch_destroy(struct kgsl_cmdbatch *cmdbatch) kgsl_cancel_event(cmdbatch->device, &event->context->events, event->timestamp, kgsl_cmdbatch_sync_func, event); + /* + * Do context put here to make sure the context is alive + * till this thread cancels kgsl event. + */ + kgsl_context_put(event->context); + } else if (event->type == KGSL_CMD_SYNCPOINT_TYPE_FENCE) { /* Put events that are successfully canceled */ spin_lock_irqsave(&cmdbatch->lock, flags);