* [PATCH v2] vvmx: fix instruction decode segment limit check
@ 2012-04-25 7:48 Jan Beulich
2012-04-25 8:29 ` Tim Deegan
0 siblings, 1 reply; 2+ messages in thread
From: Jan Beulich @ 2012-04-25 7:48 UTC (permalink / raw)
To: xen-devel; +Cc: Tim Deegan, Eddie Dong
[-- Attachment #1: Type: text/plain, Size: 2169 bytes --]
- no limit check for 64-bit mode (and GS: is not special in any way)
- limit check is needed in compatibility mode
- canonical address check should instead be performed for 64-bit mode
- the last accessed byte must be within limits, not the first byte past
the accessed range
- segment base address should be ignored for 64-bit mode unless FS: or
GS: is in use
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -336,8 +336,17 @@ static int decode_vmx_inst(struct cpu_us
}
else
{
+ bool_t mode_64bit = 0;
+
decode->type = VMX_INST_MEMREG_TYPE_MEMORY;
- if ( info.fields.segment > 5 )
+
+ if ( hvm_long_mode_enabled(v) )
+ {
+ hvm_get_segment_register(v, x86_seg_cs, &seg);
+ mode_64bit = seg.attr.fields.l;
+ }
+
+ if ( info.fields.segment > VMX_SREG_GS )
goto gp_fault;
hvm_get_segment_register(v, sreg_to_index[info.fields.segment], &seg);
seg_base = seg.base;
@@ -355,15 +364,20 @@ static int decode_vmx_inst(struct cpu_us
size = 1 << (info.fields.addr_size + 1);
offset = base + index * scale + disp;
- if ( (offset > seg.limit || offset + size > seg.limit) &&
- (!hvm_long_mode_enabled(v) || info.fields.segment == VMX_SREG_GS) )
+ base = !mode_64bit || info.fields.segment >= VMX_SREG_FS ?
+ seg_base + offset : offset;
+ if ( offset + size - 1 < offset ||
+ (mode_64bit ?
+ !is_canonical_address((long)base < 0 ? base :
+ base + size - 1) :
+ offset + size - 1 > seg.limit) )
goto gp_fault;
if ( poperandS != NULL &&
- hvm_copy_from_guest_virt(poperandS, seg_base + offset, size, 0)
+ hvm_copy_from_guest_virt(poperandS, base, size, 0)
!= HVMCOPY_okay )
return X86EMUL_EXCEPTION;
- decode->mem = seg_base + offset;
+ decode->mem = base;
decode->len = size;
}
[-- Attachment #2: vvmx-decode-insn.patch --]
[-- Type: text/plain, Size: 2215 bytes --]
vvmx: fix instruction decode segment limit check
- no limit check for 64-bit mode (and GS: is not special in any way)
- limit check is needed in compatibility mode
- canonical address check should instead be performed for 64-bit mode
- the last accessed byte must be within limits, not the first byte past
the accessed range
- segment base address should be ignored for 64-bit mode unless FS: or
GS: is in use
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -336,8 +336,17 @@ static int decode_vmx_inst(struct cpu_us
}
else
{
+ bool_t mode_64bit = 0;
+
decode->type = VMX_INST_MEMREG_TYPE_MEMORY;
- if ( info.fields.segment > 5 )
+
+ if ( hvm_long_mode_enabled(v) )
+ {
+ hvm_get_segment_register(v, x86_seg_cs, &seg);
+ mode_64bit = seg.attr.fields.l;
+ }
+
+ if ( info.fields.segment > VMX_SREG_GS )
goto gp_fault;
hvm_get_segment_register(v, sreg_to_index[info.fields.segment], &seg);
seg_base = seg.base;
@@ -355,15 +364,20 @@ static int decode_vmx_inst(struct cpu_us
size = 1 << (info.fields.addr_size + 1);
offset = base + index * scale + disp;
- if ( (offset > seg.limit || offset + size > seg.limit) &&
- (!hvm_long_mode_enabled(v) || info.fields.segment == VMX_SREG_GS) )
+ base = !mode_64bit || info.fields.segment >= VMX_SREG_FS ?
+ seg_base + offset : offset;
+ if ( offset + size - 1 < offset ||
+ (mode_64bit ?
+ !is_canonical_address((long)base < 0 ? base :
+ base + size - 1) :
+ offset + size - 1 > seg.limit) )
goto gp_fault;
if ( poperandS != NULL &&
- hvm_copy_from_guest_virt(poperandS, seg_base + offset, size, 0)
+ hvm_copy_from_guest_virt(poperandS, base, size, 0)
!= HVMCOPY_okay )
return X86EMUL_EXCEPTION;
- decode->mem = seg_base + offset;
+ decode->mem = base;
decode->len = size;
}
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH v2] vvmx: fix instruction decode segment limit check
2012-04-25 7:48 [PATCH v2] vvmx: fix instruction decode segment limit check Jan Beulich
@ 2012-04-25 8:29 ` Tim Deegan
0 siblings, 0 replies; 2+ messages in thread
From: Tim Deegan @ 2012-04-25 8:29 UTC (permalink / raw)
To: Jan Beulich; +Cc: Eddie Dong, xen-devel
At 08:48 +0100 on 25 Apr (1335343691), Jan Beulich wrote:
> - no limit check for 64-bit mode (and GS: is not special in any way)
> - limit check is needed in compatibility mode
> - canonical address check should instead be performed for 64-bit mode
> - the last accessed byte must be within limits, not the first byte past
> the accessed range
> - segment base address should be ignored for 64-bit mode unless FS: or
> GS: is in use
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Much clearer, thanks.
Acked-by: Tim Deegan <tim@xen.org>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2012-04-25 8:29 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-04-25 7:48 [PATCH v2] vvmx: fix instruction decode segment limit check Jan Beulich
2012-04-25 8:29 ` Tim Deegan
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).