diff mbox

x86/HVM: fix boundary check in hvmemul_insn_fetch() (again)

Message ID 598C2722020000780016E67B@prv-mh.provo.novell.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jan Beulich Aug. 10, 2017, 7:28 a.m. UTC 
Commit 5a992b670b ("x86/hvm: Fix boundary check in
hvmemul_insn_fetch()") went a little too far in its correction to
commit 0943a03037 ("x86/hvm: Fixes to hvmemul_insn_fetch()"): Keep the
start offset check, but restore the original end offset one.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

Comments

Paul Durrant Aug. 10, 2017, 8:35 a.m. UTC | #1 
> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: 10 August 2017 08:28
> To: xen-devel <xen-devel@lists.xenproject.org>
> Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Paul Durrant
> <Paul.Durrant@citrix.com>
> Subject: [PATCH] x86/HVM: fix boundary check in hvmemul_insn_fetch()
> (again)
> 
> Commit 5a992b670b ("x86/hvm: Fix boundary check in
> hvmemul_insn_fetch()") went a little too far in its correction to
> commit 0943a03037 ("x86/hvm: Fixes to hvmemul_insn_fetch()"): Keep the
> start offset check, but restore the original end offset one.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -959,7 +959,7 @@ int hvmemul_insn_fetch(
>               * which means something went wrong with instruction decoding...
>               */
>              if ( insn_off >= sizeof(hvmemul_ctxt->insn_buf) ||
> -                 (insn_off + bytes) >= sizeof(hvmemul_ctxt->insn_buf) )
> +                 insn_off + bytes > sizeof(hvmemul_ctxt->insn_buf) )

I thought it was generally good style to have brackets in clauses such as this. Anyway...

Reviewed-by: Paul Durrant <paul.durrant@citrix.com>

>              {
>                  ASSERT_UNREACHABLE();
>                  return X86EMUL_UNHANDLEABLE;
> 
>
diff mbox

Patch

--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -959,7 +959,7 @@  int hvmemul_insn_fetch(
              * which means something went wrong with instruction decoding...
              */
             if ( insn_off >= sizeof(hvmemul_ctxt->insn_buf) ||
-                 (insn_off + bytes) >= sizeof(hvmemul_ctxt->insn_buf) )
+                 insn_off + bytes > sizeof(hvmemul_ctxt->insn_buf) )
             {
                 ASSERT_UNREACHABLE();
                 return X86EMUL_UNHANDLEABLE;