linux-kernel - Re: [PATCH 1/1] tee: amdtee: Add return_origin to 'struct tee_cmd_load

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 5 May 2023 09:30:28 +0200
From:   Jens Wiklander <jens.wiklander@...aro.org>
To:     Rijo Thomas <Rijo-john.Thomas@....com>
Cc:     Sumit Garg <sumit.garg@...aro.org>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        Jan Dabros <jsd@...ihalf.com>,
        Tom Lendacky <thomas.lendacky@....com>,
        Jarkko Nikula <jarkko.nikula@...ux.intel.com>,
        op-tee@...ts.trustedfirmware.org, linux-kernel@...r.kernel.org,
        Mythri PK <Mythri.Pandeshwarakrishna@....com>,
        Devaraj Rangasamy <Devaraj.Rangasamy@....com>,
        stable@...r.kernel.org, Sourabh Das <sourabh.das@....com>,
        Nimesh Easow <nimesh.easow@....com>
Subject: Re: [PATCH 1/1] tee: amdtee: Add return_origin to 'struct tee_cmd_load_ta'

Hi,

On Tue, May 2, 2023 at 8:25 AM Rijo Thomas <Rijo-john.Thomas@....com> wrote:
>
> After TEE has completed processing of TEE_CMD_ID_LOAD_TA, set proper
> value in 'return_origin' argument passed by open_session() call. To do
> so, add 'return_origin' field to the structure tee_cmd_load_ta. The
> Trusted OS shall update return_origin as part of TEE processing.
>
> This change to 'struct tee_cmd_load_ta' interface requires a similar update
> in AMD-TEE Trusted OS's TEE_CMD_ID_LOAD_TA interface.

This is an ABI change, but it's not clear if it's an incompatible ABI
change or not. What happens if the AMD-TEE Trusted OS isn't updated?

>
> This patch has been verified on Phoenix Birman setup. On older APUs,
> return_origin value will be 0.

Why, because MD-TEE Trusted OS will not be updated on the older APUs?

>
> Cc: stable@...r.kernel.org

Which stable kernels are you targeting? A Fixes tag might answer that.

Thanks,
Jens

> Tested-by: Sourabh Das <sourabh.das@....com>
> Signed-off-by: Rijo Thomas <Rijo-john.Thomas@....com>
> ---
>  drivers/tee/amdtee/amdtee_if.h | 10 ++++++----
>  drivers/tee/amdtee/call.c      | 30 +++++++++++++++++-------------
>  2 files changed, 23 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/tee/amdtee/amdtee_if.h b/drivers/tee/amdtee/amdtee_if.h
> index ff48c3e47375..e2014e21530a 100644
> --- a/drivers/tee/amdtee/amdtee_if.h
> +++ b/drivers/tee/amdtee/amdtee_if.h
> @@ -118,16 +118,18 @@ struct tee_cmd_unmap_shared_mem {
>
>  /**
>   * struct tee_cmd_load_ta - load Trusted Application (TA) binary into TEE
> - * @low_addr:    [in] bits [31:0] of the physical address of the TA binary
> - * @hi_addr:     [in] bits [63:32] of the physical address of the TA binary
> - * @size:        [in] size of TA binary in bytes
> - * @ta_handle:   [out] return handle of the loaded TA
> + * @low_addr:       [in] bits [31:0] of the physical address of the TA binary
> + * @hi_addr:        [in] bits [63:32] of the physical address of the TA binary
> + * @size:           [in] size of TA binary in bytes
> + * @ta_handle:      [out] return handle of the loaded TA
> + * @return_origin:  [out] origin of return code after TEE processing
>   */
>  struct tee_cmd_load_ta {
>         u32 low_addr;
>         u32 hi_addr;
>         u32 size;
>         u32 ta_handle;
> +       u32 return_origin;
>  };
>
>  /**
> diff --git a/drivers/tee/amdtee/call.c b/drivers/tee/amdtee/call.c
> index e8cd9aaa3467..e9b63dcb3194 100644
> --- a/drivers/tee/amdtee/call.c
> +++ b/drivers/tee/amdtee/call.c
> @@ -423,19 +423,23 @@ int handle_load_ta(void *data, u32 size, struct tee_ioctl_open_session_arg *arg)
>         if (ret) {
>                 arg->ret_origin = TEEC_ORIGIN_COMMS;
>                 arg->ret = TEEC_ERROR_COMMUNICATION;
> -       } else if (arg->ret == TEEC_SUCCESS) {
> -               ret = get_ta_refcount(load_cmd.ta_handle);
> -               if (!ret) {
> -                       arg->ret_origin = TEEC_ORIGIN_COMMS;
> -                       arg->ret = TEEC_ERROR_OUT_OF_MEMORY;
> -
> -                       /* Unload the TA on error */
> -                       unload_cmd.ta_handle = load_cmd.ta_handle;
> -                       psp_tee_process_cmd(TEE_CMD_ID_UNLOAD_TA,
> -                                           (void *)&unload_cmd,
> -                                           sizeof(unload_cmd), &ret);
> -               } else {
> -                       set_session_id(load_cmd.ta_handle, 0, &arg->session);
> +       } else {
> +               arg->ret_origin = load_cmd.return_origin;
> +
> +               if (arg->ret == TEEC_SUCCESS) {
> +                       ret = get_ta_refcount(load_cmd.ta_handle);
> +                       if (!ret) {
> +                               arg->ret_origin = TEEC_ORIGIN_COMMS;
> +                               arg->ret = TEEC_ERROR_OUT_OF_MEMORY;
> +
> +                               /* Unload the TA on error */
> +                               unload_cmd.ta_handle = load_cmd.ta_handle;
> +                               psp_tee_process_cmd(TEE_CMD_ID_UNLOAD_TA,
> +                                                   (void *)&unload_cmd,
> +                                                   sizeof(unload_cmd), &ret);
> +                       } else {
> +                               set_session_id(load_cmd.ta_handle, 0, &arg->session);
> +                       }
>                 }
>         }
>         mutex_unlock(&ta_refcount_mutex);
> --
> 2.25.1
>