linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Yoochan Lee <yoochan1026@gmail.com>
To: jens.wiklander@linaro.org
Cc: op-tee@lists.trustedfirmware.org, linux-kernel@vger.kernel.org,
	Yoochan Lee <yoochan1026@gmail.com>
Subject: [PATCH] tee: optee: smc_abi: Fix use-after-free in optee_smc_open
Date: Sat, 31 Dec 2022 14:19:54 +0900	[thread overview]
Message-ID: <20221231051954.2038772-1-yoochan1026@gmail.com> (raw)

A race condition may occur if the user physically removes the
smc_abi device while calling open().

This is a race condition between optee_smc_open() function and
the optee_smc_remove() function, which may lead to Use-After-Free.

Therefore, add a refcount check to optee_smc_remove() function
to free the "optee" structure after the device is close()d.

---------------CPU 0--------------------CPU 1-----------------
         optee_smc_open()     |     optee_smc_remove()
--------------------------------------------------------------
struct optee *optee = tee_get_|
drvdata(ctx->teedev); — (1)   |
                              | struct optee *optee = platform_
                              | get_drvdata(pdev);
                              | ...
                              | kfree(optee); — (2)
u32 sec_caps = optee->smc.sec_|
caps; — (3)

Signed-off-by: Yoochan Lee <yoochan1026@gmail.com>
---
 drivers/tee/optee/optee_private.h |  1 +
 drivers/tee/optee/smc_abi.c       | 66 ++++++++++++++++++++++---------
 2 files changed, 48 insertions(+), 19 deletions(-)

diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h
index 04ae58892608..f52b1cf20eab 100644
--- a/drivers/tee/optee/optee_private.h
+++ b/drivers/tee/optee/optee_private.h
@@ -175,6 +175,7 @@ struct optee {
 	bool   scan_bus_done;
 	struct workqueue_struct *scan_bus_wq;
 	struct work_struct scan_bus_work;
+	struct kref refcnt;
 };
 
 struct optee_session {
diff --git a/drivers/tee/optee/smc_abi.c b/drivers/tee/optee/smc_abi.c
index a1c1fa1a9c28..4fbec2acc255 100644
--- a/drivers/tee/optee/smc_abi.c
+++ b/drivers/tee/optee/smc_abi.c
@@ -1077,18 +1077,61 @@ static void optee_get_version(struct tee_device *teedev,
 	*vers = v;
 }
 
+static void optee_smc_delete(struct kref *kref)
+{
+	struct optee *optee = container_of(kref, struct optee, refcnt);
+
+	/*
+	 * Ask OP-TEE to free all cached shared memory objects to decrease
+	 * reference counters and also avoid wild pointers in secure world
+	 * into the old shared memory range.
+	 */
+	if (!optee->rpc_param_count)
+		optee_disable_shm_cache(optee);
+
+	optee_smc_notif_uninit_irq(optee);
+
+	optee_remove_common(optee);
+
+	if (optee->smc.memremaped_shm)
+		memunmap(optee->smc.memremaped_shm);
+
+	kfree(optee);
+}
+
+static void optee_smc_release_supp(struct tee_context *ctx)
+{
+	struct optee *optee = tee_get_drvdata(ctx->teedev);
+
+	optee_release_helper(ctx, optee_close_session_helper);
+	if (optee->scan_bus_wq) {
+		destroy_workqueue(optee->scan_bus_wq);
+		optee->scan_bus_wq = NULL;
+	}
+	optee_supp_release(&optee->supp);
+	kref_put(&optee->refcnt, optee_smc_delete);
+}
+
+static void optee_smc_release(struct tee_context *ctx)
+{
+	struct optee *optee = tee_get_drvdata(ctx->teedev);
+
+	optee_release_helper(ctx, optee_close_session_helper);
+	kref_put(&optee->refcnt, optee_smc_delete);
+}
+
 static int optee_smc_open(struct tee_context *ctx)
 {
 	struct optee *optee = tee_get_drvdata(ctx->teedev);
 	u32 sec_caps = optee->smc.sec_caps;
-
+	kref_get(&optee->refcnt);
 	return optee_open(ctx, sec_caps & OPTEE_SMC_SEC_CAP_MEMREF_NULL);
 }
 
 static const struct tee_driver_ops optee_clnt_ops = {
 	.get_version = optee_get_version,
 	.open = optee_smc_open,
-	.release = optee_release,
+	.release = optee_smc_release,
 	.open_session = optee_open_session,
 	.close_session = optee_close_session,
 	.invoke_func = optee_invoke_func,
@@ -1106,7 +1149,7 @@ static const struct tee_desc optee_clnt_desc = {
 static const struct tee_driver_ops optee_supp_ops = {
 	.get_version = optee_get_version,
 	.open = optee_smc_open,
-	.release = optee_release_supp,
+	.release = optee_smc_release_supp,
 	.supp_recv = optee_supp_recv,
 	.supp_send = optee_supp_send,
 	.shm_register = optee_shm_register_supp,
@@ -1319,22 +1362,7 @@ static int optee_smc_remove(struct platform_device *pdev)
 {
 	struct optee *optee = platform_get_drvdata(pdev);
 
-	/*
-	 * Ask OP-TEE to free all cached shared memory objects to decrease
-	 * reference counters and also avoid wild pointers in secure world
-	 * into the old shared memory range.
-	 */
-	if (!optee->rpc_param_count)
-		optee_disable_shm_cache(optee);
-
-	optee_smc_notif_uninit_irq(optee);
-
-	optee_remove_common(optee);
-
-	if (optee->smc.memremaped_shm)
-		memunmap(optee->smc.memremaped_shm);
-
-	kfree(optee);
+	kref_put(&optee->refcnt, optee_smc_delete);
 
 	return 0;
 }
-- 
2.39.0


             reply	other threads:[~2022-12-31  5:20 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-31  5:19 Yoochan Lee [this message]
2023-01-01 21:42 ` [PATCH] tee: optee: smc_abi: Fix use-after-free in optee_smc_open kernel test robot
2023-01-02  0:18   ` Yoochan Lee
2023-01-03  8:07     ` Jens Wiklander

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221231051954.2038772-1-yoochan1026@gmail.com \
    --to=yoochan1026@gmail.com \
    --cc=jens.wiklander@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=op-tee@lists.trustedfirmware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).