Re: [RFC 00/11] KVM, EFI, arm64: EFI Runtime Services Sandboxing

[Christoffer Dall]

On Tue, Sep 26, 2017 at 11:14:52PM +0200, Florent Revest wrote:
> On Thu, 2017-08-31 at 11:26 +0200, Christoffer Dall wrote:
> > I wonder if this should be split into two series; one that sets up
> > anything you may need from KVM, and another one that uses that for
> > UEFI.
> > 
> > There's a lot KVM and UEFI intertwined logic and assumptions in patch
> > 10, which makes this series a bit hard to read.
> 
> The way hypercalls are currently handled in handle_hvc required this
> mixed patch. Would some kind of HVC subscription mechanism be suitable
> to have in KVM? (e.g: a function allowing to register a callback on a
> certain HVC function ID) This would allow the 10/11 patch to keep the
> kvm code intact.

Yes, I would have no objections to that if it were relatively
non-invasive at runtime for normal VMs.

> 
> > I'd like some documentation (in the series and in
> > Documentation/virtual/kvm) of how this works, and which hidden
> > assumptions there are. For example, how do you ensure you never
> > attempt to return to userspace?
> 
> I don't think my code ensured this. I'd need to give it a second look.
> 
> >  How many VCPUs do you support?
> 
> You can create as many VCPUs as you would in a "normal VM". Also, each
> VCPU can be ran in a kthread.
> 
> >  Do you support any form of virtual interrupts? How about timers?
> 
> No support for virtual interrupts or timers indeed. The EFI Runtime
> Services sandboxing wouldn't require that.
> 
> > Can a VM access physical devices?
> 
> The very idea of Runtime Services sandboxing requires Internal VMs to
> have access to some of the physical devices.
> 
> >  How do you debug and trace something like this? Can the VM be
> > monitored from userspace?
> 
> There is nothing ready for that.
> 
> > These feel like fundamental questions to me that needs addressing
> > before I can competently review the code.
> > 
> > I think a slightly more concrete motivation and outlining the example
> > of the broken UEFI on Seattle would help paving the way for these
> > patches.
> 
> As far as I can remember, EFI Runtime Services on this platform have
> already been reported to sometimes disable or enable interrupts. Maybe
> someone at ARM has more details about the problem ?
> 

Thanks for answering these questions.  If you or anyone else picks up
this work, we can gather some of the stuff in the thread for
documentation and todo items.

Thanks,
-Christoffer
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [RFC 00/11] KVM, EFI, arm64: EFI Runtime Services Sandboxing

[Florent Revest]

On Thu, 2017-08-31 at 11:26 +0200, Christoffer Dall wrote:
> I wonder if this should be split into two series; one that sets up
> anything you may need from KVM, and another one that uses that for
> UEFI.
> 
> There's a lot KVM and UEFI intertwined logic and assumptions in patch
> 10, which makes this series a bit hard to read.

The way hypercalls are currently handled in handle_hvc required this
mixed patch. Would some kind of HVC subscription mechanism be suitable
to have in KVM? (e.g: a function allowing to register a callback on a
certain HVC function ID) This would allow the 10/11 patch to keep the
kvm code intact.

> I'd like some documentation (in the series and in
> Documentation/virtual/kvm) of how this works, and which hidden
> assumptions there are. For example, how do you ensure you never
> attempt to return to userspace?

I don't think my code ensured this. I'd need to give it a second look.

>  How many VCPUs do you support?

You can create as many VCPUs as you would in a "normal VM". Also, each
VCPU can be ran in a kthread.

>  Do you support any form of virtual interrupts? How about timers?

No support for virtual interrupts or timers indeed. The EFI Runtime
Services sandboxing wouldn't require that.

> Can a VM access physical devices?

The very idea of Runtime Services sandboxing requires Internal VMs to
have access to some of the physical devices.

>  How do you debug and trace something like this? Can the VM be
> monitored from userspace?

There is nothing ready for that.

> These feel like fundamental questions to me that needs addressing
> before I can competently review the code.
> 
> I think a slightly more concrete motivation and outlining the example
> of the broken UEFI on Seattle would help paving the way for these
> patches.

As far as I can remember, EFI Runtime Services on this platform have
already been reported to sometimes disable or enable interrupts. Maybe
someone at ARM has more details about the problem ?

