From: Jens Wiklander <jens.wiklander@linaro.org>
To: linux-integrity@vger.kernel.org, keyrings@vger.kernel.org,
linux-security-module@vger.kernel.org,
linux-kernel@vger.kernel.org, op-tee@lists.trustedfirmware.org
Cc: Sumit Garg <sumit.garg@linaro.org>,
James Bottomley <jejb@linux.ibm.com>,
Jarkko Sakkinen <jarkko@kernel.org>,
Mimi Zohar <zohar@linux.ibm.com>,
Jens Wiklander <jens.wiklander@linaro.org>
Subject: [PATCH] KEYS: trusted: tee: use tee_shm_register_alloc_buf()
Date: Thu, 3 Aug 2023 14:35:15 +0200 [thread overview]
Message-ID: <20230803123515.4018838-1-jens.wiklander@linaro.org> (raw)
Prior to this patch was trusted_tee_seal() and trusted_tee_get_random()
relying on tee_shm_register_kernel_buf() to share memory with the TEE.
Depending on the memory allocation pattern the pages holding the
registered buffers overlap with other buffers also shared with the TEE.
The OP-TEE driver using the old SMC based ABI permits overlapping shared
buffers, but with the new FF-A based ABI each physical page may only
be registered once.
Fix this problem by allocating a temporary page aligned shared memory
buffer to be used as a bounce buffer for the needed data buffers.
Since TEE trusted keys doesn't depend on registered shared memory
support any longer remove that explicit dependency when opening a
context to the TEE.
Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
---
security/keys/trusted-keys/trusted_tee.c | 68 +++++++++++++-----------
1 file changed, 36 insertions(+), 32 deletions(-)
diff --git a/security/keys/trusted-keys/trusted_tee.c b/security/keys/trusted-keys/trusted_tee.c
index ac3e270ade69..3085343c489a 100644
--- a/security/keys/trusted-keys/trusted_tee.c
+++ b/security/keys/trusted-keys/trusted_tee.c
@@ -8,6 +8,7 @@
#include <linux/err.h>
#include <linux/key-type.h>
+#include <linux/minmax.h>
#include <linux/module.h>
#include <linux/slab.h>
#include <linux/string.h>
@@ -65,38 +66,37 @@ static int trusted_tee_seal(struct trusted_key_payload *p, char *datablob)
int ret;
struct tee_ioctl_invoke_arg inv_arg;
struct tee_param param[4];
- struct tee_shm *reg_shm_in = NULL, *reg_shm_out = NULL;
+ struct tee_shm *shm;
+ uint8_t *buf;
memset(&inv_arg, 0, sizeof(inv_arg));
memset(¶m, 0, sizeof(param));
- reg_shm_in = tee_shm_register_kernel_buf(pvt_data.ctx, p->key,
- p->key_len);
- if (IS_ERR(reg_shm_in)) {
- dev_err(pvt_data.dev, "key shm register failed\n");
- return PTR_ERR(reg_shm_in);
+ shm = tee_shm_alloc_kernel_buf(pvt_data.ctx,
+ p->key_len + sizeof(p->blob));
+ if (IS_ERR(shm)) {
+ dev_err(pvt_data.dev, "key shm alloc failed\n");
+ return PTR_ERR(shm);
}
-
- reg_shm_out = tee_shm_register_kernel_buf(pvt_data.ctx, p->blob,
- sizeof(p->blob));
- if (IS_ERR(reg_shm_out)) {
- dev_err(pvt_data.dev, "blob shm register failed\n");
- ret = PTR_ERR(reg_shm_out);
+ buf = tee_shm_get_va(shm, 0);
+ if (IS_ERR(buf)) {
+ ret = PTR_ERR(buf);
goto out;
}
+ memcpy(buf, p->key, p->key_len);
inv_arg.func = TA_CMD_SEAL;
inv_arg.session = pvt_data.session_id;
inv_arg.num_params = 4;
param[0].attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT;
- param[0].u.memref.shm = reg_shm_in;
+ param[0].u.memref.shm = shm;
param[0].u.memref.size = p->key_len;
param[0].u.memref.shm_offs = 0;
param[1].attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT;
- param[1].u.memref.shm = reg_shm_out;
+ param[1].u.memref.shm = shm;
param[1].u.memref.size = sizeof(p->blob);
- param[1].u.memref.shm_offs = 0;
+ param[1].u.memref.shm_offs = p->key_len;
ret = tee_client_invoke_func(pvt_data.ctx, &inv_arg, param);
if ((ret < 0) || (inv_arg.ret != 0)) {
@@ -104,14 +104,13 @@ static int trusted_tee_seal(struct trusted_key_payload *p, char *datablob)
inv_arg.ret);
ret = -EFAULT;
} else {
+ memcpy(p->blob, buf + p->key_len,
+ min(param[1].u.memref.size, sizeof(p->blob)));
p->blob_len = param[1].u.memref.size;
}
out:
- if (reg_shm_out)
- tee_shm_free(reg_shm_out);
- if (reg_shm_in)
- tee_shm_free(reg_shm_in);
+ tee_shm_free(shm);
return ret;
}
@@ -166,11 +165,9 @@ static int trusted_tee_unseal(struct trusted_key_payload *p, char *datablob)
p->key_len = param[1].u.memref.size;
}
+ tee_shm_free(reg_shm_out);
out:
- if (reg_shm_out)
- tee_shm_free(reg_shm_out);
- if (reg_shm_in)
- tee_shm_free(reg_shm_in);
+ tee_shm_free(reg_shm_in);
return ret;
}
@@ -183,15 +180,21 @@ static int trusted_tee_get_random(unsigned char *key, size_t key_len)
int ret;
struct tee_ioctl_invoke_arg inv_arg;
struct tee_param param[4];
- struct tee_shm *reg_shm = NULL;
+ struct tee_shm *shm;
+ void *buf;
memset(&inv_arg, 0, sizeof(inv_arg));
memset(¶m, 0, sizeof(param));
- reg_shm = tee_shm_register_kernel_buf(pvt_data.ctx, key, key_len);
- if (IS_ERR(reg_shm)) {
- dev_err(pvt_data.dev, "key shm register failed\n");
- return PTR_ERR(reg_shm);
+ shm = tee_shm_alloc_kernel_buf(pvt_data.ctx, key_len);
+ if (IS_ERR(shm)) {
+ dev_err(pvt_data.dev, "key shm alloc failed\n");
+ return PTR_ERR(shm);
+ }
+ buf = tee_shm_get_va(shm, 0);
+ if (IS_ERR(buf)) {
+ ret = PTR_ERR(buf);
+ goto out;
}
inv_arg.func = TA_CMD_GET_RANDOM;
@@ -199,7 +202,7 @@ static int trusted_tee_get_random(unsigned char *key, size_t key_len)
inv_arg.num_params = 4;
param[0].attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT;
- param[0].u.memref.shm = reg_shm;
+ param[0].u.memref.shm = shm;
param[0].u.memref.size = key_len;
param[0].u.memref.shm_offs = 0;
@@ -209,18 +212,19 @@ static int trusted_tee_get_random(unsigned char *key, size_t key_len)
inv_arg.ret);
ret = -EFAULT;
} else {
+ memcpy(key, buf, min(param[0].u.memref.size, key_len));
ret = param[0].u.memref.size;
}
- tee_shm_free(reg_shm);
+out:
+ tee_shm_free(shm);
return ret;
}
static int optee_ctx_match(struct tee_ioctl_version_data *ver, const void *data)
{
- if (ver->impl_id == TEE_IMPL_ID_OPTEE &&
- ver->gen_caps & TEE_GEN_CAP_REG_MEM)
+ if (ver->impl_id == TEE_IMPL_ID_OPTEE)
return 1;
else
return 0;
--
2.34.1
next reply other threads:[~2023-08-03 12:35 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-03 12:35 Jens Wiklander [this message]
2023-08-07 7:58 ` [PATCH] KEYS: trusted: tee: use tee_shm_register_alloc_buf() Sumit Garg
2023-08-08 9:07 ` Jens Wiklander
2023-08-21 7:45 ` Jens Wiklander
2023-08-21 8:31 ` Sumit Garg
2023-08-21 9:35 ` Sumit Garg
2023-08-21 10:55 ` Jens Wiklander
2023-08-21 10:51 ` Jens Wiklander
2023-08-21 12:09 ` Sumit Garg
2023-08-21 15:36 ` Jens Wiklander
2023-08-22 11:39 ` Sumit Garg
2023-08-21 8:21 ` Sumit Garg
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=20230803123515.4018838-1-jens.wiklander@linaro.org \
--to=jens.wiklander@linaro.org \
--cc=jarkko@kernel.org \
--cc=jejb@linux.ibm.com \
--cc=keyrings@vger.kernel.org \
--cc=linux-integrity@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=op-tee@lists.trustedfirmware.org \
--cc=sumit.garg@linaro.org \
--cc=zohar@linux.ibm.com \
/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 read-only IMAP folder(s) and NNTP newsgroup(s).