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 <[email protected]>
---
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
Hi Yoochan,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v6.2-rc1 next-20221226]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Yoochan-Lee/tee-optee-smc_abi-Fix-use-after-free-in-optee_smc_open/20221231-132046
patch link: https://lore.kernel.org/r/20221231051954.2038772-1-yoochan1026%40gmail.com
patch subject: [PATCH] tee: optee: smc_abi: Fix use-after-free in optee_smc_open
config: arm-randconfig-c002-20221231
compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project f5700e7b69048de958172fb513b336564e7f8709)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install arm cross compiling tool for clang build
# apt-get install binutils-arm-linux-gnueabi
# https://github.com/intel-lab-lkp/linux/commit/6d3d5f78a2463a577c1a908cdedda61affba9c01
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Yoochan-Lee/tee-optee-smc_abi-Fix-use-after-free-in-optee_smc_open/20221231-132046
git checkout 6d3d5f78a2463a577c1a908cdedda61affba9c01
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash drivers/tee/optee/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <[email protected]>
All errors (new ones prefixed by >>):
>> drivers/tee/optee/smc_abi.c:1106:2: error: call to undeclared function 'optee_release_helper'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
optee_release_helper(ctx, optee_close_session_helper);
^
drivers/tee/optee/smc_abi.c:1106:2: note: did you mean 'optee_release_supp'?
drivers/tee/optee/optee_private.h:258:6: note: 'optee_release_supp' declared here
void optee_release_supp(struct tee_context *ctx);
^
drivers/tee/optee/smc_abi.c:1119:2: error: call to undeclared function 'optee_release_helper'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
optee_release_helper(ctx, optee_close_session_helper);
^
2 errors generated.
vim +/optee_release_helper +1106 drivers/tee/optee/smc_abi.c
1101
1102 static void optee_smc_release_supp(struct tee_context *ctx)
1103 {
1104 struct optee *optee = tee_get_drvdata(ctx->teedev);
1105
> 1106 optee_release_helper(ctx, optee_close_session_helper);
1107 if (optee->scan_bus_wq) {
1108 destroy_workqueue(optee->scan_bus_wq);
1109 optee->scan_bus_wq = NULL;
1110 }
1111 optee_supp_release(&optee->supp);
1112 kref_put(&optee->refcnt, optee_smc_delete);
1113 }
1114
--
0-DAY CI Kernel Test Service
https://01.org/lkp
I fix the errors in the previous patch.
From 2046cc19aeaddb5f6fb5e9b1d9a380a116892657 Mon Sep 17 00:00:00 2001
From: Yoochan Lee <[email protected]>
Date: Mon, 2 Jan 2023 09:18:23 +0900
Subject: [PATCH 2/2] fix errors in previous patch
---
drivers/tee/optee/smc_abi.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/tee/optee/smc_abi.c b/drivers/tee/optee/smc_abi.c
index 4fbec2acc255..be662f263f29 100644
--- a/drivers/tee/optee/smc_abi.c
+++ b/drivers/tee/optee/smc_abi.c
@@ -1103,12 +1103,7 @@ 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);
+ optee_release_supp(ctx)
kref_put(&optee->refcnt, optee_smc_delete);
}
@@ -1116,7 +1111,7 @@ 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);
+ optee_release(ctx);
kref_put(&optee->refcnt, optee_smc_delete);
}
--
2.39.0
2023년 1월 2일 (월) 오전 6:43, kernel test robot <[email protected]>님이 작성:
>
> Hi Yoochan,
>
> Thank you for the patch! Yet something to improve:
>
> [auto build test ERROR on linus/master]
> [also build test ERROR on v6.2-rc1 next-20221226]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Yoochan-Lee/tee-optee-smc_abi-Fix-use-after-free-in-optee_smc_open/20221231-132046
> patch link: https://lore.kernel.org/r/20221231051954.2038772-1-yoochan1026%40gmail.com
> patch subject: [PATCH] tee: optee: smc_abi: Fix use-after-free in optee_smc_open
> config: arm-randconfig-c002-20221231
> compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project f5700e7b69048de958172fb513b336564e7f8709)
> reproduce (this is a W=1 build):
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # install arm cross compiling tool for clang build
> # apt-get install binutils-arm-linux-gnueabi
> # https://github.com/intel-lab-lkp/linux/commit/6d3d5f78a2463a577c1a908cdedda61affba9c01
> git remote add linux-review https://github.com/intel-lab-lkp/linux
> git fetch --no-tags linux-review Yoochan-Lee/tee-optee-smc_abi-Fix-use-after-free-in-optee_smc_open/20221231-132046
> git checkout 6d3d5f78a2463a577c1a908cdedda61affba9c01
> # save the config file
> mkdir build_dir && cp config build_dir/.config
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm olddefconfig
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash drivers/tee/optee/
>
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <[email protected]>
>
> All errors (new ones prefixed by >>):
>
> >> drivers/tee/optee/smc_abi.c:1106:2: error: call to undeclared function 'optee_release_helper'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
> optee_release_helper(ctx, optee_close_session_helper);
> ^
> drivers/tee/optee/smc_abi.c:1106:2: note: did you mean 'optee_release_supp'?
> drivers/tee/optee/optee_private.h:258:6: note: 'optee_release_supp' declared here
> void optee_release_supp(struct tee_context *ctx);
> ^
> drivers/tee/optee/smc_abi.c:1119:2: error: call to undeclared function 'optee_release_helper'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
> optee_release_helper(ctx, optee_close_session_helper);
> ^
> 2 errors generated.
>
>
> vim +/optee_release_helper +1106 drivers/tee/optee/smc_abi.c
>
> 1101
> 1102 static void optee_smc_release_supp(struct tee_context *ctx)
> 1103 {
> 1104 struct optee *optee = tee_get_drvdata(ctx->teedev);
> 1105
> > 1106 optee_release_helper(ctx, optee_close_session_helper);
> 1107 if (optee->scan_bus_wq) {
> 1108 destroy_workqueue(optee->scan_bus_wq);
> 1109 optee->scan_bus_wq = NULL;
> 1110 }
> 1111 optee_supp_release(&optee->supp);
> 1112 kref_put(&optee->refcnt, optee_smc_delete);
> 1113 }
> 1114
>
> --
> 0-DAY CI Kernel Test Service
> https://01.org/lkp
Hi,
On Mon, Jan 2, 2023 at 1:18 AM Yoochan Lee <[email protected]> wrote:
>
> I fix the errors in the previous patch.
>
> From 2046cc19aeaddb5f6fb5e9b1d9a380a116892657 Mon Sep 17 00:00:00 2001
> From: Yoochan Lee <[email protected]>
> Date: Mon, 2 Jan 2023 09:18:23 +0900
> Subject: [PATCH 2/2] fix errors in previous patch
>
> ---
> drivers/tee/optee/smc_abi.c | 9 ++-------
> 1 file changed, 2 insertions(+), 7 deletions(-)
Please fix up the original patch and post it as a v2. By the way, have
you tried to test this in some way?
Thanks,
Jens
>
> diff --git a/drivers/tee/optee/smc_abi.c b/drivers/tee/optee/smc_abi.c
> index 4fbec2acc255..be662f263f29 100644
> --- a/drivers/tee/optee/smc_abi.c
> +++ b/drivers/tee/optee/smc_abi.c
> @@ -1103,12 +1103,7 @@ 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);
> + optee_release_supp(ctx)
> kref_put(&optee->refcnt, optee_smc_delete);
> }
>
> @@ -1116,7 +1111,7 @@ 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);
> + optee_release(ctx);
> kref_put(&optee->refcnt, optee_smc_delete);
> }
>
> --
> 2.39.0
>
> 2023년 1월 2일 (월) 오전 6:43, kernel test robot <[email protected]>님이 작성:
> >
> > Hi Yoochan,
> >
> > Thank you for the patch! Yet something to improve:
> >
> > [auto build test ERROR on linus/master]
> > [also build test ERROR on v6.2-rc1 next-20221226]
> > [If your patch is applied to the wrong git tree, kindly drop us a note.
> > And when submitting patch, we suggest to use '--base' as documented in
> > https://git-scm.com/docs/git-format-patch#_base_tree_information]
> >
> > url: https://github.com/intel-lab-lkp/linux/commits/Yoochan-Lee/tee-optee-smc_abi-Fix-use-after-free-in-optee_smc_open/20221231-132046
> > patch link: https://lore.kernel.org/r/20221231051954.2038772-1-yoochan1026%40gmail.com
> > patch subject: [PATCH] tee: optee: smc_abi: Fix use-after-free in optee_smc_open
> > config: arm-randconfig-c002-20221231
> > compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project f5700e7b69048de958172fb513b336564e7f8709)
> > reproduce (this is a W=1 build):
> > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> > chmod +x ~/bin/make.cross
> > # install arm cross compiling tool for clang build
> > # apt-get install binutils-arm-linux-gnueabi
> > # https://github.com/intel-lab-lkp/linux/commit/6d3d5f78a2463a577c1a908cdedda61affba9c01
> > git remote add linux-review https://github.com/intel-lab-lkp/linux
> > git fetch --no-tags linux-review Yoochan-Lee/tee-optee-smc_abi-Fix-use-after-free-in-optee_smc_open/20221231-132046
> > git checkout 6d3d5f78a2463a577c1a908cdedda61affba9c01
> > # save the config file
> > mkdir build_dir && cp config build_dir/.config
> > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm olddefconfig
> > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash drivers/tee/optee/
> >
> > If you fix the issue, kindly add following tag where applicable
> > | Reported-by: kernel test robot <[email protected]>
> >
> > All errors (new ones prefixed by >>):
> >
> > >> drivers/tee/optee/smc_abi.c:1106:2: error: call to undeclared function 'optee_release_helper'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
> > optee_release_helper(ctx, optee_close_session_helper);
> > ^
> > drivers/tee/optee/smc_abi.c:1106:2: note: did you mean 'optee_release_supp'?
> > drivers/tee/optee/optee_private.h:258:6: note: 'optee_release_supp' declared here
> > void optee_release_supp(struct tee_context *ctx);
> > ^
> > drivers/tee/optee/smc_abi.c:1119:2: error: call to undeclared function 'optee_release_helper'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
> > optee_release_helper(ctx, optee_close_session_helper);
> > ^
> > 2 errors generated.
> >
> >
> > vim +/optee_release_helper +1106 drivers/tee/optee/smc_abi.c
> >
> > 1101
> > 1102 static void optee_smc_release_supp(struct tee_context *ctx)
> > 1103 {
> > 1104 struct optee *optee = tee_get_drvdata(ctx->teedev);
> > 1105
> > > 1106 optee_release_helper(ctx, optee_close_session_helper);
> > 1107 if (optee->scan_bus_wq) {
> > 1108 destroy_workqueue(optee->scan_bus_wq);
> > 1109 optee->scan_bus_wq = NULL;
> > 1110 }
> > 1111 optee_supp_release(&optee->supp);
> > 1112 kref_put(&optee->refcnt, optee_smc_delete);
> > 1113 }
> > 1114
> >
> > --
> > 0-DAY CI Kernel Test Service
> > https://01.org/lkp