Thanks a lot for your review,
    Florent
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [RFC 00/11] KVM, EFI, arm64: EFI Runtime Services Sandboxing

[Florent Revest]

On Fri, 2017-09-22 at 14:44 -0700, Ard Biesheuvel wrote:
> From the EFI side, there are some minor concerns on my part regarding
> the calling convention, and the fact that we can no longer invoke
> runtime services from a kernel running at EL1, but those all seem
> fixable. I will respond to the patches in question in greater detail
> at a later time.

Indeed, this RFC currently breaks EFI Runtime Services at EL1. This
would need to be fixed in a new patchset.

The patch 10/11 also underlines that the current argument passing
method does not respect alignment. The way arguments are currently
pushed and pulled makes it quite hard to fix the issue. Any suggestion
would be welcome.

> In the mean time, Christoffer has raised a number for valid concerns,
> and those need to be addressed first before it makes sense to talk
> about EFI specifics. I hope you will find more time to invest in
> this: I would really love to have this feature upstream.

Unfortunately, I'm no longer working at ARM and my other projects keep
me very busy. I would also love to invest more time in this patchset to
have it upstream but I'm really unsure when I will be able to find the
time for this.

Best,
    Florent
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [RFC 00/11] KVM, EFI, arm64: EFI Runtime Services Sandboxing

[Ard Biesheuvel]

On 25 August 2017 at 01:31, Florent Revest  wrote:
> Hi,
>
> This series implements a mechanism to sandbox EFI Runtime Services on arm64.
> It can be enabled with CONFIG_EFI_SANDBOX. At boot it spawns an internal KVM
> virtual machine that is ran everytime an EFI Runtime Service is called. This
> limits the possible security and stability impact of EFI runtime on the 
> kernel.
>
> The patch set is split as follow:
>  - Patches 1 and 2: Give more control over HVC handling to KVM
>  - Patches 3 to 6: Introduce the concept of KVM "internal VMs"
>  - Patches 7 to 9: Reorder KVM and EFI initialization on ARM
>  - Patch 10: Introduces the EFI sandboxing VM and wrappers
>  - Patch 11: Workarounds some EFI Runtime Services relying on EL3
>
> The sandboxing has been tested to work reliably (rtc and efivars) on a
> SoftIron OverDrive 1000 box and on a ARMv8.3 model with VHE enabled. Normal
> userspace KVM instance have also been tested to still work correctly.
>
> Those patches apply cleanly on the Linus' v4.13-rc6 tag and have no other
> dependencies.
>
> Florent Revest (11):
>   arm64: Add an SMCCC function IDs header
>   KVM: arm64: Return an Unknown ID on unhandled HVC
>   KVM: Allow VM lifecycle management without userspace
>   KVM, arm, arm64: Offer PAs to IPAs idmapping to internal VMs
>   KVM: Expose VM/VCPU creation functions
>   KVM, arm64: Expose a VCPU initialization function
>   KVM: Allow initialization before the module target
>   KVM, arm, arm64: Initialize KVM's core earlier
>   EFI, arm, arm64: Enable EFI Runtime Services later
>   efi, arm64: Sandbox Runtime Services in a VM
>   KVM, arm64: Don't trap internal VMs SMC calls
>

Hello Florent,

This is really nice work. Thanks for contributing it.

>From the EFI side, there are some minor concerns on my part regarding
the calling convention, and the fact that we can no longer invoke
runtime services from a kernel running at EL1, but those all seem
fixable. I will respond to the patches in question in greater detail
at a later time.

In the mean time, Christoffer has raised a number for valid concerns,
and those need to be addressed first before it makes sense to talk
about EFI specifics. I hope you will find more time to invest in this:
I would really love to have this feature upstream.

Regards,
Ard.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [RFC 00/11] KVM, EFI, arm64: EFI Runtime Services Sandboxing

[Christoffer Dall]

Hi Florent,

On Fri, Aug 25, 2017 at 09:31:30AM +0100, Florent Revest wrote:
> Hi,
> 
> This series implements a mechanism to sandbox EFI Runtime Services on arm64.
> It can be enabled with CONFIG_EFI_SANDBOX. At boot it spawns an internal KVM
> virtual machine that is ran everytime an EFI Runtime Service is called. This
> limits the possible security and stability impact of EFI runtime on the 
> kernel.

