core: elf_load: R_ARM_ABS32/R_AARCH64_ABS64 relocations against undefined symbol

The symbol referenced by a R_ARM_ABS32 or R_AARCH64_ABS64 relocation
may very well be external to the binary being re

core: elf_load: R_ARM_ABS32/R_AARCH64_ABS64 relocations against undefined symbol

The symbol referenced by a R_ARM_ABS32 or R_AARCH64_ABS64 relocation
may very well be external to the binary being relocated (for example,
defined in a shared library). In this case, the section table index for
the symbol is SHN_UNDEF and we need to perform process-wide symbol
resolution.

This fixes an issue I found when linking a TA against a shared version
of libutee (this configuration is introduced in a later commit). In this
case, ta_head::entry is set to __utee_entry which is in libutee.so,
hence undefined in the TA binary.

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
Acked-by: Jens Wiklander <jens.wiklander@linaro.org>

show more ...

core: cleanup generic traces

Remove useless newline character in few generic debug traces.

Remove argument __func__ from a FMSG trace since already
output by macro FMSG().

Remove error trace from

core: cleanup generic traces

Remove useless newline character in few generic debug traces.

Remove argument __func__ from a FMSG trace since already
output by macro FMSG().

Remove error trace from syscall_storage_obj_read() that, prior
this change, output failing error code from storage read()
handler. This is useless and not done for other storage handlers
return code.

Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
Reviewed-by: Jerome Forissier <jerome.forissier@linaro.org>

show more ...

core/generic_boot: consistent DTB info traces

Use IMSG() traces when external and embedded DTB are tested.

Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
Reviewed-by: Jerome Forissie

core/generic_boot: consistent DTB info traces

Use IMSG() traces when external and embedded DTB are tested.

Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
Reviewed-by: Jerome Forissier <jerome.forissier@linaro.org>

show more ...

core/generic_boot: tone down trace for missing external DTB

Change trace message indicating absence of external DTB
from error level to debug level. Implementation and comment
clearly state the conf

core/generic_boot: tone down trace for missing external DTB

Change trace message indicating absence of external DTB
from error level to debug level. Implementation and comment
clearly state the configuration is fully legitimate.

Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
Reviewed-by: Jerome Forissier <jerome.forissier@linaro.org>

show more ...

Compile user TAs with -fpic rather than -fpie

TA source files are compiled with the -fpie GCC flag in order to
generate a Position Independent Executable. This is not suitable to
produce a shared li

Compile user TAs with -fpic rather than -fpie

