msm: kgsl: Fix race condition in snapshot sysfs read

If there are concurrent sysfs reads of snapshot binary
there can be a race condition where the snapshot data
is prematurely free'd by one reader while the other reader
is still reading it. Fix this by proper refcounting using
an atomic.

CRs-Fixed: 902816
Change-Id: I7a156c3a22f5475df0394ae30328d0fd6140f3da
Signed-off-by: Harshdeep Dhatt <hdhatt@codeaurora.org>

2 files changed, 27 insertions, 11 deletions

diff --git a/drivers/gpu/msm/kgsl_device.h b/drivers/gpu/msm/kgsl_device.h
index 706de527c444..abad774d481a 100644
--- a/drivers/gpu/msm/kgsl_device.h
+++ b/drivers/gpu/msm/kgsl_device.h
@@ -449,6 +449,7 @@ struct kgsl_device_private {
  * @work: worker to dump the frozen memory
  * @dump_gate: completion gate signaled by worker when it is finished.
  * @process: the process that caused the hang, if known.
+ * @sysfs_read: An atomic for concurrent snapshot reads via syfs.
  */
 struct kgsl_snapshot {
 	u8 *start;
@@ -463,6 +464,7 @@ struct kgsl_snapshot {
 	struct work_struct work;
 	struct completion dump_gate;
 	struct kgsl_process_private *process;
+	atomic_t sysfs_read;
 };
 
 /**
diff --git a/drivers/gpu/msm/kgsl_snapshot.c b/drivers/gpu/msm/kgsl_snapshot.c
index 8116ccaa96bd..0977d1477b6a 100644
--- a/drivers/gpu/msm/kgsl_snapshot.c
+++ b/drivers/gpu/msm/kgsl_snapshot.c
@@ -608,6 +608,7 @@ void kgsl_device_snapshot(struct kgsl_device *device,
 	snapshot->start = device->snapshot_memory.ptr;
 	snapshot->ptr = device->snapshot_memory.ptr;
 	snapshot->remain = device->snapshot_memory.size;
+	atomic_set(&snapshot->sysfs_read, 0);
 
 	header = (struct kgsl_snapshot_header *) snapshot->ptr;
 
@@ -711,6 +712,8 @@ static ssize_t snapshot_show(struct file *filep, struct kobject *kobj,
 
 	mutex_lock(&device->mutex);
 	snapshot = device->snapshot;
+	if (snapshot != NULL)
+		atomic_inc(&snapshot->sysfs_read);
 	mutex_unlock(&device->mutex);
 
 	/* Return nothing if we haven't taken a snapshot yet */
@@ -722,8 +725,10 @@ static ssize_t snapshot_show(struct file *filep, struct kobject *kobj,
 	 * to allow userspace to bail if things go horribly wrong.
 	 */
 	ret = wait_for_completion_interruptible(&snapshot->dump_gate);
-	if (ret)
+	if (ret) {
+		atomic_dec(&snapshot->sysfs_read);
 		return ret;
+	}
 
 	obj_itr_init(&itr, buf, off, count);
 
@@ -732,7 +737,7 @@ static ssize_t snapshot_show(struct file *filep, struct kobject *kobj,
 		goto done;
 
 	/* Dump the memory pool if it exists */
-	if (device->snapshot->mempool) {
+	if (snapshot->mempool) {
 		ret = obj_itr_out(&itr, snapshot->mempool,
 				snapshot->mempool_size);
 		if (ret == 0)
@@ -751,26 +756,35 @@ static ssize_t snapshot_show(struct file *filep, struct kobject *kobj,
 	 * Make sure everything has been written out before destroying things.
 	 * The best way to confirm this is to go all the way through without
 	 * writing any bytes - so only release if we get this far and
-	 * itr->write is 0
+	 * itr->write is 0 and there are no concurrent reads pending
 	 */
 
 	if (itr.write == 0) {
+		bool snapshot_free = false;
+
 		mutex_lock(&device->mutex);
-		device->snapshot = NULL;
+		if (atomic_dec_and_test(&snapshot->sysfs_read)) {
+			device->snapshot = NULL;
+			snapshot_free = true;
+		}
 		mutex_unlock(&device->mutex);
 
-		list_for_each_entry_safe(obj, tmp, &snapshot->obj_list, node)
-			kgsl_snapshot_put_object(obj);
+		if (snapshot_free) {
+			list_for_each_entry_safe(obj, tmp,
+						&snapshot->obj_list, node)
+				kgsl_snapshot_put_object(obj);
 
-		if (snapshot->mempool)
-			vfree(snapshot->mempool);
+			if (snapshot->mempool)
+				vfree(snapshot->mempool);
 
-		kfree(snapshot);
-		KGSL_CORE_ERR("snapshot: objects released\n");
+			kfree(snapshot);
+			KGSL_CORE_ERR("snapshot: objects released\n");
+		}
+		return 0;
 	}
 
 done:
-
+	atomic_dec(&snapshot->sysfs_read);
 	return itr.write;
 }