I wonder if this should be split into two series; one that sets up
anything you may need from KVM, and another one that uses that for UEFI.

There's a lot KVM and UEFI intertwined logic and assumptions in patch
10, which makes this series a bit hard to read.

I'd like some documentation (in the series and in
Documentation/virtual/kvm) of how this works, and which hidden
assumptions there are.  For example, how do you ensure you never attempt
to return to userspace?  How many VCPUs do you support?  Do you support
any form of virtual interrupts?  How about timers?  Can a VM access
physical devices?  How do you debug and trace something like this?  Can
the VM be monitored from userspace?

These feel like fundamental questions to me that needs addressing before
I can competently review the code.

I think a slightly more concrete motivation and outlining the example of
the broken UEFI on Seattle would help paving the way for these patches.


Thanks,
-Christoffer

> 
> The patch set is split as follow:
>  - Patches 1 and 2: Give more control over HVC handling to KVM
>  - Patches 3 to 6: Introduce the concept of KVM "internal VMs"
>  - Patches 7 to 9: Reorder KVM and EFI initialization on ARM
>  - Patch 10: Introduces the EFI sandboxing VM and wrappers
>  - Patch 11: Workarounds some EFI Runtime Services relying on EL3
> 
> The sandboxing has been tested to work reliably (rtc and efivars) on a
> SoftIron OverDrive 1000 box and on a ARMv8.3 model with VHE enabled. Normal
> userspace KVM instance have also been tested to still work correctly.
> 
> Those patches apply cleanly on the Linus' v4.13-rc6 tag and have no other
> dependencies.
> 
> Florent Revest (11):
>   arm64: Add an SMCCC function IDs header
>   KVM: arm64: Return an Unknown ID on unhandled HVC
>   KVM: Allow VM lifecycle management without userspace
>   KVM, arm, arm64: Offer PAs to IPAs idmapping to internal VMs
>   KVM: Expose VM/VCPU creation functions
>   KVM, arm64: Expose a VCPU initialization function
>   KVM: Allow initialization before the module target
>   KVM, arm, arm64: Initialize KVM's core earlier
>   EFI, arm, arm64: Enable EFI Runtime Services later
>   efi, arm64: Sandbox Runtime Services in a VM
>   KVM, arm64: Don't trap internal VMs SMC calls
> 
>  arch/arm/include/asm/efi.h |   7 +
>  arch/arm/include/asm/kvm_coproc.h  |   3 +
>  arch/arm/include/asm/kvm_host.h|   1 +
>  arch/arm/include/asm/kvm_psci.h|   1 +
>  arch/arm/kvm/coproc.c  |   6 +
>  arch/arm/kvm/coproc_a15.c  |   3 +-
>  arch/arm/kvm/coproc_a7.c   |   3 +-
>  arch/arm64/include/asm/efi.h   |  71 
>  arch/arm64/include/asm/kvm_emulate.h   |   3 +
>  arch/arm64/include/asm/kvm_host.h  |   4 +
>  arch/arm64/include/asm/kvm_psci.h  |   1 +
>  arch/arm64/kernel/asm-offsets.c|   3 +
>  arch/arm64/kvm/handle_exit.c   |  27 +-
>  arch/arm64/kvm/sys_regs_generic_v8.c   |   8 +-
>  arch/x86/include/asm/efi.h |   2 +
>  drivers/firmware/efi/Kconfig   |  10 +
>  drivers/firmware/efi/Makefile  |   1 +
>  drivers/firmware/efi/arm-runtime.c |   5 +-
>  drivers/firmware/efi/arm-sandbox-payload.S |  96 +
>  drivers/firmware/efi/arm-sandbox.c | 569 
> +
>  drivers/firmware/efi/efi.c |   3 +
>  include/linux/kvm_host.h   |   4 +
>  include/linux/smccc_fn.h   |  53 +++
>  include/uapi/linux/psci.h  |   2 +
>  virt/kvm/arm/arm.c |  18 +-
>  virt/kvm/arm/mmu.c |  76 +++-
>  virt/kvm/arm/psci.c|  21 ++
>  virt/kvm/kvm_main.c| 102 --
>  28 files changed, 1050 insertions(+), 53 deletions(-)
>  create mode 100644 drivers/firmware/efi/arm-sandbox-payload.S
>  create mode 100644 drivers/firmware/efi/arm-sandbox.c
>  create mode 100644 include/linux/smccc_fn.h
> 
> --
> 1.9.1
> 
> IMPORTANT NOTICE: The contents of this email and any attachments are 
> confidential and may also be privileged. If you are not the intended 
> recipient, please notify the sender immediately and do not disclose the 
> contents to any other person, use it for any purpose, or store or copy the 
> information in any medium. Thank you.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[RFC 00/11] KVM, EFI, arm64: EFI Runtime Services Sandboxing

