Re: [Xen-devel] [PATCH] x86/hvm: Fix boundary check in hvmemul_insn_fetch()

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Andrew Cooper [mailto:andrew.cooper3@xxxxxxxxxx]
> Sent: 25 July 2017 19:56
> To: Xen-devel <xen-devel@xxxxxxxxxxxxx>
> Cc: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>; Jan Beulich
> <JBeulich@xxxxxxxx>; Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> Subject: [PATCH] x86/hvm: Fix boundary check in hvmemul_insn_fetch()
> 
> c/s 0943a03037 added some extra protection for overflowing the emulation
> instruction cache, but Coverity points out that boundary condition is off by
> one when memcpy()'ing out of the buffer.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Oops. Yes.

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

> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Paul Durrant <paul.durrant@xxxxxxxxxx>
> ---
>  xen/arch/x86/hvm/emulate.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
> index 495e312..52bed04 100644
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -958,8 +958,8 @@ int hvmemul_insn_fetch(
>               * Will we overflow insn_buf[]?  This shouldn't be able to 
> happen,
>               * which means something went wrong with instruction decoding...
>               */
> -            if ( insn_off > sizeof(hvmemul_ctxt->insn_buf) ||
> -                 (insn_off + bytes) > sizeof(hvmemul_ctxt->insn_buf) )
> +            if ( insn_off >= sizeof(hvmemul_ctxt->insn_buf) ||
> +                 (insn_off + bytes) >= sizeof(hvmemul_ctxt->insn_buf) )
>              {
>                  ASSERT_UNREACHABLE();
>                  return X86EMUL_UNHANDLEABLE;
> --
> 2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel