From: Will Deacon <will@kernel.org> Date: 2021-09-23 11:25:17
Hi folks,
This series restricts the hypercalls available to the KVM host on arm64
when pKVM is enabled so that it is not possible for the host to use them
to replace the EL2 component with something else.
This occurs in two stages: when switching to the pKVM vectors, the stub
hypercalls are removed and then later when pKVM is finalised, the pKVM
init hypercalls are removed.
There are still a few dubious calls remaining in terms of protecting the
guest (e.g. __kvm_adjust_pc) but these will be dealt with later when we
have more VM state at EL2 to play with.
Patches based on -rc2. Feedback welcome.
Cheers,
Will
Cc: Marc Zyngier <maz@kernel.org>
Cc: Quentin Perret <redacted>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Alexandru Elisei <redacted>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: kvmarm@lists.cs.columbia.edu
--->8
Will Deacon (5):
arm64: Prevent kexec and hibernation if is_protected_kvm_enabled()
KVM: arm64: Reject stub hypercalls after pKVM has been initialised
KVM: arm64: Propagate errors from __pkvm_prot_finalize hypercall
KVM: arm64: Prevent re-finalisation of pKVM for a given CPU
KVM: arm64: Disable privileged hypercalls after pKVM finalisation
arch/arm64/include/asm/kvm_asm.h | 43 ++++++++++---------
arch/arm64/kernel/smp.c | 3 +-
arch/arm64/kvm/arm.c | 61 ++++++++++++++++++---------
arch/arm64/kvm/hyp/nvhe/host.S | 26 ++++++++----
arch/arm64/kvm/hyp/nvhe/hyp-main.c | 26 +++++++-----
arch/arm64/kvm/hyp/nvhe/mem_protect.c | 3 ++
6 files changed, 103 insertions(+), 59 deletions(-)
--
2.33.0.464.g1972c5931b-goog
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Will Deacon <will@kernel.org> Date: 2021-09-23 11:25:25
When pKVM is enabled, the hypervisor code at EL2 and its data structures
are inaccessible to the host kernel and cannot be torn down or replaced
as this would defeat the integrity properies which pKVM aims to provide.
Furthermore, the ABI between the host and EL2 is flexible and private to
whatever the current implementation of KVM requires and so booting a new
kernel with an old EL2 component is very likely to end in disaster.
In preparation for uninstalling the hyp stub calls which are relied upon
to reset EL2, disable kexec and hibernation in the host when protected
KVM is enabled.
Cc: Marc Zyngier <maz@kernel.org>
Cc: Quentin Perret <redacted>
Signed-off-by: Will Deacon <will@kernel.org>
---
arch/arm64/kernel/smp.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
From: Will Deacon <will@kernel.org> Date: 2021-09-23 11:25:41
The stub hypercalls provide mechanisms to reset and replace the EL2 code,
so uninstall them once pKVM has been initialised in order to ensure the
integrity of the hypervisor code.
To ensure pKVM initialisation remains functional, split cpu_hyp_reinit()
into two helper functions to separate usage of the stub from usage of
pkvm hypercalls either side of __pkvm_init on the boot CPU.
Cc: Marc Zyngier <maz@kernel.org>
Cc: Quentin Perret <redacted>
Signed-off-by: Will Deacon <will@kernel.org>
---
arch/arm64/kvm/arm.c | 31 +++++++++++++++++++++++--------
arch/arm64/kvm/hyp/nvhe/host.S | 26 +++++++++++++++++---------
2 files changed, 40 insertions(+), 17 deletions(-)
From: Will Deacon <will@kernel.org> Date: 2021-09-23 11:25:47
If the __pkvm_prot_finalize hypercall returns an error, we WARN but fail
to propagate the failure code back to kvm_arch_init().
Pass a pointer to a zero-initialised return variable so that failure
to finalise the pKVM protections on a host CPU can be reported back to
KVM.
Cc: Marc Zyngier <maz@kernel.org>
Cc: Quentin Perret <redacted>
Signed-off-by: Will Deacon <will@kernel.org>
---
arch/arm64/kvm/arm.c | 30 +++++++++++++++++++-----------
1 file changed, 19 insertions(+), 11 deletions(-)
From: Will Deacon <will@kernel.org> Date: 2021-09-23 11:26:14
After pKVM has been 'finalised' using the __pkvm_prot_finalize hypercall,
the calling CPU will have a Stage-2 translation enabled to prevent access
to memory pages owned by EL2.
Although this forms a significant part of the process to deprivilege the
host kernel, we also need to ensure that the hypercall interface is
reduced so that the EL2 code cannot, for example, be re-initialised using
a new set of vectors.
Re-order the hypercalls so that only a suffix remains available after
finalisation of pKVM.
Cc: Marc Zyngier <maz@kernel.org>
Cc: Quentin Perret <redacted>
Signed-off-by: Will Deacon <will@kernel.org>
---
arch/arm64/include/asm/kvm_asm.h | 43 ++++++++++++++++--------------
arch/arm64/kvm/hyp/nvhe/hyp-main.c | 26 +++++++++++-------
2 files changed, 39 insertions(+), 30 deletions(-)
From: Will Deacon <will@kernel.org> Date: 2021-09-23 11:26:17
__pkvm_prot_finalize() completes the deprivilege of the host when pKVM
is in use by installing a stage-2 translation table for the calling CPU.
Issuing the hypercall multiple times for a given CPU makes little sense,
but in such a case just return early with -EPERM rather than go through
the whole page-table dance again.
Cc: Marc Zyngier <maz@kernel.org>
Cc: Quentin Perret <redacted>
Signed-off-by: Will Deacon <will@kernel.org>
---
arch/arm64/kvm/hyp/nvhe/mem_protect.c | 3 +++
1 file changed, 3 insertions(+)
From: Mark Rutland <mark.rutland@arm.com> Date: 2021-09-23 11:47:33
On Thu, Sep 23, 2021 at 12:22:52PM +0100, Will Deacon wrote:
quoted hunk
When pKVM is enabled, the hypervisor code at EL2 and its data structures
are inaccessible to the host kernel and cannot be torn down or replaced
as this would defeat the integrity properies which pKVM aims to provide.
Furthermore, the ABI between the host and EL2 is flexible and private to
whatever the current implementation of KVM requires and so booting a new
kernel with an old EL2 component is very likely to end in disaster.
In preparation for uninstalling the hyp stub calls which are relied upon
to reset EL2, disable kexec and hibernation in the host when protected
KVM is enabled.
Cc: Marc Zyngier <maz@kernel.org>
Cc: Quentin Perret <redacted>
Signed-off-by: Will Deacon <will@kernel.org>
---
arch/arm64/kernel/smp.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
IIUC you'll also need to do something to prevent kdump, since even with
CPUs stuck in the kernel that will try to do a kexec on the crashed CPU
and __cpu_soft_restart() won't be able to return to EL2.
You could fiddle with the BUG_ON() in machine_kexec() to die in this
case too.
Thanks,
Mark.
From: Marc Zyngier <maz@kernel.org> Date: 2021-09-23 12:33:01
On Thu, 23 Sep 2021 12:22:51 +0100,
Will Deacon [off-list ref] wrote:
Hi folks,
This series restricts the hypercalls available to the KVM host on arm64
when pKVM is enabled so that it is not possible for the host to use them
to replace the EL2 component with something else.
This occurs in two stages: when switching to the pKVM vectors, the stub
hypercalls are removed and then later when pKVM is finalised, the pKVM
init hypercalls are removed.
There are still a few dubious calls remaining in terms of protecting the
guest (e.g. __kvm_adjust_pc) but these will be dealt with later when we
have more VM state at EL2 to play with.
Yup. This particular one should have an equivalent at EL2 and pending
exceptions committed to the state before exiting to EL1.
M.
--
Without deviation from the norm, progress is not possible.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Will Deacon <will@kernel.org> Date: 2021-09-23 12:36:06
On Thu, Sep 23, 2021 at 12:45:06PM +0100, Mark Rutland wrote:
On Thu, Sep 23, 2021 at 12:22:52PM +0100, Will Deacon wrote:
quoted
When pKVM is enabled, the hypervisor code at EL2 and its data structures
are inaccessible to the host kernel and cannot be torn down or replaced
as this would defeat the integrity properies which pKVM aims to provide.
Furthermore, the ABI between the host and EL2 is flexible and private to
whatever the current implementation of KVM requires and so booting a new
kernel with an old EL2 component is very likely to end in disaster.
In preparation for uninstalling the hyp stub calls which are relied upon
to reset EL2, disable kexec and hibernation in the host when protected
KVM is enabled.
Cc: Marc Zyngier <maz@kernel.org>
Cc: Quentin Perret <redacted>
Signed-off-by: Will Deacon <will@kernel.org>
---
arch/arm64/kernel/smp.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
IIUC you'll also need to do something to prevent kdump, since even with
CPUs stuck in the kernel that will try to do a kexec on the crashed CPU
and __cpu_soft_restart() won't be able to return to EL2.
You could fiddle with the BUG_ON() in machine_kexec() to die in this
case too.
I wondered about that, and I'm happy to do it if you reckon it's better,
but if the host is crashing _anyway_ then I wasn't convinced it was worth
the effort. With the approach here, we'll WARN and then enter the kdump
kernel at EL1 which maybe might work sometimes possibly? I suppose if the
kdump kernel is careful about the memory it accesses, then it has a
fighting chance of doing something useful.
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Marc Zyngier <maz@kernel.org> Date: 2021-09-23 12:58:52
On Thu, 23 Sep 2021 12:22:56 +0100,
Will Deacon [off-list ref] wrote:
quoted hunk
After pKVM has been 'finalised' using the __pkvm_prot_finalize hypercall,
the calling CPU will have a Stage-2 translation enabled to prevent access
to memory pages owned by EL2.
Although this forms a significant part of the process to deprivilege the
host kernel, we also need to ensure that the hypercall interface is
reduced so that the EL2 code cannot, for example, be re-initialised using
a new set of vectors.
Re-order the hypercalls so that only a suffix remains available after
finalisation of pKVM.
Cc: Marc Zyngier <maz@kernel.org>
Cc: Quentin Perret <redacted>
Signed-off-by: Will Deacon <will@kernel.org>
---
arch/arm64/include/asm/kvm_asm.h | 43 ++++++++++++++++--------------
arch/arm64/kvm/hyp/nvhe/hyp-main.c | 26 +++++++++++-------
2 files changed, 39 insertions(+), 30 deletions(-)
So I can still issue a pkvm_prot_finalize after finalisation? Seems
odd. As hcall_min has to be inclusive, you probably want it to be set
to __KVM_HOST_SMCCC_FUNC___pkvm_host_share_hyp once protected.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Will Deacon <will@kernel.org> Date: 2021-09-23 12:59:59
On Thu, Sep 23, 2021 at 12:22:56PM +0100, Will Deacon wrote:
After pKVM has been 'finalised' using the __pkvm_prot_finalize hypercall,
the calling CPU will have a Stage-2 translation enabled to prevent access
to memory pages owned by EL2.
Although this forms a significant part of the process to deprivilege the
host kernel, we also need to ensure that the hypercall interface is
reduced so that the EL2 code cannot, for example, be re-initialised using
a new set of vectors.
Re-order the hypercalls so that only a suffix remains available after
finalisation of pKVM.
Cc: Marc Zyngier <maz@kernel.org>
Cc: Quentin Perret <redacted>
Signed-off-by: Will Deacon <will@kernel.org>
---
arch/arm64/include/asm/kvm_asm.h | 43 ++++++++++++++++--------------
arch/arm64/kvm/hyp/nvhe/hyp-main.c | 26 +++++++++++-------
2 files changed, 39 insertions(+), 30 deletions(-)
Not that it makes any functional difference, but I was trying to keep this
in numerical order and evidently didn't manage it after renumbering
__vgic_v3_get_gic_config. Will fix for v2.
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Will Deacon <will@kernel.org> Date: 2021-09-23 13:04:51
On Thu, Sep 23, 2021 at 01:56:21PM +0100, Marc Zyngier wrote:
On Thu, 23 Sep 2021 12:22:56 +0100,
Will Deacon [off-list ref] wrote:
quoted
After pKVM has been 'finalised' using the __pkvm_prot_finalize hypercall,
the calling CPU will have a Stage-2 translation enabled to prevent access
to memory pages owned by EL2.
Although this forms a significant part of the process to deprivilege the
host kernel, we also need to ensure that the hypercall interface is
reduced so that the EL2 code cannot, for example, be re-initialised using
a new set of vectors.
Re-order the hypercalls so that only a suffix remains available after
finalisation of pKVM.
Cc: Marc Zyngier <maz@kernel.org>
Cc: Quentin Perret <redacted>
Signed-off-by: Will Deacon <will@kernel.org>
---
arch/arm64/include/asm/kvm_asm.h | 43 ++++++++++++++++--------------
arch/arm64/kvm/hyp/nvhe/hyp-main.c | 26 +++++++++++-------
2 files changed, 39 insertions(+), 30 deletions(-)
So I can still issue a pkvm_prot_finalize after finalisation? Seems
odd. As hcall_min has to be inclusive, you probably want it to be set
to __KVM_HOST_SMCCC_FUNC___pkvm_host_share_hyp once protected.
Yeah, I ended up addresing that one in the previous patch. The problem is
that we need to allow pkvm_prot_finalize to be called on each CPU, so I
think we'd end up having an extra "really finalize damnit!" call to be
issued _once_ after each CPU is done with the finalisation if we want
to lock it down.
The approach I took instead is to make pkvm_prot_finalize return -EBUSY
if it's called on a CPU where it's already been called.
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Marc Zyngier <maz@kernel.org> Date: 2021-09-23 13:13:17
On Thu, 23 Sep 2021 14:02:11 +0100,
Will Deacon [off-list ref] wrote:
On Thu, Sep 23, 2021 at 01:56:21PM +0100, Marc Zyngier wrote:
quoted
On Thu, 23 Sep 2021 12:22:56 +0100,
Will Deacon [off-list ref] wrote:
[...]
quoted
quoted
static void handle_host_hcall(struct kvm_cpu_context *host_ctxt) { DECLARE_REG(unsigned long, id, host_ctxt, 0);+ unsigned long hcall_min = 0; hcall_t hfn;+ if (static_branch_unlikely(&kvm_protected_mode_initialized))+ hcall_min = __KVM_HOST_SMCCC_FUNC___pkvm_prot_finalize;+ id -= KVM_HOST_SMCCC_ID(0);- if (unlikely(id >= ARRAY_SIZE(host_hcall)))+ if (unlikely(id < hcall_min || id >= ARRAY_SIZE(host_hcall)))
So I can still issue a pkvm_prot_finalize after finalisation? Seems
odd. As hcall_min has to be inclusive, you probably want it to be set
to __KVM_HOST_SMCCC_FUNC___pkvm_host_share_hyp once protected.
Yeah, I ended up addresing that one in the previous patch. The problem is
that we need to allow pkvm_prot_finalize to be called on each CPU, so I
think we'd end up having an extra "really finalize damnit!" call to be
issued _once_ after each CPU is done with the finalisation if we want
to lock it down.
The approach I took instead is to make pkvm_prot_finalize return -EBUSY
if it's called on a CPU where it's already been called.
Ah, I see. Serves me right for reading patches out of order. Finalise
is of course per-CPU, and the static key global. Epic fail.
Probably deserves a comment, because I'm surely going to jump at that
again in three months.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Thursday 23 Sep 2021 at 12:22:54 (+0100), Will Deacon wrote:
quoted hunk
If the __pkvm_prot_finalize hypercall returns an error, we WARN but fail
to propagate the failure code back to kvm_arch_init().
Pass a pointer to a zero-initialised return variable so that failure
to finalise the pKVM protections on a host CPU can be reported back to
KVM.
Cc: Marc Zyngier <maz@kernel.org>
Cc: Quentin Perret <redacted>
Signed-off-by: Will Deacon <will@kernel.org>
---
arch/arm64/kvm/arm.c | 30 +++++++++++++++++++-----------
1 file changed, 19 insertions(+), 11 deletions(-)
@@ -1986,9 +1986,25 @@ static int init_hyp_mode(void)returnerr;}-staticvoid_kvm_host_prot_finalize(void*discard)+staticvoid_kvm_host_prot_finalize(void*arg){-WARN_ON(kvm_call_hyp_nvhe(__pkvm_prot_finalize));+int*err=arg;++if(WARN_ON(kvm_call_hyp_nvhe(__pkvm_prot_finalize)))+WRITE_ONCE(*err,-EINVAL);+}
I was going to suggest to propagate the hypercall's error code directly,
but this becomes very racy so n/m...
But this got me thinking about what we should do when the hyp init fails
while the protected mode has been explicitly enabled on the kernel
cmdline. That is, if we continue and boot the kernel w/o KVM support,
then I don't know how e.g. EL3 can know that it shouldn't give keys to
VMs because the kernel (and EL2) can't be trusted. It feels like it is
the kernel's responsibility to do something while it _is_ still
trustworthy.
I guess we could make any error code fatal in kvm_arch_init() when
is_protected_kvm_enabled() is on, or something along those lines? Maybe
dependent on CONFIG_NVHE_EL2_DEBUG=n?
It's probably a bit theoretical because there really shouldn't be any
reason to fail hyp init in production when using a signed kernel image
etc etc, but then if that is the case the additional check I'm
suggesting shouldn't hurt and will give us some peace of mind. Thoughts?
Thanks,
Quentin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Thursday 23 Sep 2021 at 12:22:53 (+0100), Will Deacon wrote:
The stub hypercalls provide mechanisms to reset and replace the EL2 code,
so uninstall them once pKVM has been initialised in order to ensure the
integrity of the hypervisor code.
To ensure pKVM initialisation remains functional, split cpu_hyp_reinit()
into two helper functions to separate usage of the stub from usage of
pkvm hypercalls either side of __pkvm_init on the boot CPU.
Cc: Marc Zyngier <maz@kernel.org>
Cc: Quentin Perret <redacted>
Signed-off-by: Will Deacon <will@kernel.org>
On Thursday 23 Sep 2021 at 12:22:55 (+0100), Will Deacon wrote:
quoted hunk
__pkvm_prot_finalize() completes the deprivilege of the host when pKVM
is in use by installing a stage-2 translation table for the calling CPU.
Issuing the hypercall multiple times for a given CPU makes little sense,
but in such a case just return early with -EPERM rather than go through
the whole page-table dance again.
Cc: Marc Zyngier <maz@kernel.org>
Cc: Quentin Perret <redacted>
Signed-off-by: Will Deacon <will@kernel.org>
---
arch/arm64/kvm/hyp/nvhe/mem_protect.c | 3 +++
1 file changed, 3 insertions(+)
@@ -123,6 +123,9 @@ int __pkvm_prot_finalize(void)structkvm_s2_mmu*mmu=&host_kvm.arch.mmu;structkvm_nvhe_init_params*params=this_cpu_ptr(&kvm_init_params);+if(params->hcr_el2&HCR_VM)+return-EPERM;
And you check this rather than the static key because we flip it upfront
I guess. Makes sense to me, but maybe a little comment would be useful :)
In any case:
Reviewed-by: Quentin Perret <redacted>
Thanks,
Quentin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
From: Will Deacon <will@kernel.org> Date: 2021-10-05 11:32:44
Hey Quentin,
On Wed, Sep 29, 2021 at 02:36:47PM +0100, Quentin Perret wrote:
On Thursday 23 Sep 2021 at 12:22:54 (+0100), Will Deacon wrote:
quoted
If the __pkvm_prot_finalize hypercall returns an error, we WARN but fail
to propagate the failure code back to kvm_arch_init().
Pass a pointer to a zero-initialised return variable so that failure
to finalise the pKVM protections on a host CPU can be reported back to
KVM.
Cc: Marc Zyngier <maz@kernel.org>
Cc: Quentin Perret <redacted>
Signed-off-by: Will Deacon <will@kernel.org>
---
arch/arm64/kvm/arm.c | 30 +++++++++++++++++++-----------
1 file changed, 19 insertions(+), 11 deletions(-)
@@ -1986,9 +1986,25 @@ static int init_hyp_mode(void)returnerr;}-staticvoid_kvm_host_prot_finalize(void*discard)+staticvoid_kvm_host_prot_finalize(void*arg){-WARN_ON(kvm_call_hyp_nvhe(__pkvm_prot_finalize));+int*err=arg;++if(WARN_ON(kvm_call_hyp_nvhe(__pkvm_prot_finalize)))+WRITE_ONCE(*err,-EINVAL);+}
I was going to suggest to propagate the hypercall's error code directly,
but this becomes very racy so n/m...
But this got me thinking about what we should do when the hyp init fails
while the protected mode has been explicitly enabled on the kernel
cmdline. That is, if we continue and boot the kernel w/o KVM support,
then I don't know how e.g. EL3 can know that it shouldn't give keys to
VMs because the kernel (and EL2) can't be trusted. It feels like it is
the kernel's responsibility to do something while it _is_ still
trustworthy.
I guess we could make any error code fatal in kvm_arch_init() when
is_protected_kvm_enabled() is on, or something along those lines? Maybe
dependent on CONFIG_NVHE_EL2_DEBUG=n?
It's probably a bit theoretical because there really shouldn't be any
reason to fail hyp init in production when using a signed kernel image
etc etc, but then if that is the case the additional check I'm
suggesting shouldn't hurt and will give us some peace of mind. Thoughts?
It's an interesting one.
I'm not hugely keen on crashing the system if we fail to deprivilege the
host (which I think is effectively what is happening in the case you
describe), but you're right that we need to disable pKVM somehow in this
case. I think the best thing would be to wipe the pvmfw memory; that would
mean that the host can do whatever it likes at EL2, as the keys will no
longer be available.
I'll make a note about this, since I've parked the pvmfw patches until
we've got more of the pKVM infrastructure up and running.
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel