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