* [PATCH] x86/HVM: linear address must be canonical for the whole accessed range
@ 2013-09-20 13:07 Jan Beulich
2013-09-20 15:30 ` Keir Fraser
0 siblings, 1 reply; 2+ messages in thread
From: Jan Beulich @ 2013-09-20 13:07 UTC (permalink / raw)
To: xen-devel; +Cc: Keir Fraser
[-- Attachment #1: Type: text/plain, Size: 2955 bytes --]
... rather than just for the first byte.
While at it, also
- make the real mode case at least dpo a wrap around check
- drop the mis-named "gpf" label (we're not generating faults here)
and use in-place returns instead
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1931,8 +1931,7 @@ int hvm_virtual_to_linear_addr(
unsigned int addr_size,
unsigned long *linear_addr)
{
- unsigned long addr = offset;
- uint32_t last_byte;
+ unsigned long addr = offset, last_byte;
if ( !(current->arch.hvm_vcpu.guest_cr[0] & X86_CR0_PE) )
{
@@ -1941,6 +1940,9 @@ int hvm_virtual_to_linear_addr(
* Certain of them are not done in native real mode anyway.
*/
addr = (uint32_t)(addr + reg->base);
+ last_byte = (uint32_t)addr + bytes - 1;
+ if ( last_byte < addr )
+ return 0;
}
else if ( addr_size != 64 )
{
@@ -1952,17 +1954,17 @@ int hvm_virtual_to_linear_addr(
{
case hvm_access_read:
if ( (reg->attr.fields.type & 0xa) == 0x8 )
- goto gpf; /* execute-only code segment */
+ return 0; /* execute-only code segment */
break;
case hvm_access_write:
if ( (reg->attr.fields.type & 0xa) != 0x2 )
- goto gpf; /* not a writable data segment */
+ return 0; /* not a writable data segment */
break;
default:
break;
}
- last_byte = offset + bytes - 1;
+ last_byte = (uint32_t)offset + bytes - 1;
/* Is this a grows-down data segment? Special limit check if so. */
if ( (reg->attr.fields.type & 0xc) == 0x4 )
@@ -1973,10 +1975,10 @@ int hvm_virtual_to_linear_addr(
/* Check first byte and last byte against respective bounds. */
if ( (offset <= reg->limit) || (last_byte < offset) )
- goto gpf;
+ return 0;
}
else if ( (last_byte > reg->limit) || (last_byte < offset) )
- goto gpf; /* last byte is beyond limit or wraps 0xFFFFFFFF */
+ return 0; /* last byte is beyond limit or wraps 0xFFFFFFFF */
/*
* Hardware truncates to 32 bits in compatibility mode.
@@ -1993,15 +1995,14 @@ int hvm_virtual_to_linear_addr(
if ( (seg == x86_seg_fs) || (seg == x86_seg_gs) )
addr += reg->base;
- if ( !is_canonical_address(addr) )
- goto gpf;
+ last_byte = addr + bytes - 1;
+ if ( !is_canonical_address(addr) || last_byte < addr ||
+ !is_canonical_address(last_byte) )
+ return 0;
}
*linear_addr = addr;
return 1;
-
- gpf:
- return 0;
}
/* On non-NULL return, we leave this function holding an additional
[-- Attachment #2: x86-HVM-linear-canonical.patch --]
[-- Type: text/plain, Size: 3023 bytes --]
x86/HVM: linear address must be canonical for the whole accessed range
... rather than just for the first byte.
While at it, also
- make the real mode case at least dpo a wrap around check
- drop the mis-named "gpf" label (we're not generating faults here)
and use in-place returns instead
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1931,8 +1931,7 @@ int hvm_virtual_to_linear_addr(
unsigned int addr_size,
unsigned long *linear_addr)
{
- unsigned long addr = offset;
- uint32_t last_byte;
+ unsigned long addr = offset, last_byte;
if ( !(current->arch.hvm_vcpu.guest_cr[0] & X86_CR0_PE) )
{
@@ -1941,6 +1940,9 @@ int hvm_virtual_to_linear_addr(
* Certain of them are not done in native real mode anyway.
*/
addr = (uint32_t)(addr + reg->base);
+ last_byte = (uint32_t)addr + bytes - 1;
+ if ( last_byte < addr )
+ return 0;
}
else if ( addr_size != 64 )
{
@@ -1952,17 +1954,17 @@ int hvm_virtual_to_linear_addr(
{
case hvm_access_read:
if ( (reg->attr.fields.type & 0xa) == 0x8 )
- goto gpf; /* execute-only code segment */
+ return 0; /* execute-only code segment */
break;
case hvm_access_write:
if ( (reg->attr.fields.type & 0xa) != 0x2 )
- goto gpf; /* not a writable data segment */
+ return 0; /* not a writable data segment */
break;
default:
break;
}
- last_byte = offset + bytes - 1;
+ last_byte = (uint32_t)offset + bytes - 1;
/* Is this a grows-down data segment? Special limit check if so. */
if ( (reg->attr.fields.type & 0xc) == 0x4 )
@@ -1973,10 +1975,10 @@ int hvm_virtual_to_linear_addr(
/* Check first byte and last byte against respective bounds. */
if ( (offset <= reg->limit) || (last_byte < offset) )
- goto gpf;
+ return 0;
}
else if ( (last_byte > reg->limit) || (last_byte < offset) )
- goto gpf; /* last byte is beyond limit or wraps 0xFFFFFFFF */
+ return 0; /* last byte is beyond limit or wraps 0xFFFFFFFF */
/*
* Hardware truncates to 32 bits in compatibility mode.
@@ -1993,15 +1995,14 @@ int hvm_virtual_to_linear_addr(
if ( (seg == x86_seg_fs) || (seg == x86_seg_gs) )
addr += reg->base;
- if ( !is_canonical_address(addr) )
- goto gpf;
+ last_byte = addr + bytes - 1;
+ if ( !is_canonical_address(addr) || last_byte < addr ||
+ !is_canonical_address(last_byte) )
+ return 0;
}
*linear_addr = addr;
return 1;
-
- gpf:
- return 0;
}
/* On non-NULL return, we leave this function holding an additional
[-- 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] x86/HVM: linear address must be canonical for the whole accessed range
2013-09-20 13:07 [PATCH] x86/HVM: linear address must be canonical for the whole accessed range Jan Beulich
@ 2013-09-20 15:30 ` Keir Fraser
0 siblings, 0 replies; 2+ messages in thread
From: Keir Fraser @ 2013-09-20 15:30 UTC (permalink / raw)
To: Jan Beulich, xen-devel
On 20/09/2013 14:07, "Jan Beulich" <JBeulich@suse.com> wrote:
> ... rather than just for the first byte.
>
> While at it, also
> - make the real mode case at least dpo a wrap around check
> - drop the mis-named "gpf" label (we're not generating faults here)
> and use in-place returns instead
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Keir Fraser <keir@xen.org>
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -1931,8 +1931,7 @@ int hvm_virtual_to_linear_addr(
> unsigned int addr_size,
> unsigned long *linear_addr)
> {
> - unsigned long addr = offset;
> - uint32_t last_byte;
> + unsigned long addr = offset, last_byte;
>
> if ( !(current->arch.hvm_vcpu.guest_cr[0] & X86_CR0_PE) )
> {
> @@ -1941,6 +1940,9 @@ int hvm_virtual_to_linear_addr(
> * Certain of them are not done in native real mode anyway.
> */
> addr = (uint32_t)(addr + reg->base);
> + last_byte = (uint32_t)addr + bytes - 1;
> + if ( last_byte < addr )
> + return 0;
> }
> else if ( addr_size != 64 )
> {
> @@ -1952,17 +1954,17 @@ int hvm_virtual_to_linear_addr(
> {
> case hvm_access_read:
> if ( (reg->attr.fields.type & 0xa) == 0x8 )
> - goto gpf; /* execute-only code segment */
> + return 0; /* execute-only code segment */
> break;
> case hvm_access_write:
> if ( (reg->attr.fields.type & 0xa) != 0x2 )
> - goto gpf; /* not a writable data segment */
> + return 0; /* not a writable data segment */
> break;
> default:
> break;
> }
>
> - last_byte = offset + bytes - 1;
> + last_byte = (uint32_t)offset + bytes - 1;
>
> /* Is this a grows-down data segment? Special limit check if so. */
> if ( (reg->attr.fields.type & 0xc) == 0x4 )
> @@ -1973,10 +1975,10 @@ int hvm_virtual_to_linear_addr(
>
> /* Check first byte and last byte against respective bounds. */
> if ( (offset <= reg->limit) || (last_byte < offset) )
> - goto gpf;
> + return 0;
> }
> else if ( (last_byte > reg->limit) || (last_byte < offset) )
> - goto gpf; /* last byte is beyond limit or wraps 0xFFFFFFFF */
> + return 0; /* last byte is beyond limit or wraps 0xFFFFFFFF */
>
> /*
> * Hardware truncates to 32 bits in compatibility mode.
> @@ -1993,15 +1995,14 @@ int hvm_virtual_to_linear_addr(
> if ( (seg == x86_seg_fs) || (seg == x86_seg_gs) )
> addr += reg->base;
>
> - if ( !is_canonical_address(addr) )
> - goto gpf;
> + last_byte = addr + bytes - 1;
> + if ( !is_canonical_address(addr) || last_byte < addr ||
> + !is_canonical_address(last_byte) )
> + return 0;
> }
>
> *linear_addr = addr;
> return 1;
> -
> - gpf:
> - return 0;
> }
>
> /* On non-NULL return, we leave this function holding an additional
>
>
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2013-09-20 15:30 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-20 13:07 [PATCH] x86/HVM: linear address must be canonical for the whole accessed range Jan Beulich
2013-09-20 15:30 ` Keir Fraser
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).