v3:
- Tyler inherited the original series from Allen Pais
- New patch to fix memory leaks in OP-TEE's pool_op_alloc()
+ Unrelated to kexec/kdump
- New patch to refuse to load the OP-TEE driver when booting the kdump
kernel
- Minor comment typo cleanups (s/alter/alert/) in the "optee: fix tee
out of memory failure seen during kexec reboot" patch, as mentioned in
v2 feedback
- New patch to clear stale cache entries during initialization to avoid
crashes when kexec'ing from a buggy kernel, that didn't disable the
shm cache, to a fixed kernel
- Three new patches to allow drivers to allocate a multi-page dynamic
shm that's not dma-buf backed but is still fully registered with the
TEE, ensuring that all driver private shms are unregistered during
kexec
v2: https://lore.kernel.org/lkml/[email protected]/
v1: https://lore.kernel.org/lkml/[email protected]/
This series fixes several bugs uncovered while exercising the OP-TEE
(Open Portable Trusted Execution Environment), ftpm (firmware TPM), and
tee_bnxt_fw (Broadcom BNXT firmware manager) drivers with kexec and
kdump (emergency kexec) based workflows.
The majority of the problems are caused by missing .shutdown hooks in
the drivers. The .shutdown hooks are used by the normal kexec code path
to let the drivers clean up prior to executing the target kernel. The
.remove hooks, which are already implemented in these drivers, are not
called as part of the kexec code path. This resulted in shared memory
regions, that were cached and/or registered with OP-TEE, not being
cleared/unregistered prior to kexec. The new kernel would then run into
problems when handling the previously cached virtual addresses or trying
to register newly allocated shared memory objects that overlapped with
the previously registered virtual addresses. The TEE didn't receive
notification that the old virtual addresses were no longer meaningful
and that a new kernel, with a new address space, would soon be running.
However, implementing .shutdown hooks was not enough for supporting
kexec. There was an additional problem caused by the TEE driver's
reliance on the dma-buf subsystem for multi-page shared memory objects
that were registered with the TEE. Shared memory objects backed by a
dma-buf use a different mechanism for reference counting. When the final
reference is released, work is scheduled to be executed to unregister
the shared memory with the TEE but that work is only completed prior to
the current task returning the userspace. In the case of a kexec
operation, the current task that's calling the driver .shutdown hooks
never returns to userspace prior to the kexec operation so the shared
memory was never unregistered. This eventually caused problems from
overlapping shared memory regions that were registered with the TEE
after several kexec operations. The large 4M contiguous region
allocated by the tee_bnxt_fw driver reliably ran into this issue on the
fourth kexec on a system with 8G of RAM.
The use of dma-buf makes sense for shared memory that's in use by
userspace but dma-buf's aren't needed for shared memory that will only
used by the driver. This series separates dma-buf backed shared memory
allocated by the kernel from multi-page shared memory that the kernel
simply needs registered with the TEE for private use.
One other noteworthy change in this series is to completely refuse to
load the OP-TEE driver in the kdump kernel. This is needed because the
secure world may have had all of its threads in suspended state when the
regular kernel crashed. The kdump kernel would then hang during boot
because the OP-TEE driver's .probe function would attempt to use a
secure world thread when they're all in suspended state. Another problem
is that shared memory allocations could fail under the kdump kernel
because the previously registered were not unregistered (the .shutdown
hook is not called when kexec'ing into the kdump kernel).
The first patch in the series fixes potential memory leaks that are not
directly related to kexec or kdump but were noticed during the
development of this series.
Tyler
Allen Pais (2):
optee: fix tee out of memory failure seen during kexec reboot
firmware: tee_bnxt: Release shm, session, and context during kexec
Tyler Hicks (5):
optee: Fix memory leak when failing to register shm pages
optee: Refuse to load the driver under the kdump kernel
optee: Clear stale cache entries during initialization
tee: Support shm registration without dma-buf backing
tpm_ftpm_tee: Free and unregister dynamic shared memory during kexec
drivers/char/tpm/tpm_ftpm_tee.c | 2 +-
drivers/firmware/broadcom/tee_bnxt_fw.c | 11 ++++++-
drivers/tee/optee/call.c | 11 ++++++-
drivers/tee/optee/core.c | 42 ++++++++++++++++++++++++-
drivers/tee/optee/optee_private.h | 2 +-
drivers/tee/optee/shm_pool.c | 17 +++++++---
drivers/tee/tee_shm.c | 11 ++++++-
7 files changed, 85 insertions(+), 11 deletions(-)
--
2.25.1
The shm cache could contain invalid addresses if
optee_disable_shm_cache() was not called from the .shutdown hook of the
previous kernel before a kexec. These addresses could be unmapped or
they could point to mapped but unintended locations in memory.
Clear the shared memory cache, while being careful to not translate the
addresses returned from OPTEE_SMC_DISABLE_SHM_CACHE, during driver
initialization. Once all pre-cache shm objects are removed, proceed with
enabling the cache so that we know that we can handle cached shm objects
with confidence later in the .shutdown hook.
Signed-off-by: Tyler Hicks <[email protected]>
---
drivers/tee/optee/call.c | 11 ++++++++++-
drivers/tee/optee/core.c | 13 +++++++++++--
drivers/tee/optee/optee_private.h | 2 +-
3 files changed, 22 insertions(+), 4 deletions(-)
diff --git a/drivers/tee/optee/call.c b/drivers/tee/optee/call.c
index 6e6eb836e9b6..5dcba6105ed7 100644
--- a/drivers/tee/optee/call.c
+++ b/drivers/tee/optee/call.c
@@ -419,8 +419,10 @@ void optee_enable_shm_cache(struct optee *optee)
* optee_disable_shm_cache() - Disables caching of some shared memory allocation
* in OP-TEE
* @optee: main service struct
+ * @is_mapped: true if the cached shared memory addresses were mapped by this
+ * kernel, are safe to dereference, and should be freed
*/
-void optee_disable_shm_cache(struct optee *optee)
+void optee_disable_shm_cache(struct optee *optee, bool is_mapped)
{
struct optee_call_waiter w;
@@ -439,6 +441,13 @@ void optee_disable_shm_cache(struct optee *optee)
if (res.result.status == OPTEE_SMC_RETURN_OK) {
struct tee_shm *shm;
+ /*
+ * Shared memory references that were not mapped by
+ * this kernel must be ignored to prevent a crash.
+ */
+ if (!is_mapped)
+ continue;
+
shm = reg_pair_to_ptr(res.result.shm_upper32,
res.result.shm_lower32);
tee_shm_free(shm);
diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
index 0987074d7ed0..6974e1104bd4 100644
--- a/drivers/tee/optee/core.c
+++ b/drivers/tee/optee/core.c
@@ -589,7 +589,7 @@ static int optee_remove(struct platform_device *pdev)
* reference counters and also avoid wild pointers in secure world
* into the old shared memory range.
*/
- optee_disable_shm_cache(optee);
+ optee_disable_shm_cache(optee, true);
/*
* The two devices have to be unregistered before we can free the
@@ -619,7 +619,7 @@ static int optee_remove(struct platform_device *pdev)
*/
static void optee_shutdown(struct platform_device *pdev)
{
- optee_disable_shm_cache(platform_get_drvdata(pdev));
+ optee_disable_shm_cache(platform_get_drvdata(pdev), true);
}
static int optee_probe(struct platform_device *pdev)
@@ -716,6 +716,15 @@ static int optee_probe(struct platform_device *pdev)
optee->memremaped_shm = memremaped_shm;
optee->pool = pool;
+ /*
+ * Ensure that there are no pre-existing shm objects before enabling
+ * the shm cache so that there's no chance of receiving an invalid
+ * address during shutdown. This could occur, for example, if we're
+ * kexec booting from an older kernel that did not properly cleanup the
+ * shm cache.
+ */
+ optee_disable_shm_cache(optee, false);
+
optee_enable_shm_cache(optee);
if (optee->sec_caps & OPTEE_SMC_SEC_CAP_DYNAMIC_SHM)
diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h
index e25b216a14ef..16d8c82213e7 100644
--- a/drivers/tee/optee/optee_private.h
+++ b/drivers/tee/optee/optee_private.h
@@ -158,7 +158,7 @@ int optee_invoke_func(struct tee_context *ctx, struct tee_ioctl_invoke_arg *arg,
int optee_cancel_req(struct tee_context *ctx, u32 cancel_id, u32 session);
void optee_enable_shm_cache(struct optee *optee);
-void optee_disable_shm_cache(struct optee *optee);
+void optee_disable_shm_cache(struct optee *optee, bool is_mapped);
int optee_shm_register(struct tee_context *ctx, struct tee_shm *shm,
struct page **pages, size_t num_pages,
--
2.25.1