From abcc12b931db96410b69b7dc7a2c59f3733ab76d Mon Sep 17 00:00:00 2001
From: Sumukh Hallymysore Ravindra <shallymy@codeaurora.org>
Date: Mon, 13 Apr 2020 16:37:42 +0530
Subject: [PATCH] msm: synx: validate external callback

Validate the external callback to prevent acting
on delayed invocations, after the corresponding
handle has been released and reallocated by the
clients.
Change also avoids release of memory shared with
external callback if the callback de-registration
fails. This avoids double free issue if callback
has been queued up for dispatch during the cleanup.

Change-Id: Ib784c8a630a7c45ace946edab34e36ab1aec1869
Signed-off-by: Sumukh Hallymysore Ravindra <shallymy@codeaurora.org>
---
 drivers/media/platform/msm/synx/synx.c      | 46 ++++++++++++++++++++-
 drivers/media/platform/msm/synx/synx_util.c | 13 +++---
 2 files changed, 51 insertions(+), 8 deletions(-)

diff --git a/drivers/media/platform/msm/synx/synx.c b/drivers/media/platform/msm/synx/synx.c
index 7d742601ffad..4d75fbaf7e0a 100644
--- a/drivers/media/platform/msm/synx/synx.c
+++ b/drivers/media/platform/msm/synx/synx.c
@@ -16,6 +16,40 @@
 
 struct synx_device *synx_dev;
 
+static int synx_validate_callback(
+	struct synx_coredata *synx_obj,
+	s32 sync_id,
+	void *data)
+{
+	u32 i;
+	int rc = -EINVAL;
+	struct synx_bind_desc *bind_desc = NULL;
+
+	if (!synx_obj)
+		return -EINVAL;
+
+	/* need to validate the callback
+	 * as it could be dispatched and/or
+	 * scheduled late, after the handle
+	 * has been released and re-allocated.
+	 */
+	spin_lock_bh(&synx_obj->lock);
+	for (i = 0; i < synx_obj->num_bound_synxs; i++) {
+		bind_desc = &synx_obj->bound_synxs[i];
+		if ((sync_id ==
+			bind_desc->external_desc.id[0]) &&
+			(data == bind_desc->external_data)) {
+			rc = 0;
+			pr_debug("callback validation success %d\n",
+				sync_id);
+			break;
+		}
+	}
+	spin_unlock_bh(&synx_obj->lock);
+
+	return rc;
+}
+
 void synx_external_callback(s32 sync_obj, int status, void *data)
 {
 	struct synx_coredata *synx_obj;
@@ -29,8 +63,11 @@ void synx_external_callback(s32 sync_obj, int status, void *data)
 	}
 
 	client = synx_get_client(bind_data->session_id);
-	if (!client)
+	if (!client) {
+		pr_err("invalid payload content from sync external obj %d\n",
+			sync_obj);
 		goto free;
+	}
 
 	synx_obj = synx_util_acquire_object(client, bind_data->h_synx);
 	if (!synx_obj) {
@@ -39,12 +76,19 @@ void synx_external_callback(s32 sync_obj, int status, void *data)
 		goto fail;
 	}
 
+	if (synx_validate_callback(synx_obj, sync_obj, data)) {
+		pr_err("[sess: %u] stale callback from external obj %d handle %d\n",
+			client->id, sync_obj, bind_data->h_synx);
+		goto release;
+	}
+
 	pr_debug("[sess: %u] external callback from %d on handle %d\n",
 		client->id, sync_obj, bind_data->h_synx);
 	if (synx_signal_core(synx_obj, status, true, sync_obj))
 		pr_err("[sess: %u] signal callback failed for handle %d\n",
 			client->id, bind_data->h_synx);
 
+release:
 	synx_util_release_object(client, bind_data->h_synx);
 fail:
 	synx_put_client(client);
diff --git a/drivers/media/platform/msm/synx/synx_util.c b/drivers/media/platform/msm/synx/synx_util.c
index 2b532429086a..9ede611c7011 100644
--- a/drivers/media/platform/msm/synx/synx_util.c
+++ b/drivers/media/platform/msm/synx/synx_util.c
@@ -139,17 +139,16 @@ void synx_util_object_destroy(struct synx_coredata *synx_obj)
 		bind_ops = synx_util_get_bind_ops(type);
 		rc = bind_ops->deregister_callback(
 				synx_external_callback, data, sync_id);
-		if (rc < 0)
+		if (rc < 0) {
 			pr_err("de-registration fail id: %d, type: %u, err: %d\n",
 				sync_id, type, rc);
+			continue;
+		}
+
 		/*
 		 * release the memory allocated for external data.
-		 * It is safe to release this memory as external cb
-		 * has been already deregistered before this.
-		 * even if deregistration fails, it implies the
-		 * external fence might have been cleaned up already
-		 * or in some bad state. Have to release cb data
-		 * at this point or else will lead to memory leak.
+		 * It is safe to release this memory
+		 * only if deregistration is successful.
 		 */
 		kfree(data);
 	}