msm: kgsl: Fix race condition between drawobj and context destroy

drawobj_destroy_sync() 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 drawobj_destroy_sync() 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 <rajeshk@codeaurora.org>

drivers/gpu/msm/kgsl_drawobj.c

@@ -1,4 +1,4 @@
-/* Copyright (c) 2016-2017, The Linux Foundation. All rights reserved.
+/* Copyright (c) 2016-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
@@ -173,8 +173,11 @@ static void syncobj_timer(unsigned long data)
 /*
  * 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 drawobj_sync_expire(struct kgsl_device *device,
+static bool drawobj_sync_expire(struct kgsl_device *device,
 	struct kgsl_drawobj_sync_event *event)
 {
 	struct kgsl_drawobj_sync *syncobj = event->syncobj;
@@ -183,7 +186,7 @@ static void drawobj_sync_expire(struct kgsl_device *device,
 	 * leave without doing anything useful
 	 */
 	if (!test_and_clear_bit(event->id, &syncobj->pending))
-		return;
+		return false;
 
 	/*
 	 * If no more pending events, delete the timer and schedule the command
@@ -196,6 +199,7 @@ static void drawobj_sync_expire(struct kgsl_device *device,
 			device->ftbl->drawctxt_sched(device,
 				event->syncobj->base.context);
 	}
+	return true;
 }
 
 /*
@@ -210,8 +214,13 @@ static void drawobj_sync_func(struct kgsl_device *device,
 	trace_syncpoint_timestamp_expire(event->syncobj,
 		event->context, event->timestamp);
 
-	drawobj_sync_expire(device, event);
+	/*
+	 * Put down the context ref count only if
+	 * this thread successfully clears the pending bit mask.
+	 */
+	if (drawobj_sync_expire(device, event))
 		kgsl_context_put(event->context);
+
 	drawobj_put(&event->syncobj->base);
 }
 
@@ -241,19 +250,12 @@ static void drawobj_destroy_sparse(struct kgsl_drawobj *drawobj)
 static void drawobj_destroy_sync(struct kgsl_drawobj *drawobj)
 {
 	struct kgsl_drawobj_sync *syncobj = SYNCOBJ(drawobj);
-	unsigned long pending, flags;
+	unsigned long flags;
 	unsigned int i;
 
 	/* Zap the canary timer */
 	del_timer_sync(&syncobj->timer);
 
-	/*
-	 * Copy off the pending list and clear all pending events - this will
-	 * render any subsequent asynchronous callback harmless
-	 */
-	bitmap_copy(&pending, &syncobj->pending, KGSL_MAX_SYNCPOINTS);
-	bitmap_zero(&syncobj->pending, KGSL_MAX_SYNCPOINTS);
-
 	/*
 	 * Clear all pending events - this will render any subsequent async
 	 * callbacks harmless
@@ -261,8 +263,12 @@ static void drawobj_destroy_sync(struct kgsl_drawobj *drawobj)
 	for (i = 0; i < syncobj->numsyncs; i++) {
 		struct kgsl_drawobj_sync_event *event = &syncobj->synclist[i];
 
-		/* Don't do anything if the event has already expired */
-		if (!test_bit(i, &pending))
+		/*
+		 * Don't do anything if the event has already expired.
+		 * If this thread clears the pending bit mask then it is
+		 * responsible for doing context put.
+		 */
+		if (!test_and_clear_bit(i, &syncobj->pending))
 			continue;
 
 		switch (event->type) {
@@ -270,6 +276,11 @@ static void drawobj_destroy_sync(struct kgsl_drawobj *drawobj)
 			kgsl_cancel_event(drawobj->device,
 				&event->context->events, event->timestamp,
 				drawobj_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);
 			break;
 		case KGSL_CMD_SYNCPOINT_TYPE_FENCE:
 			spin_lock_irqsave(&event->handle_lock, flags);
@@ -291,7 +302,7 @@ static void drawobj_destroy_sync(struct kgsl_drawobj *drawobj)
 	 * If we cancelled an event, there's a good chance that the context is
 	 * on a dispatcher queue, so schedule to get it removed.
 	 */
-	if (!bitmap_empty(&pending, KGSL_MAX_SYNCPOINTS) &&
+	if (!bitmap_empty(&syncobj->pending, KGSL_MAX_SYNCPOINTS) &&
 		drawobj->device->ftbl->drawctxt_sched)
 		drawobj->device->ftbl->drawctxt_sched(drawobj->device,
 							drawobj->context);