Re: [PATCH v2 7/7] KVM: PPC: mmio: Reject instructions that access more than mmio.data size

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 <[email protected]>


> 
> Signed-off-by: Fabiano Rosas <[email protected]>
> ---
>   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;

-- 
Alexey