[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [Xen-changelog] [xen-3.2-testing] SVM: clean up __get_instruction_length_from_list()
# 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
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |