| Message ID | 20220106200304.4070825-8-farosas@linux.ibm.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | KVM: PPC: MMIO fixes | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/github-powerpc_selftests | success | Successfully ran 8 jobs. |
| snowpatch_ozlabs/github-powerpc_ppctests | success | Successfully ran 8 jobs. |
| snowpatch_ozlabs/github-powerpc_sparse | success | Successfully ran 4 jobs. |
| snowpatch_ozlabs/github-powerpc_clang | success | Successfully ran 7 jobs. |
| snowpatch_ozlabs/github-powerpc_kernel_qemu | success | Successfully ran 24 jobs. |
On 07/01/2022 07:03, Fabiano Rosas wrote: > The MMIO interface between the kernel and userspace uses a structure > that supports a maximum of 8-bytes of data. Instructions that access > more than that need to be emulated in parts. > > We currently don't have generic support for splitting the emulation in > parts and each set of instructions needs to be explicitly included. > > There's already an error message being printed when a load or store > exceeds the mmio.data buffer but we don't fail the emulation until > later at kvmppc_complete_mmio_load and even then we allow userspace to > make a partial copy of the data, which ends up overwriting some fields > of the mmio structure. > > This patch makes the emulation fail earlier at kvmppc_handle_load|store, > which will send a Program interrupt to the guest. This is better than > allowing the guest to proceed with partial data. > > Note that this was caught in a somewhat artificial scenario using > quadword instructions (lq/stq), there's no account of an actual guest > in the wild running instructions that are not properly emulated. Ah thereee it is :-) I'd merge it into 3/7. anyway, Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru> > > Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com> > --- > arch/powerpc/kvm/powerpc.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c > index 50e08635e18a..a1643ca988e0 100644 > --- a/arch/powerpc/kvm/powerpc.c > +++ b/arch/powerpc/kvm/powerpc.c > @@ -1247,6 +1247,7 @@ static int __kvmppc_handle_load(struct kvm_vcpu *vcpu, > if (bytes > sizeof(run->mmio.data)) { > printk(KERN_ERR "%s: bad MMIO length: %d\n", __func__, > bytes); > + return EMULATE_FAIL; > } > > run->mmio.phys_addr = vcpu->arch.paddr_accessed; > @@ -1336,6 +1337,7 @@ int kvmppc_handle_store(struct kvm_vcpu *vcpu, > if (bytes > sizeof(run->mmio.data)) { > printk(KERN_ERR "%s: bad MMIO length: %d\n", __func__, > bytes); > + return EMULATE_FAIL; > } > > run->mmio.phys_addr = vcpu->arch.paddr_accessed;
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c index 50e08635e18a..a1643ca988e0 100644 --- a/arch/powerpc/kvm/powerpc.c +++ b/arch/powerpc/kvm/powerpc.c @@ -1247,6 +1247,7 @@ static int __kvmppc_handle_load(struct kvm_vcpu *vcpu, if (bytes > sizeof(run->mmio.data)) { printk(KERN_ERR "%s: bad MMIO length: %d\n", __func__, bytes); + return EMULATE_FAIL; } run->mmio.phys_addr = vcpu->arch.paddr_accessed; @@ -1336,6 +1337,7 @@ int kvmppc_handle_store(struct kvm_vcpu *vcpu, if (bytes > sizeof(run->mmio.data)) { printk(KERN_ERR "%s: bad MMIO length: %d\n", __func__, bytes); + return EMULATE_FAIL; } run->mmio.phys_addr = vcpu->arch.paddr_accessed;
The MMIO interface between the kernel and userspace uses a structure that supports a maximum of 8-bytes of data. Instructions that access more than that need to be emulated in parts. We currently don't have generic support for splitting the emulation in parts and each set of instructions needs to be explicitly included. There's already an error message being printed when a load or store exceeds the mmio.data buffer but we don't fail the emulation until later at kvmppc_complete_mmio_load and even then we allow userspace to make a partial copy of the data, which ends up overwriting some fields of the mmio structure. This patch makes the emulation fail earlier at kvmppc_handle_load|store, which will send a Program interrupt to the guest. This is better than allowing the guest to proceed with partial data. Note that this was caught in a somewhat artificial scenario using quadword instructions (lq/stq), there's no account of an actual guest in the wild running instructions that are not properly emulated. Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com> --- arch/powerpc/kvm/powerpc.c | 2 ++ 1 file changed, 2 insertions(+)