diff mbox series

[v2,8/8] KVM: x86: Bug the VM on an out-of-bounds data read

Message ID 20220526210817.3428868-9-seanjc@google.com (mailing list archive)
State New, archived
Headers show
Series KVM: x86: Emulator _regs fixes and cleanups | expand

Commit Message

Sean Christopherson May 26, 2022, 9:08 p.m. UTC 
Bug the VM and terminate emulation if an out-of-bounds read into the
emulator's data cache occurs.  Knowingly contuining on all but guarantees
that KVM will overwrite random kernel data, which is far, far worse than
killing the VM.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/emulate.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Kees Cook May 31, 2022, 6:06 p.m. UTC | #1 
On Thu, May 26, 2022 at 09:08:17PM +0000, Sean Christopherson wrote:
> Bug the VM and terminate emulation if an out-of-bounds read into the
> emulator's data cache occurs.  Knowingly contuining on all but guarantees
> that KVM will overwrite random kernel data, which is far, far worse than
> killing the VM.
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>

Reviewed-by: Kees Cook <keescook@chromium.org>
Vitaly Kuznetsov June 1, 2022, 8:39 a.m. UTC | #2 
Sean Christopherson <seanjc@google.com> writes:

> Bug the VM and terminate emulation if an out-of-bounds read into the
> emulator's data cache occurs.  Knowingly contuining on all but guarantees
> that KVM will overwrite random kernel data, which is far, far worse than
> killing the VM.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/kvm/emulate.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index 2aa17462a9ac..39ea9138224c 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -1373,7 +1373,8 @@ static int read_emulated(struct x86_emulate_ctxt *ctxt,
>  	if (mc->pos < mc->end)
>  		goto read_cached;
>  
> -	WARN_ON((mc->end + size) >= sizeof(mc->data));
> +	if (KVM_EMULATOR_BUG_ON((mc->end + size) >= sizeof(mc->data), ctxt))
> +		return X86EMUL_UNHANDLEABLE;
>  
>  	rc = ctxt->ops->read_emulated(ctxt, addr, mc->data + mc->end, size,
>  				      &ctxt->exception);

The last WARN_ON() is gone, cool)

Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
diff mbox series

Patch

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 2aa17462a9ac..39ea9138224c 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -1373,7 +1373,8 @@  static int read_emulated(struct x86_emulate_ctxt *ctxt,
 	if (mc->pos < mc->end)
 		goto read_cached;
 
-	WARN_ON((mc->end + size) >= sizeof(mc->data));
+	if (KVM_EMULATOR_BUG_ON((mc->end + size) >= sizeof(mc->data), ctxt))
+		return X86EMUL_UNHANDLEABLE;
 
 	rc = ctxt->ops->read_emulated(ctxt, addr, mc->data + mc->end, size,
 				      &ctxt->exception);