mbox series

[0/4] KVM/arm64: Enable PtrAuth on non-VHE KVM

Message ID 20200615081954.6233-1-maz@kernel.org (mailing list archive)
Headers show
Series KVM/arm64: Enable PtrAuth on non-VHE KVM | expand

Message

Marc Zyngier June 15, 2020, 8:19 a.m. UTC
Not having PtrAuth on non-VHE KVM (for whatever reason VHE is not
enabled on a v8.3 system) has always looked like an oddity. This
trivial series remedies it, and allows a non-VHE KVM to offer PtrAuth
to its guests.

In the tradition of not having separate security between host-EL1 and
EL2, EL2 reuses the keys set up by host-EL1. It is likely that, should
we switch to a mode where EL2 is more distrusting of EL1, we'd have
private keys there.

The last patch is just an optimisation which I've lobbed with the rest
of the series in order not to forget it.

Marc Zyngier (4):
  KVM: arm64: Enable Pointer Authentication at EL2 if available
  KVM: arm64: Allow ARM64_PTR_AUTH when ARM64_VHE=n
  KVM: arm64: Allow PtrAuth to be enabled from userspace on non-VHE
    systems
  KVM: arm64: Check HCR_EL2 instead of shadow copy to swap PtrAuth
    registers

 arch/arm64/Kconfig                   |  4 +---
 arch/arm64/include/asm/kvm_ptrauth.h |  4 ++--
 arch/arm64/kvm/hyp-init.S            | 11 +++++++++++
 arch/arm64/kvm/reset.c               | 21 ++++++++++-----------
 4 files changed, 24 insertions(+), 16 deletions(-)

Comments

Dave Martin June 15, 2020, 12:59 p.m. UTC | #1
On Mon, Jun 15, 2020 at 09:19:50AM +0100, Marc Zyngier wrote:
> Not having PtrAuth on non-VHE KVM (for whatever reason VHE is not
> enabled on a v8.3 system) has always looked like an oddity. This
> trivial series remedies it, and allows a non-VHE KVM to offer PtrAuth
> to its guests.

How likely do you think it is that people will use such a configuration?

The only reason I can see for people to build a kernel with CONFIG_VHE=n
is as a workaround for broken hardware, or because the kernel is too old
to support VHE (in which case it doesn't understand ptrauth either, so
it is irrelevant whether ptrauth depends on VHE).

I wonder whether it's therefore better to "encourage" people to turn
VHE on by making subsequent features depend on it where appropriate.
We do want multiplatform kernels to be configured with CONFIG_VHE=y for
example.


I ask this, because SVE suffers the same "oddity".  If SVE can be
enabled for non-VHE kernels straightforwardly then there's no reason not
to do so, but I worried in the past that this would duplicate complex
code that would never be tested or used.

If supporting ptrauth with !VHE is as simple as this series suggests,
then it's low-risk.  Perhaps SVE isn't much worse.  I was chasing nasty
bugs around at the time the SVE KVM support was originally written, and
didn't want to add more unknowns into the mix...

(Note, this is not an offer from me to do the SVE work!)

[...]

Cheers
---Dave
Marc Zyngier June 15, 2020, 1:22 p.m. UTC | #2
Hi Dave,

On 2020-06-15 13:59, Dave Martin wrote:
> On Mon, Jun 15, 2020 at 09:19:50AM +0100, Marc Zyngier wrote:
>> Not having PtrAuth on non-VHE KVM (for whatever reason VHE is not
>> enabled on a v8.3 system) has always looked like an oddity. This
>> trivial series remedies it, and allows a non-VHE KVM to offer PtrAuth
>> to its guests.
> 
> How likely do you think it is that people will use such a 
> configuration?

Depending on the use case, very. See below.

> The only reason I can see for people to build a kernel with 
> CONFIG_VHE=n
> is as a workaround for broken hardware, or because the kernel is too 
> old
> to support VHE (in which case it doesn't understand ptrauth either, so
> it is irrelevant whether ptrauth depends on VHE).

Part of the work happening around running protected VMs (which cannot
be tampered with from EL1/0 host) makes it mandatory to disable VHE,
so that we can wrap the host EL1 in its own Stage-2 page tables.
We (the Android kernel team) are actively working on enabling this
feature.

