core: remove algo from crypto_hash_*()

Removes the algo parameters from all crypto_hash_*() functions except
crypto_hash_alloc_ctx().

Reviewed-by: Etienne Carriere <etienne.carriere@linaro.org>
Ack

core: remove algo from crypto_hash_*()

Removes the algo parameters from all crypto_hash_*() functions except
crypto_hash_alloc_ctx().

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

show more ...

cryp: prevent direct calls to update and final functions

With inconsistent or malformed data it has been possible to call
"update" and "final" crypto functions directly. Using a fuzzer tool [1]
we h

cryp: prevent direct calls to update and final functions

With inconsistent or malformed data it has been possible to call
"update" and "final" crypto functions directly. Using a fuzzer tool [1]
we have seen that this results in asserts, i.e., a crash that
potentially could leak sensitive information.

By setting the state (initialized) in the crypto context (i.e., the
tee_cryp_state) at the end of all syscall_*_init functions and then add
a check of the state at the beginning of all update and final functions,
we prevent direct entrance to the "update" and "final" functions.

[1] https://github.com/MartijnB/optee_fuzzer

Fixes: OP-TEE-2019-0021

Signed-off-by: Joakim Bech <joakim.bech@linaro.org>
Reported-by: Martijn Bogaard <bogaard@riscure.com>
Acked-by: Jerome Forissier <jerome.forissier@linaro.org>
Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>

show more ...

cryp: ensure that mode is cipher in syscall_cipher_init

When calling syscall_cipher_init there is no check being done that the
state coming from the TA has been initialized to a valid cipher state.

cryp: ensure that mode is cipher in syscall_cipher_init

When calling syscall_cipher_init there is no check being done that the
state coming from the TA has been initialized to a valid cipher state.
By checking the class we prevent an assert in cipher_ops.

Fixes: OP-TEE-2019-0020

Signed-off-by: Joakim Bech <joakim.bech@linaro.org>
Reported-by: Martijn Bogaard <bogaard@riscure.com>
Acked-by: Jerome Forissier <jerome.forissier@linaro.org>
Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>

show more ...

cryp: ensure that mode is AE in syscall_authenc_ functions

When doing calls to syscall_authenc_xyz functions (all of them except
syscall_authenc_init) there is no check being done that the state com

cryp: ensure that mode is AE in syscall_authenc_ functions

When doing calls to syscall_authenc_xyz functions (all of them except
syscall_authenc_init) there is no check being done that the state coming
from the TA has been initialized to a valid authenticated encryption
state. As a consequence of that it's possible to redirect execution to
other functions. Doing like that will make TEE core end up with a data
abort.

Fixes: OP-TEE-2019-0019

Signed-off-by: Joakim Bech <joakim.bech@linaro.org>
Reported-by: Martijn Bogaard <bogaard@riscure.com>
Acked-by: Jerome Forissier <jerome.forissier@linaro.org>
Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>

show more ...

core: tadb.c: get rid of atomic reference counting

This commit changes the way the tadb_db global variable is protected
against concurrent access on creation and deletion. Instead of using an
atomic

core: tadb.c: get rid of atomic reference counting

This commit changes the way the tadb_db global variable is protected
against concurrent access on creation and deletion. Instead of using an
atomic reference counter (struct refcount) and a mutex, only the mutex
is used and taken unconditionally. The reference count becomes a global
integer protected by the same mutex.

Using a struct refcount was apparently an optimization to avoid taking
the lock unless actual creation or deletion of the tadb_db was needed.
Unfortunately this implementation was causing occasional crashes of the
TEE core (easily reproducible on HiKey running 'xtest 1013' in a loop).
The new implementation is simpler and appears to be rock solid with no
measurable difference in performance.

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

show more ...

core: move sockets PTA to core/tee

The sockets pseudo-TA is architecture-independent. Move it to
core/tee and drop the pta_ prefix which is not really useful.

Signed-off-by: Jerome Forissier <jerom

