On Tue, Jan 04, 2022 at 10:24:06AM +0100, Christoph Hellwig wrote:
> On Mon, Dec 13, 2021 at 10:37:55PM +0200, Andy Shevchenko wrote:
> > + Cc: Christoph. Maybe you can apply this one, please?
>
> I've not even seen the original patch.
It's easy to retrieve with b4 tool:
`b4 am -s 20210121183741.45333-1-andriy.shevchenko(a)linux.intel.com`
But for your convenience I may resend once more with you being Cc'ed.
--
With Best Regards,
Andy Shevchenko
There is export_uuid() function which exports uuid_t to the u8 array.
Use it instead of open coding variant.
This allows to hide the uuid_t internals.
Signed-off-by: Andy Shevchenko <andriy.shevchenko(a)linux.intel.com>
---
drivers/firmware/broadcom/tee_bnxt_fw.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/firmware/broadcom/tee_bnxt_fw.c b/drivers/firmware/broadcom/tee_bnxt_fw.c
index ed10da5313e8..4cf0c2576037 100644
--- a/drivers/firmware/broadcom/tee_bnxt_fw.c
+++ b/drivers/firmware/broadcom/tee_bnxt_fw.c
@@ -197,7 +197,7 @@ static int tee_bnxt_fw_probe(struct device *dev)
return -ENODEV;
/* Open session with Bnxt load Trusted App */
- memcpy(sess_arg.uuid, bnxt_device->id.uuid.b, TEE_IOCTL_UUID_LEN);
+ export_uuid(sess_arg.uuid, &bnxt_device->id.uuid);
sess_arg.clnt_login = TEE_IOCTL_LOGIN_PUBLIC;
sess_arg.num_params = 0;
--
2.29.2
translate_noncontig() allocates domheap page for translated list
before calling to allocate_optee_shm_buf(), which can fail for number
of reason. Anyways, after fail we need to free the allocated page(s).
Another leak is possible if the same translate_noncontig() function
fails to get domain page. In this case it should free allocated
optee_shm_buf prior exit. This will also free allocated domheap page.
Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk(a)epam.com>
---
xen/arch/arm/tee/optee.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/tee/optee.c b/xen/arch/arm/tee/optee.c
index 6df0d44eb9..131d2f9a8a 100644
--- a/xen/arch/arm/tee/optee.c
+++ b/xen/arch/arm/tee/optee.c
@@ -781,7 +781,10 @@ static int translate_noncontig(struct optee_domain *ctx,
optee_shm_buf = allocate_optee_shm_buf(ctx, param->u.tmem.shm_ref,
pg_count, xen_pgs, order);
if ( IS_ERR(optee_shm_buf) )
+ {
+ free_domheap_pages(xen_pgs, order);
return PTR_ERR(optee_shm_buf);
+ }
gfn = gaddr_to_gfn(param->u.tmem.buf_ptr &
~(OPTEE_MSG_NONCONTIG_PAGE_SIZE - 1));
@@ -807,7 +810,7 @@ static int translate_noncontig(struct optee_domain *ctx,
{
guest_pg = get_domain_ram_page(gfn);
if ( !guest_pg )
- return -EINVAL;
+ goto free_shm_buf;
guest_data = __map_domain_page(guest_pg);
xen_data = __map_domain_page(xen_pgs);
@@ -854,6 +857,7 @@ err_unmap:
unmap_domain_page(guest_data);
unmap_domain_page(xen_data);
put_page(guest_pg);
+free_shm_buf:
free_optee_shm_buf(ctx, optee_shm_buf->cookie);
return -EINVAL;
--
2.33.0
Hi Oleksandr, Stefano,
Oleksandr <olekstysh(a)gmail.com> writes:
> On 07.10.21 01:42, Stefano Stabellini wrote:
>
> Hi Stefano, Julien.
>
>> On Wed, 6 Oct 2021, Julien Grall wrote:
>>> Hi Stefano,
>>>
>>> On 28/09/2021 06:52, Stefano Stabellini wrote:
>>>> On Mon, 27 Sep 2021, Oleksandr Tyshchenko wrote:
>>>>> From: Oleksandr Tyshchenko <oleksandr_tyshchenko(a)epam.com>
>>>>>
>>>>> Fix a possible copy-paste error in arm_smccc_smc's first argument (a0)
>>>>> for OPTEE_SMC_DISABLE_SHM_CACHE case.
>>>>>
>>>>> This error causes Linux > v5.14-rc5
>>>>> (b5c10dd04b7418793517e3286cde5c04759a86de
>>>>> optee: Clear stale cache entries during initialization) to stuck
>>>>> repeatedly issuing OPTEE_SMC_DISABLE_SHM_CACHE call and waiting for
>>>>> the result to be OPTEE_SMC_RETURN_ENOTAVAIL which will never happen.
>>>>>
>>>>> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko(a)epam.com>
>>>> Acked-by: Stefano Stabellini <sstabellini(a)kernel.org>
>>>>
>>>> I added Fixes: and Backport: tags to the commit
>>> Per SUPPORT.MD, OP-TEE is still a technical preview. So I would argue that we
>>> should not do any backport because the feature itself is not officially
>>> considered supported.
>> Good point!
>>
>>
>>> That said, what's missing to make the feature officially supported?
>> If Oleksandr is also happy to make OP-TEE support in Xen "Supported" in
>> SUPPORT.md I'd be happy with that too. Specifically I suggest to change
>> it to:
>>
>> Status: Supported, not security supported
>>
>> Security Support is a bit of a heavy process and I am thinking that
>> "Supported, not security supported" would be an excellent next step.
>
> I would be happy, and can send a formal patch. But I am not an expert
> in this code.
I'm will be happy with this too. We are using this mediator in our
projects and I know that OP-TEE community adopted tests for
virtualization in theirs CI stack. So this is kind of official now.
Also, I helped other people to bring up virtualization on theirs
platforms, so there are other users for this feature besides EPAM and
Linaro.
> (looks like there are some TODO left in the code and I have no idea
> what are the implications)
Well, there were a lot of TODOs when I submitted initial
implementation. At that time it indeed wasn't ready for production. But
I eventually fixed almost all of them. Only one left now. It is about
very unlikely situation when one of guest pages in mapped at PA=0. I'm
not sure that is even possible at all.
--
Volodymyr Babchuk at EPAM
OP-TEE mediator already have support for NULL memory references. It
was added in patch 0dbed3ad336 ("optee: allow plain TMEM buffers with
NULL address"). But it does not propagate
OPTEE_SMC_SEC_CAP_MEMREF_NULL capability flag to a guest, so well
behaving guest can't use this feature.
Note: linux optee driver honors this capability flag when handling
buffers from userspace clients, but ignores it when working with
internal calls. For instance, __optee_enumerate_devices() function
uses NULL argument to get buffer size hint from OP-TEE. This was the
reason, why "optee: allow plain TMEM buffers with NULL address" was
introduced in the first place.
This patch adds the mentioned capability to list of known
capabilities. From Linux point of view it means that userspace clients
can use this feature, which is confirmed by OP-TEE test suite:
* regression_1025 Test memref NULL and/or 0 bytes size
o regression_1025.1 Invalid NULL buffer memref registration
regression_1025.1 OK
o regression_1025.2 Input/Output MEMREF Buffer NULL - Size 0 bytes
regression_1025.2 OK
o regression_1025.3 Input MEMREF Buffer NULL - Size non 0 bytes
regression_1025.3 OK
o regression_1025.4 Input MEMREF Buffer NULL over PTA invocation
regression_1025.4 OK
regression_1025 OK
Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk(a)epam.com>
---
xen/arch/arm/tee/optee.c | 3 ++-
xen/include/asm-arm/tee/optee_smc.h | 3 +++
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/tee/optee.c b/xen/arch/arm/tee/optee.c
index 9570dc6771..6b59027964 100644
--- a/xen/arch/arm/tee/optee.c
+++ b/xen/arch/arm/tee/optee.c
@@ -96,7 +96,8 @@
#define OPTEE_KNOWN_NSEC_CAPS OPTEE_SMC_NSEC_CAP_UNIPROCESSOR
#define OPTEE_KNOWN_SEC_CAPS (OPTEE_SMC_SEC_CAP_HAVE_RESERVED_SHM | \
OPTEE_SMC_SEC_CAP_UNREGISTERED_SHM | \
- OPTEE_SMC_SEC_CAP_DYNAMIC_SHM)
+ OPTEE_SMC_SEC_CAP_DYNAMIC_SHM | \
+ OPTEE_SMC_SEC_CAP_MEMREF_NULL)
enum optee_call_state {
OPTEE_CALL_NORMAL,
diff --git a/xen/include/asm-arm/tee/optee_smc.h b/xen/include/asm-arm/tee/optee_smc.h
index d568bb2fe1..2f5c702326 100644
--- a/xen/include/asm-arm/tee/optee_smc.h
+++ b/xen/include/asm-arm/tee/optee_smc.h
@@ -244,6 +244,9 @@
*/
#define OPTEE_SMC_SEC_CAP_DYNAMIC_SHM (1 << 2)
+/* Secure world supports Shared Memory with a NULL reference */
+#define OPTEE_SMC_SEC_CAP_MEMREF_NULL (1 << 4)
+
#define OPTEE_SMC_FUNCID_EXCHANGE_CAPABILITIES 9
#define OPTEE_SMC_EXCHANGE_CAPABILITIES \
OPTEE_SMC_FAST_CALL_VAL(OPTEE_SMC_FUNCID_EXCHANGE_CAPABILITIES)
--
2.31.0
Hello,
I've recently been involved in producing PoC, which utilizes OP-TEE
to produce proof of a secret key possession. That is used during tunnel
establishment by OpenVPN.
In case someone finds it interesting, for example as kind of "real-world"
use case of OP-TEE, then details are described here:
https://www.amongbytes.com/post/20210112-optee-openssl-engine/
and code is here:
https://github.com/henrydcase/optee_eng
The actual PoC used post-quantum schemes integrated into TEE OS and TF-A
(secure boot). Those two points are not described really described for brevity
(and probably there is low interest anyway).
Kind regards,
Kris
Hi,
I have been seeing out of memory failures due to tee_shm_free() on
kexec path. This issue in discussed in length at:
link: https://github.com/OP-TEE/optee_os/issues/3637
Driver: drivers/firmware/broadcom/tee_bnxt_fw.c
I have tried various experiments to try and debug this issue but
haven't found a fix. All I have manged is to delay the occurrence
of the issue.
Any pointers would be helpful.
Thanks,
- Allen