> I wonder whether it's therefore better to "encourage" people to turn
> VHE on by making subsequent features depend on it where appropriate.
> We do want multiplatform kernels to be configured with CONFIG_VHE=y for
> example.

I'm all for having VHE on for platforms that support it. Which is why
CONFIG_VHE=y is present in defconfig. However, we cannot offer the same
level of guarantee as we can hopefully achieve with non-VHE (we can
drop mappings from Stage-1, but can't protect VMs from an evil or
compromised host). This is a very different use case from the usual
"reduced hypervisor overhead" that we want in the general case.

> I ask this, because SVE suffers the same "oddity".  If SVE can be
> enabled for non-VHE kernels straightforwardly then there's no reason 
> not
> to do so, but I worried in the past that this would duplicate complex
> code that would never be tested or used.

It is a concern. I guess that if we manage to get some traction on
Android, then the feature will get some testing! And yes, SVE is
next on my list.

> If supporting ptrauth with !VHE is as simple as this series suggests,
> then it's low-risk.  Perhaps SVE isn't much worse.  I was chasing nasty
> bugs around at the time the SVE KVM support was originally written, and
> didn't want to add more unknowns into the mix...

I think having started with a slightly smaller problem space was the
right thing to do at the time. We are now reasonably confident that
KVM and SVE are working correctly together, and we can now try to enable
it on !VHE.

Thanks,

         M.
Dave Martin June 15, 2020, 2:17 p.m. UTC | #3
On Mon, Jun 15, 2020 at 02:22:19PM +0100, Marc Zyngier wrote:
> Hi Dave,
> 
> On 2020-06-15 13:59, Dave Martin wrote:
> >On Mon, Jun 15, 2020 at 09:19:50AM +0100, Marc Zyngier wrote:
> >>Not having PtrAuth on non-VHE KVM (for whatever reason VHE is not
> >>enabled on a v8.3 system) has always looked like an oddity. This
> >>trivial series remedies it, and allows a non-VHE KVM to offer PtrAuth
> >>to its guests.
> >
> >How likely do you think it is that people will use such a configuration?
> 
> Depending on the use case, very. See below.
> 
> >The only reason I can see for people to build a kernel with CONFIG_VHE=n
> >is as a workaround for broken hardware, or because the kernel is too old
> >to support VHE (in which case it doesn't understand ptrauth either, so
> >it is irrelevant whether ptrauth depends on VHE).
> 
> Part of the work happening around running protected VMs (which cannot
> be tampered with from EL1/0 host) makes it mandatory to disable VHE,
> so that we can wrap the host EL1 in its own Stage-2 page tables.
> We (the Android kernel team) are actively working on enabling this
> feature.
> 
> >I wonder whether it's therefore better to "encourage" people to turn
> >VHE on by making subsequent features depend on it where appropriate.
> >We do want multiplatform kernels to be configured with CONFIG_VHE=y for
> >example.
> 
> I'm all for having VHE on for platforms that support it. Which is why
> CONFIG_VHE=y is present in defconfig. However, we cannot offer the same
> level of guarantee as we can hopefully achieve with non-VHE (we can
> drop mappings from Stage-1, but can't protect VMs from an evil or
> compromised host). This is a very different use case from the usual
> "reduced hypervisor overhead" that we want in the general case.
> 
> >I ask this, because SVE suffers the same "oddity".  If SVE can be
> >enabled for non-VHE kernels straightforwardly then there's no reason not
> >to do so, but I worried in the past that this would duplicate complex
> >code that would never be tested or used.
> 
> It is a concern. I guess that if we manage to get some traction on
> Android, then the feature will get some testing! And yes, SVE is
> next on my list.
> 
> >If supporting ptrauth with !VHE is as simple as this series suggests,
> >then it's low-risk.  Perhaps SVE isn't much worse.  I was chasing nasty
> >bugs around at the time the SVE KVM support was originally written, and
> >didn't want to add more unknowns into the mix...
> 
> I think having started with a slightly smaller problem space was the
> right thing to do at the time. We are now reasonably confident that
> KVM and SVE are working correctly together, and we can now try to enable
> it on !VHE.

Cool, now I understand.

Cheers
---Dave