[Florent Revest]

Hi,

This series implements a mechanism to sandbox EFI Runtime Services on arm64.
It can be enabled with CONFIG_EFI_SANDBOX. At boot it spawns an internal KVM
virtual machine that is ran everytime an EFI Runtime Service is called. This
limits the possible security and stability impact of EFI runtime on the kernel.

The patch set is split as follow:
 - Patches 1 and 2: Give more control over HVC handling to KVM
 - Patches 3 to 6: Introduce the concept of KVM "internal VMs"
 - Patches 7 to 9: Reorder KVM and EFI initialization on ARM
 - Patch 10: Introduces the EFI sandboxing VM and wrappers
 - Patch 11: Workarounds some EFI Runtime Services relying on EL3

The sandboxing has been tested to work reliably (rtc and efivars) on a
SoftIron OverDrive 1000 box and on a ARMv8.3 model with VHE enabled. Normal
userspace KVM instance have also been tested to still work correctly.

Those patches apply cleanly on the Linus' v4.13-rc6 tag and have no other
dependencies.

Florent Revest (11):
  arm64: Add an SMCCC function IDs header
  KVM: arm64: Return an Unknown ID on unhandled HVC
  KVM: Allow VM lifecycle management without userspace
  KVM, arm, arm64: Offer PAs to IPAs idmapping to internal VMs
  KVM: Expose VM/VCPU creation functions
  KVM, arm64: Expose a VCPU initialization function
  KVM: Allow initialization before the module target
  KVM, arm, arm64: Initialize KVM's core earlier
  EFI, arm, arm64: Enable EFI Runtime Services later
  efi, arm64: Sandbox Runtime Services in a VM
  KVM, arm64: Don't trap internal VMs SMC calls

 arch/arm/include/asm/efi.h |   7 +
 arch/arm/include/asm/kvm_coproc.h  |   3 +
 arch/arm/include/asm/kvm_host.h|   1 +
 arch/arm/include/asm/kvm_psci.h|   1 +
 arch/arm/kvm/coproc.c  |   6 +
 arch/arm/kvm/coproc_a15.c  |   3 +-
 arch/arm/kvm/coproc_a7.c   |   3 +-
 arch/arm64/include/asm/efi.h   |  71 
 arch/arm64/include/asm/kvm_emulate.h   |   3 +
 arch/arm64/include/asm/kvm_host.h  |   4 +
 arch/arm64/include/asm/kvm_psci.h  |   1 +
 arch/arm64/kernel/asm-offsets.c|   3 +
 arch/arm64/kvm/handle_exit.c   |  27 +-
 arch/arm64/kvm/sys_regs_generic_v8.c   |   8 +-
 arch/x86/include/asm/efi.h |   2 +
 drivers/firmware/efi/Kconfig   |  10 +
 drivers/firmware/efi/Makefile  |   1 +
 drivers/firmware/efi/arm-runtime.c |   5 +-
 drivers/firmware/efi/arm-sandbox-payload.S |  96 +
 drivers/firmware/efi/arm-sandbox.c | 569 +
 drivers/firmware/efi/efi.c |   3 +
 include/linux/kvm_host.h   |   4 +
 include/linux/smccc_fn.h   |  53 +++
 include/uapi/linux/psci.h  |   2 +
 virt/kvm/arm/arm.c |  18 +-
 virt/kvm/arm/mmu.c |  76 +++-
 virt/kvm/arm/psci.c|  21 ++
 virt/kvm/kvm_main.c| 102 --
 28 files changed, 1050 insertions(+), 53 deletions(-)
 create mode 100644 drivers/firmware/efi/arm-sandbox-payload.S
 create mode 100644 drivers/firmware/efi/arm-sandbox.c
 create mode 100644 include/linux/smccc_fn.h

--
1.9.1

IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium. Thank you.
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm