# HG changeset patch
# User Keir Fraser <keir.fraser@xxxxxxxxxx>
# Date 1210690471 -3600
# Node ID 784805591fa2d972e790451d3242f6c59f8eeebf
# Parent f70475e8396dc4bc0304d5ff697f18e2b35926f4
SVM: clean up __get_instruction_length_from_list()
Remove unused arguments, fix its behaviour near page boundaries,
inject appropriate pagefaults, and inject #GP if the instruction is
not decodable or %eip is not pointing to valid RAM.
Signed-off-by: Tim Deegan <Tim.Deegan@xxxxxxxxxx>
Signed-off-by: Keir Fraser <keir.fraser@xxxxxxxxxx>
xen-unstable changeset: 17575:01aa7c088e983cd54b61faeb3ff533581714a26f
xen-unstable date: Tue May 06 13:32:18 2008 +0100
---
xen/arch/x86/hvm/svm/emulate.c | 73 ++++++++++++++++++++++++++++---------
xen/arch/x86/hvm/svm/svm.c | 79 ++++++++++++++++++-----------------------
xen/arch/x86/hvm/vmx/vmx.c | 3 -
3 files changed, 92 insertions(+), 63 deletions(-)
diff -r f70475e8396d -r 784805591fa2 xen/arch/x86/hvm/svm/emulate.c
--- a/xen/arch/x86/hvm/svm/emulate.c Tue May 13 15:34:33 2008 +0100
+++ b/xen/arch/x86/hvm/svm/emulate.c Tue May 13 15:54:31 2008 +0100
@@ -28,9 +28,6 @@
#include <asm/hvm/svm/vmcb.h>
#include <asm/hvm/svm/emulate.h>
-
-extern int inst_copy_from_guest(unsigned char *buf, unsigned long guest_eip,
- int inst_len);
#define REX_PREFIX_BASE 0x40
#define REX_X 0x02
@@ -412,8 +409,8 @@ static const u8 *opc_bytes[INSTR_MAX_COU
* Intel has a vmcs entry to give the instruction length. AMD doesn't. So we
* have to do a little bit of work to find out...
*
- * The caller can either pass a NULL pointer to the guest_eip_buf, or a pointer
- * to enough bytes to satisfy the instruction including prefix bytes.
+ * The caller may supply a buffer of at least MAX_INST_LEN bytes, which
+ * the instruction will be read into.
*/
int __get_instruction_length_from_list(struct vcpu *v,
enum instruction_index *list, unsigned int list_count,
@@ -425,21 +422,40 @@ int __get_instruction_length_from_list(s
unsigned int j;
int found = 0;
enum instruction_index instr = 0;
- u8 buffer[MAX_INST_LEN];
+ unsigned long fetch_addr;
+ int fetch_len;
u8 *buf;
const u8 *opcode = NULL;
-
- if (guest_eip_buf)
- {
- buf = guest_eip_buf;
- }
- else
- {
- if ( inst_copy_from_guest(buffer, svm_rip2pointer(v), MAX_INST_LEN)
- != MAX_INST_LEN )
- return 0;
- buf = buffer;
- }
+ u8 temp_buffer[MAX_INST_LEN];
+
+ /* Use the stack if the caller didn't give us a buffer */
+ buf = ( guest_eip_buf ) ? guest_eip_buf : temp_buffer;
+
+#define FETCH(_buf, _addr, _len) do \
+ { \
+ switch ( hvm_fetch_from_guest_virt((_buf), (_addr), (_len)) ) \
+ { \
+ case HVMCOPY_okay: \
+ break; \
+ case HVMCOPY_bad_gva_to_gfn: \
+ /* OK just to give up; we'll have injected #PF already */ \
+ return 0; \
+ case HVMCOPY_bad_gfn_to_mfn: \
+ /* Not OK: fetches from non-RAM pages are not supportable. */ \
+ gdprintk(XENLOG_ERR, "Bad instruction fetch at %#lx (%#lx)\n", \
+ (unsigned long) guest_cpu_user_regs()->eip, fetch_addr); \
+ hvm_inject_exception(TRAP_gp_fault, 0, 0); \
+ return 0; \
+ } \
+ } while (0)
+
+ /* Fetch up to the next page break; we'll fetch from the next page
+ * later if we have to. */
+ fetch_addr = svm_rip2pointer(v);
+ fetch_len = PAGE_SIZE - (fetch_addr & ~PAGE_MASK) ;
+ if ( fetch_len > MAX_INST_LEN )
+ fetch_len = MAX_INST_LEN;
+ FETCH(buf, fetch_addr, fetch_len);
for (j = 0; j < list_count; j++)
{
@@ -450,7 +466,16 @@ int __get_instruction_length_from_list(s
while (inst_len < MAX_INST_LEN &&
is_prefix(buf[inst_len]) &&
!is_prefix(opcode[1]))
+ {
inst_len++;
+ if ( inst_len >= fetch_len )
+ {
+ FETCH(buf + fetch_len,
+ fetch_addr + fetch_len,
+ MAX_INST_LEN - fetch_len);
+ fetch_len = MAX_INST_LEN;
+ }
+ }
ASSERT(opcode[0] <= 15); /* Make sure the table is correct. */
found = 1;
@@ -460,6 +485,14 @@ int __get_instruction_length_from_list(s
/* If the last byte is zero, we just accept it without checking */
if (i == opcode[0]-1 && opcode[i+1] == 0)
break;
+
+ if ( inst_len + i >= fetch_len )
+ {
+ FETCH(buf + fetch_len,
+ fetch_addr + fetch_len,
+ MAX_INST_LEN - fetch_len);
+ fetch_len = MAX_INST_LEN;
+ }
if (buf[inst_len+i] != opcode[i+1])
{
@@ -487,7 +520,11 @@ int __get_instruction_length_from_list(s
printk("%s: Mismatch between expected and actual instruction bytes: "
"eip = %lx\n", __func__, (unsigned long)vmcb->rip);
+ hvm_inject_exception(TRAP_gp_fault, 0, 0);
return 0;
+
+#undef FETCH
+
}
/*
diff -r f70475e8396d -r 784805591fa2 xen/arch/x86/hvm/svm/svm.c
--- a/xen/arch/x86/hvm/svm/svm.c Tue May 13 15:34:33 2008 +0100
+++ b/xen/arch/x86/hvm/svm/svm.c Tue May 13 15:54:31 2008 +0100
@@ -78,7 +78,10 @@ static void inline __update_guest_eip(
{
struct vcpu *curr = current;
- if ( unlikely((inst_len == 0) || (inst_len > 15)) )
+ if ( unlikely(inst_len == 0) )
+ return;
+
+ if ( unlikely(inst_len > 15) )
{
gdprintk(XENLOG_ERR, "Bad instruction length %u\n", inst_len);
domain_crash(curr->domain);
@@ -1131,18 +1134,28 @@ static void svm_dr_access(struct vcpu *v
static int svm_get_prefix_info(struct vcpu *v, unsigned int dir,
svm_segment_register_t **seg,
- unsigned int *asize)
+ unsigned int *asize,
+ unsigned int isize)
{
struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb;
unsigned char inst[MAX_INST_LEN];
int i;
memset(inst, 0, MAX_INST_LEN);
- if (inst_copy_from_guest(inst, svm_rip2pointer(v), sizeof(inst))
- != MAX_INST_LEN)
- {
- gdprintk(XENLOG_ERR, "get guest instruction failed\n");
- return 0;
+
+ switch ( hvm_fetch_from_guest_virt(inst, svm_rip2pointer(v), isize) )
+ {
+ case HVMCOPY_okay:
+ break;
+ case HVMCOPY_bad_gva_to_gfn:
+ /* OK just to give up; we'll have injected #PF already */
+ return 0;
+ case HVMCOPY_bad_gfn_to_mfn:
+ gdprintk(XENLOG_ERR, "Bad prefix fetch at %#lx (%#lx)\n",
+ (unsigned long) guest_cpu_user_regs()->eip,
+ svm_rip2pointer(v));
+ domain_crash(v->domain);
+ return 0;
}
for (i = 0; i < MAX_INST_LEN; i++)
@@ -1232,12 +1245,8 @@ static int svm_get_io_address(
* to figure out what it is...
*/
isize = vmcb->exitinfo2 - regs->eip;
-
- if (info.fields.rep)
- isize --;
-
- if (isize > 1)
- if ( !svm_get_prefix_info(v, info.fields.type, &seg, &asize) )
+ if ( isize > ((info.fields.rep) ? 2 : 1) )
+ if ( !svm_get_prefix_info(v, info.fields.type, &seg, &asize, isize) )
return 0;
if (info.fields.type == IOREQ_WRITE)
@@ -1593,30 +1602,24 @@ static void svm_cr_access(
enum instruction_index list_b[] = {INSTR_MOVCR2, INSTR_SMSW};
enum instruction_index match;
- if ( inst_copy_from_guest(buffer, svm_rip2pointer(v), sizeof(buffer))
- != sizeof buffer )
- /* #PF will have been delivered if appropriate. */
+
+ if ( type == TYPE_MOV_TO_CR )
+ {
+ inst_len = __get_instruction_length_from_list(
+ v, list_a, ARRAY_SIZE(list_a), buffer, &match);
+ }
+ else /* type == TYPE_MOV_FROM_CR */
+ {
+ inst_len = __get_instruction_length_from_list(
+ v, list_b, ARRAY_SIZE(list_b), buffer, &match);
+ }
+
+ if ( inst_len == 0 )
return;
/* get index to first actual instruction byte - as we will need to know
where the prefix lives later on */
index = skip_prefix_bytes(buffer, sizeof(buffer));
-
- if ( type == TYPE_MOV_TO_CR )
- {
- inst_len = __get_instruction_length_from_list(
- v, list_a, ARRAY_SIZE(list_a), &buffer[index], &match);
- }
- else /* type == TYPE_MOV_FROM_CR */
- {
- inst_len = __get_instruction_length_from_list(
- v, list_b, ARRAY_SIZE(list_b), &buffer[index], &match);
- }
-
- if ( inst_len == 0 )
- return;
-
- inst_len += index;
/* Check for REX prefix - it's ALWAYS the last byte of any prefix bytes */
if (index > 0 && (buffer[index-1] & 0xF0) == 0x40)
@@ -1941,16 +1944,6 @@ void svm_handle_invlpg(const short invlp
unsigned long g_vaddr;
int inst_len;
- /*
- * Unknown how many bytes the invlpg instruction will take. Use the
- * maximum instruction length here
- */
- if ( inst_copy_from_guest(opcode, svm_rip2pointer(v), length) < length )
- {
- gdprintk(XENLOG_ERR, "Error reading memory %d bytes\n", length);
- return;
- }
-
if ( invlpga )
{
inst_len = __get_instruction_length(v, INSTR_INVLPGA, opcode);
@@ -1965,8 +1958,8 @@ void svm_handle_invlpg(const short invlp
else
{
/* What about multiple prefix codes? */
+ inst_len = __get_instruction_length(v, INSTR_INVLPG, opcode);
prefix = (is_prefix(opcode[0]) ? opcode[0] : 0);
- inst_len = __get_instruction_length(v, INSTR_INVLPG, opcode);
if ( inst_len <= 0 )
{
gdprintk(XENLOG_ERR, "Error getting invlpg instr len\n");
diff -r f70475e8396d -r 784805591fa2 xen/arch/x86/hvm/vmx/vmx.c
--- a/xen/arch/x86/hvm/vmx/vmx.c Tue May 13 15:34:33 2008 +0100
+++ b/xen/arch/x86/hvm/vmx/vmx.c Tue May 13 15:54:31 2008 +0100
@@ -1390,7 +1390,6 @@ static enum x86_segment vmx_outs_get_seg
unsigned char inst[MAX_INST_LEN];
enum x86_segment seg = x86_seg_ds;
int i;
- extern int inst_copy_from_guest(unsigned char *, unsigned long, int);
if ( likely(cpu_has_vmx_ins_outs_instr_info) )
{
@@ -1415,7 +1414,7 @@ static enum x86_segment vmx_outs_get_seg
eip += __vmread(GUEST_CS_BASE);
memset(inst, 0, MAX_INST_LEN);
- if ( inst_copy_from_guest(inst, eip, inst_len) != inst_len )
+ if ( hvm_fetch_from_guest_virt(inst, eip, inst_len) != HVMCOPY_okay )
{
gdprintk(XENLOG_ERR, "Get guest instruction failed\n");
domain_crash(current->domain);
_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog
|
Home