[4.4-stable PATCH 0/2] Fix KVM/arm regression in 4.4.175

[4.4-stable PATCH 0/2] Fix KVM/arm regression in 4.4.175

[Marc Zyngier]

Daniel Verkamp reported that the backport of 0d640732dbeb ("arm64: KVM: Skip
MMIO insn after emulation") to 4.4-stable has broken KVM on arm/arm64.

It turns out that the guest cannot make forward progress as soon as it hits
a device emulated by the host kernel, like the interrupt controller. The
reason for this is a set of missing dependencies from the 4.7 era. With
these patches added to 4.4.175, I'm able to boot guests normally.

Tested with both kvmtool and crossvm.

Christoffer Dall (1):
  KVM: arm/arm64: Fix MMIO emulation data handling

Marc Zyngier (1):
  arm/arm64: KVM: Feed initialized memory to MMIO accesses

 arch/arm/kvm/mmio.c | 10 ++++++----
 virt/kvm/arm/vgic.c |  7 -------
 2 files changed, 6 insertions(+), 11 deletions(-)

-- 
2.20.1

[4.4-stable PATCH 1/2] arm/arm64: KVM: Feed initialized memory to MMIO accesses

[Marc Zyngier]

commit 1d6a821277aaa0cdd666278aaff93298df313d41 upstream.

On an MMIO access, we always copy the on-stack buffer info
the shared "run" structure, even if this is a read access.
This ends up leaking up to 8 bytes of uninitialized memory
into userspace, depending on the size of the access.

An obvious fix for this one is to only perform the copy if
this is an actual write.

Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/kvm/mmio.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/kvm/mmio.c b/arch/arm/kvm/mmio.c
index 885cd0e0015b..0b9d152b38c8 100644
--- a/arch/arm/kvm/mmio.c
+++ b/arch/arm/kvm/mmio.c
@@ -207,7 +207,8 @@ int io_mem_abort(struct kvm_vcpu *vcpu, struct kvm_run *run,
 	run->mmio.is_write	= is_write;
 	run->mmio.phys_addr	= fault_ipa;
 	run->mmio.len		= len;
-	memcpy(run->mmio.data, data_buf, len);
+	if (is_write)
+		memcpy(run->mmio.data, data_buf, len);
 
 	if (!ret) {
 		/* We handled the access successfully in the kernel. */
-- 
2.20.1

[4.4-stable PATCH 2/2] KVM: arm/arm64: Fix MMIO emulation data handling

[Marc Zyngier]

From: Christoffer Dall <christoffer.dall@linaro.org>

commit 83091db981e105d97562d3ed3ffe676e21927e3a upstream

When the kernel was handling a guest MMIO read access internally, we
need to copy the emulation result into the run->mmio structure in order
for the kvm_handle_mmio_return() function to pick it up and inject the
	result back into the guest.

Currently the only user of kvm_io_bus for ARM is the VGIC, which did
this copying itself, so this was not causing issues so far.

But with the upcoming new vgic implementation we need this done
properly.

Update the kvm_handle_mmio_return description and cleanup the code to
only perform a single copying when needed.

Code and commit message inspired by Andre Przywara.

Reported-by: Andre Przywara <andre.przywara@arm.com>
Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
Reviewed-by: Andre Przywara <andre.przywara@arm.com>
Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
---
 arch/arm/kvm/mmio.c | 11 ++++++-----
 virt/kvm/arm/vgic.c |  7 -------
 2 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/arch/arm/kvm/mmio.c b/arch/arm/kvm/mmio.c
index 0b9d152b38c8..ae61e2ea7255 100644
--- a/arch/arm/kvm/mmio.c
+++ b/arch/arm/kvm/mmio.c
@@ -87,11 +87,10 @@ static unsigned long mmio_read_buf(char *buf, unsigned int len)
 
 /**
  * kvm_handle_mmio_return -- Handle MMIO loads after user space emulation
+ *			     or in-kernel IO emulation
+ *
  * @vcpu: The VCPU pointer
  * @run:  The VCPU run struct containing the mmio data
- *
- * This should only be called after returning from userspace for MMIO load
- * emulation.
  */
 int kvm_handle_mmio_return(struct kvm_vcpu *vcpu, struct kvm_run *run)
 {
@@ -207,15 +206,17 @@ int io_mem_abort(struct kvm_vcpu *vcpu, struct kvm_run *run,
 	run->mmio.is_write	= is_write;
 	run->mmio.phys_addr	= fault_ipa;
 	run->mmio.len		= len;
-	if (is_write)
-		memcpy(run->mmio.data, data_buf, len);
 
 	if (!ret) {
 		/* We handled the access successfully in the kernel. */
+		if (!is_write)
+			memcpy(run->mmio.data, data_buf, len);
 		kvm_handle_mmio_return(vcpu, run);
 		return 1;
 	}
 
+	if (is_write)
+		memcpy(run->mmio.data, data_buf, len);
 	run->exit_reason	= KVM_EXIT_MMIO;
 	return 0;
 }
diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index 5d10f104f3eb..964df643509d 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -821,7 +821,6 @@ static int vgic_handle_mmio_access(struct kvm_vcpu *vcpu,
 	struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
 	struct vgic_io_device *iodev = container_of(this,
 						    struct vgic_io_device, dev);
-	struct kvm_run *run = vcpu->run;
 	const struct vgic_io_range *range;
 	struct kvm_exit_mmio mmio;
 	bool updated_state;
@@ -850,12 +849,6 @@ static int vgic_handle_mmio_access(struct kvm_vcpu *vcpu,
 		updated_state = false;
 	}
 	spin_unlock(&dist->lock);
-	run->mmio.is_write	= is_write;
-	run->mmio.len		= len;
-	run->mmio.phys_addr	= addr;
-	memcpy(run->mmio.data, val, len);
-
-	kvm_handle_mmio_return(vcpu, run);
 
 	if (updated_state)
 		vgic_kick_vcpus(vcpu->kvm);
-- 
2.20.1

Re: [4.4-stable PATCH 0/2] Fix KVM/arm regression in 4.4.175

[Greg KH]

On Thu, Feb 28, 2019 at 01:27:19PM +0000, Marc Zyngier wrote:
> Daniel Verkamp reported that the backport of 0d640732dbeb ("arm64: KVM: Skip
> MMIO insn after emulation") to 4.4-stable has broken KVM on arm/arm64.
> 
> It turns out that the guest cannot make forward progress as soon as it hits
> a device emulated by the host kernel, like the interrupt controller. The
> reason for this is a set of missing dependencies from the 4.7 era. With
> these patches added to 4.4.175, I'm able to boot guests normally.
> 
> Tested with both kvmtool and crossvm.
> 

Thanks for figuring this out and for both of these patches, now queued
up.

greg k-h

Re: [4.4-stable PATCH 0/2] Fix KVM/arm regression in 4.4.175

[Daniel Verkamp]

On Thu, Feb 28, 2019 at 5:27 AM Marc Zyngier <marc.zyngier@arm.com> wrote:
>
> Daniel Verkamp reported that the backport of 0d640732dbeb ("arm64: KVM: Skip
> MMIO insn after emulation") to 4.4-stable has broken KVM on arm/arm64.
>
> It turns out that the guest cannot make forward progress as soon as it hits
> a device emulated by the host kernel, like the interrupt controller. The
> reason for this is a set of missing dependencies from the 4.7 era. With
> these patches added to 4.4.175, I'm able to boot guests normally.
>
> Tested with both kvmtool and crossvm.

Thanks for the quick turnaround! I have also tested with these two
patches applied on top of 4.4.175, and crosvm is working again.

-- Daniel