xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Xen-devel <xen-devel@lists.xen.org>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>,
	Paul Durrant <paul.durrant@citrix.com>,
	Jan Beulich <JBeulich@suse.com>
Subject: [PATCH 1/6] x86/hvm: Fixes to hvmemul_insn_fetch()
Date: Wed, 21 Jun 2017 16:12:27 +0100	[thread overview]
Message-ID: <1498057952-13556-2-git-send-email-andrew.cooper3@citrix.com> (raw)
In-Reply-To: <1498057952-13556-1-git-send-email-andrew.cooper3@citrix.com>

Force insn_off to a single byte, as offset can wrap around or truncate with
respect to sh_ctxt->insn_buf_eip under a number of normal circumstances.

Furthermore, don't use an ASSERT() for bounds checking the write into
hvmemul_ctxt->insn_buf[].

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Paul Durrant <paul.durrant@citrix.com>

For anyone wondering, this is way to explot XSA-186
---
 xen/arch/x86/hvm/emulate.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
index 11e4aba..495e312 100644
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -939,7 +939,8 @@ int hvmemul_insn_fetch(
 {
     struct hvm_emulate_ctxt *hvmemul_ctxt =
         container_of(ctxt, struct hvm_emulate_ctxt, ctxt);
-    unsigned int insn_off = offset - hvmemul_ctxt->insn_buf_eip;
+    /* Careful, as offset can wrap or truncate WRT insn_buf_eip. */
+    uint8_t insn_off = offset - hvmemul_ctxt->insn_buf_eip;
 
     /*
      * Fall back if requested bytes are not in the prefetch cache.
@@ -953,7 +954,17 @@ int hvmemul_insn_fetch(
 
         if ( rc == X86EMUL_OKAY && bytes )
         {
-            ASSERT(insn_off + bytes <= sizeof(hvmemul_ctxt->insn_buf));
+            /*
+             * 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) )
+            {
+                ASSERT_UNREACHABLE();
+                return X86EMUL_UNHANDLEABLE;
+            }
+
             memcpy(&hvmemul_ctxt->insn_buf[insn_off], p_data, bytes);
             hvmemul_ctxt->insn_buf_bytes = insn_off + bytes;
         }
-- 
2.1.4


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

  reply	other threads:[~2017-06-21 15:12 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-21 15:12 [PATCH 0/6] Various XSA followups Andrew Cooper
2017-06-21 15:12 ` Andrew Cooper [this message]
2017-06-21 16:04   ` [PATCH 1/6] x86/hvm: Fixes to hvmemul_insn_fetch() Paul Durrant
2017-06-21 16:15     ` Andrew Cooper
2017-06-21 16:49       ` Paul Durrant
2017-06-22  8:05   ` Jan Beulich
2017-06-21 15:12 ` [PATCH 2/6] x86/shadow: Fixes to hvm_emulate_insn_fetch() Andrew Cooper
2017-06-22  8:09   ` Jan Beulich
2017-06-22 11:52   ` Tim Deegan
2017-06-21 15:12 ` [PATCH 3/6] x86/shadow: Use ERR_PTR infrastructure for sh_emulate_map_dest() Andrew Cooper
2017-06-22  8:14   ` Jan Beulich
2017-06-22  8:21     ` Andrew Cooper
2017-06-22  9:07       ` Jan Beulich
2017-07-03 14:22         ` Andrew Cooper
2017-07-03 14:30           ` Jan Beulich
2017-06-22 12:09   ` Tim Deegan
2017-06-22 12:46     ` Andrew Cooper
2017-06-21 15:12 ` [PATCH 4/6] [RFC] x86/hvm: Rename enum hvm_copy_result to hvm_translation_result Andrew Cooper
2017-06-22  8:19   ` Jan Beulich
2017-06-21 15:12 ` [PATCH 5/6] x86/hvm: Break out __hvm_copy()'s translation logic Andrew Cooper
2017-06-22  8:30   ` Jan Beulich
2017-06-21 15:12 ` [PATCH 6/6] x86/hvm: Implement hvmemul_write() using real mappings Andrew Cooper
2017-06-21 16:19   ` Paul Durrant
2017-06-22  9:06   ` Jan Beulich
2017-07-03 15:07     ` Andrew Cooper
2017-07-03 16:06       ` Jan Beulich
     [not found]     ` <594BA4A3?= =?UTF-8?Q?0200007800165AA5@prv=ef=bf=bdmh.provo.novell.com>
     [not found]       ` <b9e9b637-0755-?= =?UTF-8?Q?a1bd-99c7-44ad3f13b5a4@citrix.com>
     [not found]         ` <595A879202000078001680B7@prv-?= =?UTF-8?Q?mh.provo.novell.com>
2017-07-03 17:24           ` Andrew Cooper
2017-07-04  7:10             ` Jan Beulich
     [not found] <1498057952-13556-1-git-send-email-andrew.cooper3@citr?= =?UTF-8?Q?ix.com>

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1498057952-13556-2-git-send-email-andrew.cooper3@citrix.com \
    --to=andrew.cooper3@citrix.com \
    --cc=JBeulich@suse.com \
    --cc=paul.durrant@citrix.com \
    --cc=xen-devel@lists.xen.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).