msm: kgsl: Avoid race condition in ioctl_syncsource_destroyandroid-lego-7.1.1_r0.10

If the ioctl syncsource_destroy is accessed by parallel
threads, where the spinlock is acquired by threads after
getting syncsource, then the simultaneous processes try
to remove the already destroyed syncsource->refcount by
the first thread that acquires this spinlock. This leads
to race condition while removing syncsource->idr.

Avoid separate lock inside getting syncsource, instead
acquire spinlock before we get the syncsource in
destroy ioctl so that the threads access the spinlock
and operate on syncsource without use-after-free issue.

Change-Id: I6add3800c40cd09f6e6e0cf2720e69059bd83cbc
Signed-off-by: Divya Ponnusamy <pdivya@codeaurora.org>

1 files changed, 17 insertions, 19 deletions

diff --git a/drivers/gpu/msm/kgsl_sync.c b/drivers/gpu/msm/kgsl_sync.c
index b80b699d3b3a..eac5b5d448fc 100644
--- a/drivers/gpu/msm/kgsl_sync.c
+++ b/drivers/gpu/msm/kgsl_sync.c
@@ -1,4 +1,4 @@
-/* Copyright (c) 2012-2015, The Linux Foundation. All rights reserved.
+/* Copyright (c) 2012-2016, 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
@@ -464,23 +464,23 @@ long kgsl_ioctl_syncsource_create(struct kgsl_device_private *dev_priv,
 		goto out;
 	}
 
+	kref_init(&syncsource->refcount);
+	syncsource->private = private;
+
 	idr_preload(GFP_KERNEL);
 	spin_lock(&private->syncsource_lock);
 	id = idr_alloc(&private->syncsource_idr, syncsource, 1, 0, GFP_NOWAIT);
-	spin_unlock(&private->syncsource_lock);
-	idr_preload_end();
-
 	if (id > 0) {
-		kref_init(&syncsource->refcount);
 		syncsource->id = id;
-		syncsource->private = private;
-
 		param->id = id;
 		ret = 0;
 	} else {
 		ret = id;
 	}
 
+	spin_unlock(&private->syncsource_lock);
+	idr_preload_end();
+
 out:
 	if (ret) {
 		if (syncsource && syncsource->oneshot)
@@ -538,25 +538,23 @@ long kgsl_ioctl_syncsource_destroy(struct kgsl_device_private *dev_priv,
 {
 	struct kgsl_syncsource_destroy *param = data;
 	struct kgsl_syncsource *syncsource = NULL;
-	struct kgsl_process_private *private;
-
-	syncsource = kgsl_syncsource_get(dev_priv->process_priv,
-				     param->id);
+	struct kgsl_process_private *private = dev_priv->process_priv;
 
-	if (syncsource == NULL)
-		return -EINVAL;
+	spin_lock(&private->syncsource_lock);
+	syncsource = idr_find(&private->syncsource_idr, param->id);
 
-	private = syncsource->private;
+	if (syncsource) {
+		idr_remove(&private->syncsource_idr, param->id);
+		syncsource->id = 0;
+	}
 
-	spin_lock(&private->syncsource_lock);
-	idr_remove(&private->syncsource_idr, param->id);
-	syncsource->id = 0;
 	spin_unlock(&private->syncsource_lock);
 
+	if (syncsource == NULL)
+		return -EINVAL;
+
 	/* put reference from syncsource creation */
 	kgsl_syncsource_put(syncsource);
-	/* put reference from getting the syncsource above */
-	kgsl_syncsource_put(syncsource);
 	return 0;
 }