Hi all,
pKVM's primary goal is to protect guest pages from a compromised host by
enforcing access control restrictions using stage-2 page-tables. Sadly,
this cannot prevent TrustZone from accessing non-secure memory, and a
compromised host could, for example, perform a 'confused deputy' attack
by asking TrustZone to use pages that have been donated to protected
guests. This would effectively allow the host to have TrustZone
exfiltrate guest secrets on its behalf, hence breaking the isolation
that pKVM intends to provide.
This series addresses this problem by providing pKVM with the ability to
monitor SMCs following the Arm FF-A protocol. FF-A provides (among other
things) a set of memory management APIs allowing the Normal World to
share, donate or lend pages with Secure. By monitoring these SMCs, pKVM
can ensure that the pages that are shared, lent or donated to Secure by
the host kernel are only pages that it owns.
It should be noted that the robustness of this approach relies on having
all Secure Software on the device use the FF-A protocol for memory
management transactions with the normal world, and not use
vendor-specific SMCs that pKVM is unable to parse. This is an important
software requirement to integrate pKVM correctly on a device. For the
record, this work is tractable in real world scenarios -- it was done
for the Pixel 7 and Pixel 7 Pro devices, both of which launched recently
with pKVM enabled.
This series introduces support for monitoring FF-A SMCs in the form of a
minimal proxy running in the EL2 hypervisor. This proxy aims to be
entirely 'transparent' to both the host kernel and TrustZone when the
system behaves correctly (the host is not compromised). The FF-A proxy
intercepts the SMCs coming from the host, and verifies that the pages
involved in the memory transition can be shared/lent/donated legally
before forwarding the SMC to Secure. The proxy also tracks which pages
have been made accessible to TrustZone at any point in time using
software bits in the stage-2 page-table of the host, to ensure they
can't be subsequently donated to guests.
The patch series is divided as follows:
- patches 01-02 refactor existing FF-A header to allow code re-use;
- patches 03-07 provide the initial infrastructure at EL2 to handle
FF-A SMCs;
- patches 08-12 use the previously introduced infrastructure to
intercept the main memory management operations to share, reclaim and
lend memory to/with Secure, and implement the core of the memory
tracking logic.
Since this series depends on Will's recent pKVM series [1], it is based
on today's kvmarm/next:
eb8be68e907e ("Merge branch kvm-arm64/misc-6.2 into kvmarm-master/next")
A branch with all the goodies applied can also be found here:
https://android-kvm.googlesource.com/linux qperret/ffa-proxy
Feedback welcome!
Cheers,
Quentin
[1] https://lore.kernel.org/kvm/[email protected]/
Fuad Tabba (1):
KVM: arm64: Handle FFA_FEATURES call from the host
Quentin Perret (1):
ANDROID: KVM: arm64: pkvm: Add support for fragmented FF-A descriptors
Will Deacon (10):
firmware: arm_ffa: Move constants to header file
firmware: arm_ffa: Move comment before the field it is documenting
KVM: arm64: Block unsafe FF-A calls from the host
KVM: arm64: Probe FF-A version and host/hyp partition ID during init
KVM: arm64: Allocate pages for hypervisor FF-A mailboxes
KVM: arm64: Handle FFA_RXTX_MAP and FFA_RXTX_UNMAP calls from the host
KVM: arm64: Add FF-A helpers to share/unshare memory with secure world
KVM: arm64: Handle FFA_MEM_SHARE calls from the host
KVM: arm64: Handle FFA_MEM_RECLAIM calls from the host
KVM: arm64: Handle FFA_MEM_LEND calls from the host
arch/arm64/include/asm/kvm_host.h | 1 +
arch/arm64/include/asm/kvm_pkvm.h | 21 +
arch/arm64/kvm/arm.c | 1 +
arch/arm64/kvm/hyp/include/nvhe/ffa.h | 17 +
arch/arm64/kvm/hyp/include/nvhe/mem_protect.h | 3 +
arch/arm64/kvm/hyp/nvhe/Makefile | 2 +-
arch/arm64/kvm/hyp/nvhe/ffa.c | 741 ++++++++++++++++++
arch/arm64/kvm/hyp/nvhe/hyp-main.c | 3 +
arch/arm64/kvm/hyp/nvhe/mem_protect.c | 68 ++
arch/arm64/kvm/hyp/nvhe/setup.c | 11 +
arch/arm64/kvm/pkvm.c | 1 +
drivers/firmware/arm_ffa/driver.c | 101 +--
include/linux/arm_ffa.h | 93 ++-
13 files changed, 970 insertions(+), 93 deletions(-)
create mode 100644 arch/arm64/kvm/hyp/include/nvhe/ffa.h
create mode 100644 arch/arm64/kvm/hyp/nvhe/ffa.c
--
2.38.1.431.g37b22c650d-goog
From: Will Deacon <[email protected]>
When KVM is initialised in protected mode, we must take care to filter
certain FFA calls from the host kernel so that the integrity of guest
and hypervisor memory is maintained and is not made available to the
secure world.
As a first step, intercept and block all memory-related FF-A SMC calls
from the host to EL3. This puts the framework in place for handling them
properly.
Co-developed-by: Andrew Walbran <[email protected]>
Signed-off-by: Andrew Walbran <[email protected]>
Signed-off-by: Will Deacon <[email protected]>
Signed-off-by: Quentin Perret <[email protected]>
---
arch/arm64/kvm/hyp/include/nvhe/ffa.h | 16 ++++
arch/arm64/kvm/hyp/nvhe/Makefile | 2 +-
arch/arm64/kvm/hyp/nvhe/ffa.c | 113 ++++++++++++++++++++++++++
arch/arm64/kvm/hyp/nvhe/hyp-main.c | 3 +
4 files changed, 133 insertions(+), 1 deletion(-)
create mode 100644 arch/arm64/kvm/hyp/include/nvhe/ffa.h
create mode 100644 arch/arm64/kvm/hyp/nvhe/ffa.c
diff --git a/arch/arm64/kvm/hyp/include/nvhe/ffa.h b/arch/arm64/kvm/hyp/include/nvhe/ffa.h
new file mode 100644
index 000000000000..fc09ec671e24
--- /dev/null
+++ b/arch/arm64/kvm/hyp/include/nvhe/ffa.h
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2022 - Google LLC
+ * Author: Andrew Walbran <[email protected]>
+ */
+#ifndef __KVM_HYP_FFA_H
+#define __KVM_HYP_FFA_H
+
+#include <asm/kvm_host.h>
+
+#define FFA_MIN_FUNC_NUM 0x60
+#define FFA_MAX_FUNC_NUM 0x7F
+
+bool kvm_host_ffa_handler(struct kvm_cpu_context *host_ctxt);
+
+#endif /* __KVM_HYP_FFA_H */
diff --git a/arch/arm64/kvm/hyp/nvhe/Makefile b/arch/arm64/kvm/hyp/nvhe/Makefile
index be0a2bc3e20d..1b34d3ff57f3 100644
--- a/arch/arm64/kvm/hyp/nvhe/Makefile
+++ b/arch/arm64/kvm/hyp/nvhe/Makefile
@@ -22,7 +22,7 @@ lib-objs := $(addprefix ../../../lib/, $(lib-objs))
hyp-obj-y := timer-sr.o sysreg-sr.o debug-sr.o switch.o tlb.o hyp-init.o host.o \
hyp-main.o hyp-smp.o psci-relay.o early_alloc.o page_alloc.o \
- cache.o setup.o mm.o mem_protect.o sys_regs.o pkvm.o stacktrace.o
+ cache.o setup.o mm.o mem_protect.o sys_regs.o pkvm.o stacktrace.o ffa.o
hyp-obj-y += ../vgic-v3-sr.o ../aarch32.o ../vgic-v2-cpuif-proxy.o ../entry.o \
../fpsimd.o ../hyp-entry.o ../exception.o ../pgtable.o
hyp-obj-$(CONFIG_DEBUG_LIST) += list_debug.o
diff --git a/arch/arm64/kvm/hyp/nvhe/ffa.c b/arch/arm64/kvm/hyp/nvhe/ffa.c
new file mode 100644
index 000000000000..6ccf935d3b41
--- /dev/null
+++ b/arch/arm64/kvm/hyp/nvhe/ffa.c
@@ -0,0 +1,113 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * FF-A v1.0 proxy to filter out invalid memory-sharing SMC calls issued by
+ * the host. FF-A is a slightly more palatable abbreviation of "Arm Firmware
+ * Framework for Arm A-profile", which is specified by Arm in document
+ * number DEN0077.
+ *
+ * Copyright (C) 2022 - Google LLC
+ * Author: Andrew Walbran <[email protected]>
+ *
+ * This driver hooks into the SMC trapping logic for the host and intercepts
+ * all calls falling within the FF-A range. Each call is either:
+ *
+ * - Forwarded on unmodified to the SPMD at EL3
+ * - Rejected as "unsupported"
+ * - Accompanied by a host stage-2 page-table check/update and reissued
+ *
+ * Consequently, any attempts by the host to make guest memory pages
+ * accessible to the secure world using FF-A will be detected either here
+ * (in the case that the memory is already owned by the guest) or during
+ * donation to the guest (in the case that the memory was previously shared
+ * with the secure world).
+ *
+ * To allow the rolling-back of page-table updates and FF-A calls in the
+ * event of failure, operations involving the RXTX buffers are locked for
+ * the duration and are therefore serialised.
+ */
+
+#include <linux/arm-smccc.h>
+#include <linux/arm_ffa.h>
+#include <nvhe/ffa.h>
+#include <nvhe/trap_handler.h>
+
+static void ffa_to_smccc_error(struct arm_smccc_res *res, u64 ffa_errno)
+{
+ *res = (struct arm_smccc_res) {
+ .a0 = FFA_ERROR,
+ .a2 = ffa_errno,
+ };
+}
+
+static void ffa_set_retval(struct kvm_cpu_context *ctxt,
+ struct arm_smccc_res *res)
+{
+ cpu_reg(ctxt, 0) = res->a0;
+ cpu_reg(ctxt, 1) = res->a1;
+ cpu_reg(ctxt, 2) = res->a2;
+ cpu_reg(ctxt, 3) = res->a3;
+}
+
+static bool is_ffa_call(u64 func_id)
+{
+ return ARM_SMCCC_IS_FAST_CALL(func_id) &&
+ ARM_SMCCC_OWNER_NUM(func_id) == ARM_SMCCC_OWNER_STANDARD &&
+ ARM_SMCCC_FUNC_NUM(func_id) >= FFA_MIN_FUNC_NUM &&
+ ARM_SMCCC_FUNC_NUM(func_id) <= FFA_MAX_FUNC_NUM;
+}
+
+static bool ffa_call_unsupported(u64 func_id)
+{
+ switch (func_id) {
+ /* Unsupported memory management calls */
+ case FFA_FN64_MEM_RETRIEVE_REQ:
+ case FFA_MEM_RETRIEVE_RESP:
+ case FFA_MEM_RELINQUISH:
+ case FFA_MEM_OP_PAUSE:
+ case FFA_MEM_OP_RESUME:
+ case FFA_MEM_FRAG_RX:
+ case FFA_FN64_MEM_DONATE:
+ /* Indirect message passing via RX/TX buffers */
+ case FFA_MSG_SEND:
+ case FFA_MSG_POLL:
+ case FFA_MSG_WAIT:
+ /* 32-bit variants of 64-bit calls */
+ case FFA_MSG_SEND_DIRECT_REQ:
+ case FFA_MSG_SEND_DIRECT_RESP:
+ case FFA_RXTX_MAP:
+ case FFA_MEM_DONATE:
+ case FFA_MEM_RETRIEVE_REQ:
+ return true;
+ }
+
+ return false;
+}
+
+bool kvm_host_ffa_handler(struct kvm_cpu_context *host_ctxt)
+{
+ DECLARE_REG(u64, func_id, host_ctxt, 0);
+ struct arm_smccc_res res;
+
+ if (!is_ffa_call(func_id))
+ return false;
+
+ switch (func_id) {
+ /* Memory management */
+ case FFA_FN64_RXTX_MAP:
+ case FFA_RXTX_UNMAP:
+ case FFA_MEM_SHARE:
+ case FFA_FN64_MEM_SHARE:
+ case FFA_MEM_LEND:
+ case FFA_FN64_MEM_LEND:
+ case FFA_MEM_RECLAIM:
+ case FFA_MEM_FRAG_TX:
+ break;
+ }
+
+ if (!ffa_call_unsupported(func_id))
+ return false; /* Pass through */
+
+ ffa_to_smccc_error(&res, FFA_RET_NOT_SUPPORTED);
+ ffa_set_retval(host_ctxt, &res);
+ return true;
+}
diff --git a/arch/arm64/kvm/hyp/nvhe/hyp-main.c b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
index 728e01d4536b..223611e43279 100644
--- a/arch/arm64/kvm/hyp/nvhe/hyp-main.c
+++ b/arch/arm64/kvm/hyp/nvhe/hyp-main.c
@@ -13,6 +13,7 @@
#include <asm/kvm_hyp.h>
#include <asm/kvm_mmu.h>
+#include <nvhe/ffa.h>
#include <nvhe/mem_protect.h>
#include <nvhe/mm.h>
#include <nvhe/pkvm.h>
@@ -373,6 +374,8 @@ static void handle_host_smc(struct kvm_cpu_context *host_ctxt)
bool handled;
handled = kvm_host_psci_handler(host_ctxt);
+ if (!handled)
+ handled = kvm_host_ffa_handler(host_ctxt);
if (!handled)
default_host_smc_handler(host_ctxt);
--
2.38.1.431.g37b22c650d-goog
Sorry, hit send a bit too early. Reviewing the patch itself:
On Wed, Nov 16, 2022 at 05:03:26PM +0000, Quentin Perret wrote:
[...]
> +static bool ffa_call_unsupported(u64 func_id)
> +{
> + switch (func_id) {
> + /* Unsupported memory management calls */
> + case FFA_FN64_MEM_RETRIEVE_REQ:
> + case FFA_MEM_RETRIEVE_RESP:
> + case FFA_MEM_RELINQUISH:
> + case FFA_MEM_OP_PAUSE:
> + case FFA_MEM_OP_RESUME:
> + case FFA_MEM_FRAG_RX:
> + case FFA_FN64_MEM_DONATE:
> + /* Indirect message passing via RX/TX buffers */
> + case FFA_MSG_SEND:
> + case FFA_MSG_POLL:
> + case FFA_MSG_WAIT:
> + /* 32-bit variants of 64-bit calls */
> + case FFA_MSG_SEND_DIRECT_REQ:
> + case FFA_MSG_SEND_DIRECT_RESP:
> + case FFA_RXTX_MAP:
> + case FFA_MEM_DONATE:
> + case FFA_MEM_RETRIEVE_REQ:
> + return true;
> + }
> +
> + return false;
> +}
Wouldn't an allowlist behave better in this case? While unlikely, you
wouldn't want EL3 implementing some FFA_BACKDOOR_PVM SMC that falls
outside of the denylist and is passed through.
> +bool kvm_host_ffa_handler(struct kvm_cpu_context *host_ctxt)
> +{
> + DECLARE_REG(u64, func_id, host_ctxt, 0);
> + struct arm_smccc_res res;
> +
> + if (!is_ffa_call(func_id))
> + return false;
> +
> + switch (func_id) {
> + /* Memory management */
> + case FFA_FN64_RXTX_MAP:
> + case FFA_RXTX_UNMAP:
> + case FFA_MEM_SHARE:
> + case FFA_FN64_MEM_SHARE:
> + case FFA_MEM_LEND:
> + case FFA_FN64_MEM_LEND:
> + case FFA_MEM_RECLAIM:
> + case FFA_MEM_FRAG_TX:
> + break;
> + }
What is the purpose of this switch?
> +
> + if (!ffa_call_unsupported(func_id))
> + return false; /* Pass through */
Another (tiny) benefit of implementing an allowlist is that it avoids
the use of double-negative logic like this.
--
Thanks,
Oliver
From: Will Deacon <[email protected]>
Handle FFA_MEM_LEND calls from the host by treating them identically to
FFA_MEM_SHARE calls for the purposes of the host stage-2 page-table, but
forwarding on the original request to EL3.
Signed-off-by: Will Deacon <[email protected]>
Signed-off-by: Quentin Perret <[email protected]>
---
arch/arm64/kvm/hyp/nvhe/ffa.c | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/arch/arm64/kvm/hyp/nvhe/ffa.c b/arch/arm64/kvm/hyp/nvhe/ffa.c
index 8f690b80af60..84024cba12ff 100644
--- a/arch/arm64/kvm/hyp/nvhe/ffa.c
+++ b/arch/arm64/kvm/hyp/nvhe/ffa.c
@@ -122,10 +122,10 @@ static int spmd_unmap_ffa_buffers(void)
return res.a0 == FFA_SUCCESS ? FFA_RET_SUCCESS : res.a2;
}
-static void spmd_mem_share(struct arm_smccc_res *res, u32 len, u32 fraglen)
+static void spmd_mem_xfer(struct arm_smccc_res *res, u64 func_id, u32 len,
+ u32 fraglen)
{
- arm_smccc_1_1_smc(FFA_FN64_MEM_SHARE,
- len, fraglen,
+ arm_smccc_1_1_smc(func_id, len, fraglen,
0, 0, 0, 0, 0,
res);
}
@@ -323,8 +323,9 @@ static int ffa_host_unshare_ranges(struct ffa_mem_region_addr_range *ranges,
return ret;
}
-static void do_ffa_mem_share(struct arm_smccc_res *res,
- struct kvm_cpu_context *ctxt)
+static __always_inline void do_ffa_mem_xfer(const u64 func_id,
+ struct arm_smccc_res *res,
+ struct kvm_cpu_context *ctxt)
{
DECLARE_REG(u32, len, ctxt, 1);
DECLARE_REG(u32, fraglen, ctxt, 2);
@@ -335,6 +336,9 @@ static void do_ffa_mem_share(struct arm_smccc_res *res,
int ret = 0;
u32 offset;
+ BUILD_BUG_ON(func_id != FFA_FN64_MEM_SHARE &&
+ func_id != FFA_FN64_MEM_LEND);
+
if (addr_mbz || npages_mbz || fraglen > len ||
fraglen > KVM_FFA_MBOX_NR_PAGES * PAGE_SIZE) {
ret = FFA_RET_INVALID_PARAMETERS;
@@ -384,7 +388,7 @@ static void do_ffa_mem_share(struct arm_smccc_res *res,
if (ret)
goto out_unlock;
- spmd_mem_share(res, len, fraglen);
+ spmd_mem_xfer(res, func_id, len, fraglen);
if (res->a0 != FFA_SUCCESS) {
WARN_ON(ffa_host_unshare_ranges(reg->constituents,
reg->addr_range_cnt));
@@ -535,13 +539,15 @@ bool kvm_host_ffa_handler(struct kvm_cpu_context *host_ctxt)
goto out_handled;
case FFA_MEM_SHARE:
case FFA_FN64_MEM_SHARE:
- do_ffa_mem_share(&res, host_ctxt);
+ do_ffa_mem_xfer(FFA_FN64_MEM_SHARE, &res, host_ctxt);
goto out_handled;
case FFA_MEM_RECLAIM:
do_ffa_mem_reclaim(&res, host_ctxt);
goto out_handled;
case FFA_MEM_LEND:
case FFA_FN64_MEM_LEND:
+ do_ffa_mem_xfer(FFA_FN64_MEM_LEND, &res, host_ctxt);
+ goto out_handled;
case FFA_MEM_FRAG_TX:
break;
}
--
2.38.1.431.g37b22c650d-goog
On Wed, 16 Nov 2022 17:03:23 +0000, Quentin Perret wrote:
> pKVM's primary goal is to protect guest pages from a compromised host by
> enforcing access control restrictions using stage-2 page-tables. Sadly,
> this cannot prevent TrustZone from accessing non-secure memory, and a
> compromised host could, for example, perform a 'confused deputy' attack
> by asking TrustZone to use pages that have been donated to protected
> guests. This would effectively allow the host to have TrustZone
> exfiltrate guest secrets on its behalf, hence breaking the isolation
> that pKVM intends to provide.
>
> [...]
Applied first two cleanups to arm64 (for-next/ffa), thanks!
[01/12] firmware: arm_ffa: Move constants to header file
https://git.kernel.org/arm64/c/229d58e31678
[02/12] firmware: arm_ffa: Move comment before the field it is documenting
https://git.kernel.org/arm64/c/c8e320b00a2a
Cheers,
--
Will
https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev
On Wed, Nov 16, 2022 at 05:48:42PM +0000, Oliver Upton wrote:
> Sorry, hit send a bit too early. Reviewing the patch itself:
>
> On Wed, Nov 16, 2022 at 05:03:26PM +0000, Quentin Perret wrote:
>
> [...]
>
> > +static bool ffa_call_unsupported(u64 func_id)
> > +{
> > + switch (func_id) {
> > + /* Unsupported memory management calls */
> > + case FFA_FN64_MEM_RETRIEVE_REQ:
> > + case FFA_MEM_RETRIEVE_RESP:
> > + case FFA_MEM_RELINQUISH:
> > + case FFA_MEM_OP_PAUSE:
> > + case FFA_MEM_OP_RESUME:
> > + case FFA_MEM_FRAG_RX:
> > + case FFA_FN64_MEM_DONATE:
> > + /* Indirect message passing via RX/TX buffers */
> > + case FFA_MSG_SEND:
> > + case FFA_MSG_POLL:
> > + case FFA_MSG_WAIT:
> > + /* 32-bit variants of 64-bit calls */
> > + case FFA_MSG_SEND_DIRECT_REQ:
> > + case FFA_MSG_SEND_DIRECT_RESP:
> > + case FFA_RXTX_MAP:
> > + case FFA_MEM_DONATE:
> > + case FFA_MEM_RETRIEVE_REQ:
> > + return true;
> > + }
> > +
> > + return false;
> > +}
>
> Wouldn't an allowlist behave better in this case? While unlikely, you
> wouldn't want EL3 implementing some FFA_BACKDOOR_PVM SMC that falls
> outside of the denylist and is passed through.
Given that we're not intercepting all SMCs (rather, only those in the
FF-A service range), I think the denylist works much better because the
default action is "allow". We _have_ to trust EL3 regardless, as it
could just use any allowed SMC for a backdoor if it wanted. Ultimately,
EL3 runs the show in the hierarchical security model of the architecture
and we sadly can't do much about that.
> > +bool kvm_host_ffa_handler(struct kvm_cpu_context *host_ctxt)
> > +{
> > + DECLARE_REG(u64, func_id, host_ctxt, 0);
> > + struct arm_smccc_res res;
> > +
> > + if (!is_ffa_call(func_id))
> > + return false;
> > +
> > + switch (func_id) {
> > + /* Memory management */
> > + case FFA_FN64_RXTX_MAP:
> > + case FFA_RXTX_UNMAP:
> > + case FFA_MEM_SHARE:
> > + case FFA_FN64_MEM_SHARE:
> > + case FFA_MEM_LEND:
> > + case FFA_FN64_MEM_LEND:
> > + case FFA_MEM_RECLAIM:
> > + case FFA_MEM_FRAG_TX:
> > + break;
> > + }
>
> What is the purpose of this switch?
As of this patch, it's not serving any functional purpose. The idea is
that later patches hook in here to provide handling at EL2. I'll remove
it and introduce it bit by bit.
>
> > +
> > + if (!ffa_call_unsupported(func_id))
> > + return false; /* Pass through */
>
> Another (tiny) benefit of implementing an allowlist is that it avoids
> the use of double-negative logic like this.
I should just rework this to be ffa_call_supported().
Will