core: move sockets PTA to core/tee

The sockets pseudo-TA is architecture-independent. Move it to
core/tee and drop the pta_ prefix which is not really useful.

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

show more ...

rpmb: fix parsing of op_result

From the eMMC spec, the "Operation result" (Table 19) -- 7 bit quantity
-- is the LSB of "Operation Results data structure" -- 16-bit quantity
-- minus the high order

rpmb: fix parsing of op_result

From the eMMC spec, the "Operation result" (Table 19) -- 7 bit quantity
-- is the LSB of "Operation Results data structure" -- 16-bit quantity
-- minus the high order bit. In other words it is
'rpmb_data_frame::op_result[1] & 0x7F' which is probably what we should
be doing here instead of bytes_to_u16().

Signed-off-by: Victor Chong <victor.chong@linaro.org>
Reviewed-by: Jerome Forissier <jerome.forissier@linaro.org>

show more ...

rpmb: Convert comment about error into EMSG

This will give users more details without having to sift through the
code.

Signed-off-by: Victor Chong <victor.chong@linaro.org>
Reviewed-by: Jerome Fori

rpmb: Convert comment about error into EMSG

This will give users more details without having to sift through the
code.

Signed-off-by: Victor Chong <victor.chong@linaro.org>
Reviewed-by: Jerome Forissier <jerome.forissier@linaro.org>

show more ...

rpmb: verify key: change DMSG to EMSG

Rather than printing all results with DMSG, it's more suitable to
print only errors with EMSG.

Signed-off-by: Victor Chong <victor.chong@linaro.org>
Reviewed-b

rpmb: verify key: change DMSG to EMSG

Rather than printing all results with DMSG, it's more suitable to
print only errors with EMSG.

Signed-off-by: Victor Chong <victor.chong@linaro.org>
Reviewed-by: Jerome Forissier <jerome.forissier@linaro.org>

show more ...

rpmb: print error if derive key fails

Let users know if an RPMB key fails to be generated during RPMB
initializations instead of just exiting the function quietly.

Signed-off-by: Victor Chong <vict

rpmb: print error if derive key fails

Let users know if an RPMB key fails to be generated during RPMB
initializations instead of just exiting the function quietly.

Signed-off-by: Victor Chong <victor.chong@linaro.org>
Reviewed-by: Jerome Forissier <jerome.forissier@linaro.org>

show more ...

rpmb: dump key if CFG_RPMB_WRITE_KEY=y

If we want to write key, then we'd want to write it down as well, so
print it for records.

Note that the key is printed with severity TRACE_DEBUG hence a rele

rpmb: dump key if CFG_RPMB_WRITE_KEY=y

If we want to write key, then we'd want to write it down as well, so
print it for records.

Note that the key is printed with severity TRACE_DEBUG hence a release
build will not leak it.

Signed-off-by: Victor Chong <victor.chong@linaro.org>
Reviewed-by: Jerome Forissier <jerome.forissier@linaro.org>

show more ...

rpmb: write key only if not yet programmed

An RPMB key should only be written if the device returns
RPMB_RESULT_AUTH_KEY_NOT_PROGRAMMED, not on any RPMB_RESULT* that is
not RPMB_RESULT_OK.

Signed-o

rpmb: write key only if not yet programmed

An RPMB key should only be written if the device returns
RPMB_RESULT_AUTH_KEY_NOT_PROGRAMMED, not on any RPMB_RESULT* that is
not RPMB_RESULT_OK.

Signed-off-by: Victor Chong <victor.chong@linaro.org>
Reviewed-by: Jerome Forissier <jerome.forissier@linaro.org>

show more ...

tee_ree_fs: create dirfile only when it's not found

Currently there's no check of return value of tee_fs_dirfile_open,
it's reasonable to do this when dir file truely doesn't exist.
However, if tee_

tee_ree_fs: create dirfile only when it's not found