TA source files are compiled with the -fpie GCC flag in order to
generate a Position Independent Executable. This is not suitable to
produce a shared library as introduced by commit f8896d1301fc ("TA dev
kit: add support for creating shared libraries"). -fpic should be used
instead. Here is what the GCC man page has to say on these flags:

-fpic
Generate position-independent code (PIC) suitable for use in a
shared library [...]

-fpie
-fPIE
These options are similar to -fpic and -fPIC, but generated
position independent code can be only linked into executables.

So, it is quite clear that -fpie is wrong for a shared library. It is
not very clear however if -fpic can be used when generating code for an
executable. I think it can, and there is a bug report against the GCC
documentation that would confirm this [1]. Therefore we can simply use
-fpic in all cases. This is quite convenient because we currently make
no difference in the compile flags when we are building an executable,
a static library or a shared library.

The difference between -fpie and -fpic has to do with the kinds of
relocations that the compiler is allowed to emit. I stumbled upon this
issue when experimenting with shared libraries and the code proposed
by Jens to share read-only pages between TAs [2]. In my test case, a
shared library already loaded by one TA, is used by another TA. During
the load phase of the second TA, the TEE core crashed with a data-abort
(write permission fault) when trying to apply an R_ARM_REL32 relocation
to some literal pool data in the .text section of the library. The
whole .text being mapped read-only, there should be no relocation to do
here. And indeed the cause was the wrong flag (-fpie) used when
compiling the shared library.

Link: [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70419
Link: [2] https://github.com/OP-TEE/optee_os/pull/2801
Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
CC: Jens Wiklander <jens.wiklander@linaro.org>
Tested-by: Jerome Forissier <jerome.forissier@linaro.org> (QEMU)
Tested-by: Jerome Forissier <jerome.forissier@linaro.org> (HiKey960 32 & 64-bit TA)
Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>
Tested-by: Etienne Carriere <etienne.carriere@linaro.org> (stm32mp1 Armv7)

show more ...

core: replace TAF with TA invocation in inline comments

TAF stands for Trusted Application Function. It is deprecated
since a while. This change replaces the few remaining occurrences
with TA invoca

core: replace TAF with TA invocation in inline comments

TAF stands for Trusted Application Function. It is deprecated
since a while. This change replaces the few remaining occurrences
with TA invocation.

Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>

show more ...

core: use PTA as acronym for pseudo TA

Make inline comments and trace messages more consistent by
using PTA as acronym for pseudo TA, rather than using pTA, PTA
and pta at various places.

Signed-of

core: use PTA as acronym for pseudo TA

Make inline comments and trace messages more consistent by
using PTA as acronym for pseudo TA, rather than using pTA, PTA
and pta at various places.

Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>

show more ...

core: add inline description of TEE session field user_ctx

Replace inline comment ??? with an appropriate description of
the user_ctx field which points the the caller user TA context
in case a pseu

core: add inline description of TEE session field user_ctx

Replace inline comment ??? with an appropriate description of
the user_ctx field which points the the caller user TA context
in case a pseudo TA is invoked from the user TA client.

Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>

show more ...

core: pta/gprof.c: remove redundant access check

There is no need to call tee_mmu_check_access_rights() to check the
parameters of gprof_start_pc_sampling(), because they have been checked
already b

core: pta/gprof.c: remove redundant access check

There is no need to call tee_mmu_check_access_rights() to check the
parameters of gprof_start_pc_sampling(), because they have been checked
already by utee_param_to_param() in core/tee/tee_svc.c.

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
Reviewed-by: Joakim Bech <joakim.bech@linaro.org>
Reviewed-by: Etienne Carriere <etienne.carriere@linaro.org>

show more ...

core: REE FS TAs: add option to verify signature before processing

Adds configuration flag CFG_REE_FS_TA_BUFFERED, default enabled.

A new TA store is introduced which depends on the TEE FS TA store

core: REE FS TAs: add option to verify signature before processing

Adds configuration flag CFG_REE_FS_TA_BUFFERED, default enabled.

A new TA store is introduced which depends on the TEE FS TA store to
load the whole binary into a temporary buffer in secure DDR and
authenticate it before being processed further.

This reduces the attack surface of the TEE core in case of a
vulnerability in the ELF loader, at the expense of increased memory
usage at load time.

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
Reported-by: Bastien Simondi <bsimondi@netflix.com> [3.6]
Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>
Reviewed-by: Etienne Carriere <etienne.carriere@linaro.org>

show more ...

libutils: remove buf_compare_ct()

Now that we have consttime_memcmp(), buf_compare_ct() is redundant.
Every time buf_compare_ct() is used, consttime_memcmp() may be used
instead.

This commit remove

libutils: remove buf_compare_ct()

Now that we have consttime_memcmp(), buf_compare_ct() is redundant.
Every time buf_compare_ct() is used, consttime_memcmp() may be used
instead.

This commit removes buf_compare_ct(). A compatibility wrapper is kept
in <string_ext.h> to avoid knowingly breaking the build of any TA that
may use it.

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>

show more ...

core: RPMB FS: check for potential overflows

This commit deals with a number of potential integer overflows in the
RPMB FS code.

rpmb_fs_init() requests device information from the REE. The RPMB si

core: RPMB FS: check for potential overflows

This commit deals with a number of potential integer overflows in the
RPMB FS code.

rpmb_fs_init() requests device information from the REE. The RPMB size
is returned in struct rpmb_dev_info (field rpmb_size_mult) and is used
in a multiplication that could overflow. Use MUL_OVERFLOW() to deal with
this case.

Some overflow checks are also added in the read and write paths.

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
Reported-by: Bastien Simondi <bsimondi@netflix.com> [2.12]
Reviewed-by: Joakim Bech <joakim.bech@linaro.org>

show more ...

core: syscall_authenc_init(): check nonce accessibility

syscall_authenc_init() does not check that the given nonce address is
within TA accessible memory. Fix that.

Signed-off-by: Jerome Forissier

core: syscall_authenc_init(): check nonce accessibility

syscall_authenc_init() does not check that the given nonce address is
within TA accessible memory. Fix that.

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
Reported-by: Bastien Simondi <bsimondi@netflix.com> [2.10]
Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>
Reviewed-by: Joakim Bech <joakim.bech@linaro.org>

show more ...

core: crypto: add overflow check when copying attributes

In copy_in_attrs(), attr_count * sizeof(struct utee_attribute) could
overflow if a very large attr_count is given. Use MUL_OVERFLOW() to
prop

core: crypto: add overflow check when copying attributes

In copy_in_attrs(), attr_count * sizeof(struct utee_attribute) could
overflow if a very large attr_count is given. Use MUL_OVERFLOW() to
properly deal with this case.

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
Reported-by: Bastien Simondi <bsimondi@netflix.com> [2.9]
Reviewed-by: Joakim Bech <joakim.bech@linaro.org>

show more ...

core: get_elf_segments(): use memmove on overlapping memory

get_elf_segments() final stage aggregates ELF segments. In the while
loop, the logic to remove the current index is to use memcpy() to shi

core: get_elf_segments(): use memmove on overlapping memory

get_elf_segments() final stage aggregates ELF segments. In the while
loop, the logic to remove the current index is to use memcpy() to shift
down everything beyond that point. This is incorrect; memmove() should
be used instead.

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
Reported-by: Bastien Simondi <bsimondi@netflix.com> [2.8]
Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>
Reviewed-by: Joakim Bech <joakim.bech@linaro.org>

show more ...

core: load_elf_from_store(): check stack size

Inside load_elf_from_store(), the ta_head structure is retrieved from
un-authenticated area, and contains the stack size. The stack size could
either al

core: load_elf_from_store(): check stack size

Inside load_elf_from_store(), the ta_head structure is retrieved from
un-authenticated area, and contains the stack size. The stack size could
either already be 0, or could be large enough so it becomes 0 when rounded
up to STACK_ALIGNMENT. This could result in vm_map() returning a virtual
address for a 0-size memory block or other issues.

Check the rounded-up stack_size value before using it.

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
Reported-by: Bastien Simondi <bsimondi@netflix.com> [2.7]
Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>
Reviewed-by: Joakim Bech <joakim.bech@linaro.org>

show more ...

core: add VA overflow check in shdr_alloc_and_copy()

Make sure that no address overflow can occur when shdr_alloc_and_copy()
copies the signed header.

Signed-off-by: Jerome Forissier <jerome.foriss

core: add VA overflow check in shdr_alloc_and_copy()

Make sure that no address overflow can occur when shdr_alloc_and_copy()
copies the signed header.

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
Reported-by: Bastien Simondi <bsimondi@netflix.com> [2.4]
Reviewed-by: Joakim Bech <joakim.bech@linaro.org>

show more ...

core: add overflow check in mobj_reg_shm_alloc()

In function mobj_reg_shm_alloc(), the macro MOBJ_REG_SHM_SIZE() could
overflow depending on 'nr_pages'. In such case, the mobj_reg_shm memory
would b

core: add overflow check in mobj_reg_shm_alloc()

In function mobj_reg_shm_alloc(), the macro MOBJ_REG_SHM_SIZE() could
overflow depending on 'nr_pages'. In such case, the mobj_reg_shm memory
would be a small memory block, while num_pages would be large, which could
lead to a generous memcpy() when copying the pages in internal memory, the
outcome of this depends on memory mapping.

Note: no attack path are identified to exploit this overflow, however it
is error prone and could lead to a future vulnerability.

This commit replaces the MOBJ_REG_SHM_SIZE() macro with a static
function that performs the same computation, but returns 0 in case of
integer overflow. The call site is updated to return an error status
should this situation happen.

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
Reported-by: Bastien Simondi <bsimondi@netflix.com> [2.3]
Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>
Reviewed-by: Joakim Bech <joakim.bech@linaro.org>

show more ...

core: do not use virtual addresses as session identifier

Session context virtual address is returned to the REE in
entry_open_session(); it is then used back in entry_close_session() and
entry_invok

core: do not use virtual addresses as session identifier

Session context virtual address is returned to the REE in
entry_open_session(); it is then used back in entry_close_session() and
entry_invoke_command(). Sharing virtual addresses with the REE leads to
virtual memory addresses disclosure that could be leveraged to defeat
ASLR (if/when implemented) and/or mount an attack.

Similarly, syscall_open_ta_session() returns a session ID directly
derived from the session virtual address to the caller TA.

This commit introduces a 32-bit identifier field in struct tee_ta_session.
The ID is generated when the session is created, starting from the id of
the last session in the queue, and counting up until a number that is not
used in the session queue is found.

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
Reported-by: Bastien Simondi <bsimondi@netflix.com> [2.1]
Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>
Reviewed-by: Joakim Bech <joakim.bech@linaro.org>

show more ...

core: ELF relocation: use ADD_OVERFLOW()

The ELF relocation functions e32_process_rel() and e64_process_rel()
can experience integer overflows which could result in invalid memory
access. Use ADD_OV

core: ELF relocation: use ADD_OVERFLOW()

The ELF relocation functions e32_process_rel() and e64_process_rel()
can experience integer overflows which could result in invalid memory
access. Use ADD_OVERFLOW() to prevent these.

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
Reported-by: Bastien Simondi <bsimondi@netflix.com> [1.8]
Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>

show more ...

core: elf_load_body(): use MUL_OVERFLOW() to get size of section headers

At the end of elf_load_body(), section headers are copied in a system heap
memory block, associated to state->shdr. As the co

core: elf_load_body(): use MUL_OVERFLOW() to get size of section headers

At the end of elf_load_body(), section headers are copied in a system heap
memory block, associated to state->shdr. As the computed size is the
result of an uncontrolled multiplication (ehdr.e_shnum * ehdr.e_shentsize),
it could have overflowed and result in allocating a small memory block.

Use an overflow checking macro to prevent this case.

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
Reported-by: Bastien Simondi <bsimondi@netflix.com> [1.7]
Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>
Reviewed-by: Joakim Bech <joakim.bech@linaro.org>

show more ...

core: umap_add_region(): add overflow check

Use ADD_OVERFLOW() to be more resilient to very large values
potentially passed to umap_add_region().

Signed-off-by: Jerome Forissier <jerome.forissier@l

core: umap_add_region(): add overflow check

Use ADD_OVERFLOW() to be more resilient to very large values
potentially passed to umap_add_region().

Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
Reported-by: Bastien Simondi <bsimondi@netflix.com> [1.3]
Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>
Reviewed-by: Joakim Bech <joakim.bech@linaro.org>

show more ...

core: entry_std.c: clean memory type inline comments

This change modifies inline comments.

Replace "non sec" and "nonsecure" with "non-secure".
Fixup "Rerefence" into "reference".
Clarify contiguou

core: entry_std.c: clean memory type inline comments

This change modifies inline comments.

Replace "non sec" and "nonsecure" with "non-secure".
Fixup "Rerefence" into "reference".
Clarify contiguous shared memory comment.
Minor rephrasing for consistency.

Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
Reviewed-by: Jerome Forissier <jerome.forissier@linaro.org>
Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>
[jf: minor edit to commit subject]
Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>

show more ...

stm32mp1: embed BSEC driver

Embed BSEC driver in platform stm32mp1. The platform implements
stm32mp_get_bsec_static_cfg() to provide BSEC static configuration.

Add BSEC node in stm32mp157c.dtsi.
Ad

stm32mp1: embed BSEC driver

Embed BSEC driver in platform stm32mp1. The platform implements
stm32mp_get_bsec_static_cfg() to provide BSEC static configuration.

Add BSEC node in stm32mp157c.dtsi.
Add BSEC node with some BSEC word definition and assignment (non-secure
and/or secure) for board stm32mp157c-ed1 and stm32mp157c-ev1.

Signed-off-by: Etienne Carriere <etienne.carriere@st.com>
Acked-by: Jens Wiklander <jens.wiklander@linaro.org>

show more ...

stm32_bsec: OTP driver for stm32mp platforms

BSEC is a one time programmable (OTP) memory interface for stm32mp
SoCs. OTPs are grouped into 32bit words identified by a incremental ID
starting from 0

stm32_bsec: OTP driver for stm32mp platforms

BSEC is a one time programmable (OTP) memory interface for stm32mp
SoCs. OTPs are grouped into 32bit words identified by a incremental ID
starting from 0. Shadowed OTPs are loaded in a volatile memory yet
used as OTP values by the software.

The platform shall implement stm32mp_get_bsec_static_cfg() to
provide BSEC driver some information as the BSEC memory size and
its lower/upper threshold ID that split non-secure from secure OTPs.

Signed-off-by: Etienne Carriere <etienne.carriere@st.com>
Signed-off-by: Christophe Montaud <christophe.montaud@st.com>
Signed-off-by: Lionel Debieve <lionel.debieve@st.com>
Signed-off-by: Mathieu Belou <mathieu.belou@st.com>
Signed-off-by: Nicolas Le Bayon <nicolas.le.bayon@st.com>
Signed-off-by: Yann Gautier <yann.gautier@st.com>
Acked-by: Jens Wiklander <jens.wiklander@linaro.org>

show more ...