Currently there's no check of return value of tee_fs_dirfile_open,
it's reasonable to do this when dir file truely doesn't exist.
However, if tee_fs_dirfile_open fails with other reason, calling
tee_fs_dirfile_open(true..) will overlap the old dir file, thus
file access in the future will fail.

Signed-off-by: Fangsuo Wu <fangsuowu@asrmicro.com>
Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>

show more ...

core: fix enum restart with syscall_storage_start_enum()

According to the GlobalPlatform specification it should be possible to
call TEE_StartPersistentObjectEnumerator(..) on an enumerator that
alr

core: fix enum restart with syscall_storage_start_enum()

According to the GlobalPlatform specification it should be possible to
call TEE_StartPersistentObjectEnumerator(..) on an enumerator that
already has been started. When doing that we trigged an assert and ended
up with a panic. This patch fixes that issue by ensuring that we are
closing the currently open directory before re-opening or opening
another directory in those cases where
TEE_StartPersistentObjectEnumerator(..) are called again and again with
no reset done in-between.

Fixes: https://github.com/OP-TEE/optee_os/issues/3093

Signed-off-by: Joakim Bech <joakim.bech@linaro.org>
Reported-by: Daniel McIlvaney <damcilva@microsoft.com>
Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>
Reviewed-by: Jerome Forissier <jerome.forissier@linaro.org>
Tested-by: Jerome Forissier <jerome.forissier@linaro.org> (QEMU, GP)

show more ...

core: REE FS: use mempool_default for temp alloc

Uses mempool_default for temporary block allocation.

This fixes one out of memory error when loading multiple TAs in
parallel.

Acked-by: Jerome For

core: REE FS: use mempool_default for temp alloc

Uses mempool_default for temporary block allocation.

This fixes one out of memory error when loading multiple TAs in
parallel.

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

show more ...

core: RPMB FS: fix panic when RPMB partition size is 16 MiB

The overflow check used when computing the number of the last block in
the RPMB parition is incorrect. It causes an overflow when
rpmb_siz

core: RPMB FS: fix panic when RPMB partition size is 16 MiB

The overflow check used when computing the number of the last block in
the RPMB parition is incorrect. It causes an overflow when
rpmb_size_mult is 128, that is, when the partition size is 16 MiB.
Indeed, max_blk_idx is a uint16_t and we are trying to store 65536
(= 128 * (128 * 1024) / 256).

Fix this by using a 32-bit temporary variable to hold the result of the
multiplication (the number of blocks), then subtract 1 to get the last
block number using SUB_OVERFLOW().

Fixes: ea81076f7896 ("core: RPMB FS: check for potential overflows")
Fixes: https://github.com/OP-TEE/optee_os/issues/3012
Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
Reported-by: Pengguang Zhu <zpghao@163.com>
Suggested-by: Jens Wiklander <jens.wiklander@linaro.org>
Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>

show more ...

core:tee: remove redundant tee_obj_attr_to_binary() calls

A couple of tee_obj_attr_to_binary() calls are useless,
remove them.

Signed-off-by: Sahil Malhotra <sahil.malhotra@nxp.com>
Fixes: https://

core:tee: remove redundant tee_obj_attr_to_binary() calls

A couple of tee_obj_attr_to_binary() calls are useless,
remove them.

Signed-off-by: Sahil Malhotra <sahil.malhotra@nxp.com>
Fixes: https://github.com/OP-TEE/optee_os/issues/3004
Reviewed-by: Jerome Forissier <jerome.forissier@linaro.org>
Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>
Tested-by: Jerome Forissier <jerome.forissier@linaro.org> (QEMU, GP)

show more ...

core: storage: add some overflow checks

Adds overflow checks to the secure storage code.

Signed-off-by: Bastien Simondi <bsimondi@netflix.com>
[jf: Fix test in syscall_storage_obj_seek() case TEE_D

core: storage: add some overflow checks

Adds overflow checks to the secure storage code.

Signed-off-by: Bastien Simondi <bsimondi@netflix.com>
[jf: Fix test in syscall_storage_obj_seek() case TEE_DATA_SEEK_END]
[jf: Get rid of { } block, initialize new local variables]
[jf: Do not fail on (data && !len) in syscall_storage_obj_create()]
Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>
Reviewed-by: Joakim Bech <joakim.bech@linaro.org>

show more ...

core: check allocated size of temporary secure memory

When servicing syscall_invoke_ta_command(), the invoked TA could modify
the .size field. Make sure the allocated buffer is not overwritten on
re

core: check allocated size of temporary secure memory

When servicing syscall_invoke_ta_command(), the invoked TA could modify
the .size field. Make sure the allocated buffer is not overwritten on
return.

Signed-off-by: Bastien Simondi <bsimondi@netflix.com>
[jf: fix multi-line comment, replace '= { 0 };' with '= { };']
[jf: add commit description]
Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>
Reviewed-by: Joakim Bech <joakim.bech@linaro.org>

show more ...

core: FS: wipe sensitive data after use

The secure storage code makes use of various cryptographic data (keys
and IVs). Make sure the buffers are wiped after use to minimize the
risks that sensitive

core: FS: wipe sensitive data after use

The secure storage code makes use of various cryptographic data (keys
and IVs). Make sure the buffers are wiped after use to minimize the
risks that sensitive data may be leaked to an attacker who would have
gained some access to the secure memory.

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

show more ...

core: scrub user-tainted memory returned by alloc_temp_sec_mem()

This is a security fix for TA-to-TA calls.

In syscall_open_ta_session() and syscall_invoke_ta_command(), caller TA
can reference som

core: scrub user-tainted memory returned by alloc_temp_sec_mem()

This is a security fix for TA-to-TA calls.

In syscall_open_ta_session() and syscall_invoke_ta_command(), caller TA
can reference some private memory, in which case the kernel makes a
temporary copy. Unfortunately, memory allocated through
alloc_temp_sec_mem() is not cleared when returned. One could leverage
this to copy arbitrary data into this secure memory pool or to snoop
former data from a previous call done by another TA (e.g., using
TEE_PARAM_TYPE_MEMREF_OUTPUT allows to map the data while not overwriting
it, hence accessing to what is already there).

This patch introduces mobj_free_wipe() to clear and free an mobj.

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

show more ...

core: scrub user-tainted kernel heap memory before freeing it

Some syscalls can be used to poison kernel heap memory. Data copied from
userland is not wiped when the syscall returns. For instance, w

core: scrub user-tainted kernel heap memory before freeing it

Some syscalls can be used to poison kernel heap memory. Data copied from
userland is not wiped when the syscall returns. For instance, when doing
syscall_log() one can copy arbitrary data of variable length onto kernel
memory. When free() is called, the block is returned to the memory pool,
tainted with that userland data. This might be used in combination with
some other vulnerability to produce an exploit.

This patch uses free_wipe() to clear the buffers that have been used to
store user-provided data before returning them to the heap.

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

show more ...

core: add get_tag() to struct user_ta_store_ops

Adds get_tag() method to struct user_ta_store_ops.

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

core: add get_tag() to struct user_ta_store_ops

Adds get_tag() method to struct user_ta_store_ops.

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

show more ...

core: add tee_tadb_get_tag()

Adds the function tee_tadb_get_tag() which returns a tag that uniquely
identifies a TA.

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

core: add tee_tadb_get_tag()

Adds the function tee_tadb_get_tag() which returns a tag that uniquely
identifies a TA.

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

show more ...

core: remove flags argument from tee_pager_alloc()

Removes the flags argument from tee_pager_alloc() since it's only used
with TEE_MATTR_LOCKED. The exception is the bignum pool, but since it
still

core: remove flags argument from tee_pager_alloc()

Removes the flags argument from tee_pager_alloc() since it's only used
with TEE_MATTR_LOCKED. The exception is the bignum pool, but since it
still releases all locked pages each time the pool becomes unused it's
efficient usage of memory.

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

show more ...