* [PATCH v7] Sanity check xsave area when migrating or restoring from older Xen verions
@ 2014-10-23 20:49 Don Koch
2014-10-23 21:59 ` Andrew Cooper
0 siblings, 1 reply; 2+ messages in thread
From: Don Koch @ 2014-10-23 20:49 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Keir Fraser, Don Koch, Jan Beulich
Xen 4.3.0, 4.2.3 and older transferred a maximum sized xsave area (as
if all the available XCR0 bits were set); the new version only
transfers based on the actual XCR0 bits. This may result in a smaller
area if the last sections were missing (e.g., the LWP area from an AMD
machine). If the size doesn't match the XCR0 derived size, the size is
checked against the maximum size and the part of the xsave area
between the actual and maximum used size is checked for zero data. If
either the max size check or any part of the overflow area is
non-zero, we return with an error.
Signed-off-by: Don Koch <dkoch@verizon.com>
---
Changes in V7:
- Note more specific older versions
- Correct copy length
Changes since v6:
- Error message shuffle.
Changes in V5:
- Back to one message before non-zero check, on in case of non-zero
- Change vcpuid format to %u
- Change offsets to %#x
- Added better comment regarding bug (using Andrew's suggestion)
Changes in V4:
- Removed check of size base on xfeature_mask.
- Unsign some ints.
- Change %d to %u for unsigned ints.
- Move printk to only print if non-zero data found.
Changes in V3:
- use h->data for zero check
- remove max size check (use size that was sent)
- fix error message (drop first byte value)
- fix "for" issues
Changes in V2:
- Add check for size.
- Add check for non-zero data in unused part of block.
xen/arch/x86/hvm/hvm.c | 32 ++++++++++++++++++++------------
1 file changed, 20 insertions(+), 12 deletions(-)
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index f0e1edc..8d27128 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1971,6 +1971,7 @@ static int hvm_load_cpu_xsave_states(struct domain *d, hvm_domain_context_t *h)
struct vcpu *v;
struct hvm_hw_cpu_xsave *ctxt;
struct hvm_save_descriptor *desc;
+ unsigned int i, desc_start;
/* Which vcpu is this? */
vcpuid = hvm_load_instance(h);
@@ -2011,15 +2012,8 @@ static int hvm_load_cpu_xsave_states(struct domain *d, hvm_domain_context_t *h)
save_area) + XSTATE_AREA_MIN_SIZE);
return -EINVAL;
}
- size = HVM_CPU_XSAVE_SIZE(xfeature_mask);
- if ( desc->length > size )
- {
- printk(XENLOG_G_WARNING
- "HVM%d.%d restore mismatch: xsave length %u > %u\n",
- d->domain_id, vcpuid, desc->length, size);
- return -EOPNOTSUPP;
- }
h->cur += sizeof (*desc);
+ desc_start = h->cur;
ctxt = (struct hvm_hw_cpu_xsave *)&h->data[h->cur];
h->cur += desc->length;
@@ -2038,10 +2032,24 @@ static int hvm_load_cpu_xsave_states(struct domain *d, hvm_domain_context_t *h)
size = HVM_CPU_XSAVE_SIZE(ctxt->xcr0_accum);
if ( desc->length > size )
{
+ /*
+ * Xen 4.3.0, 4.2.3 and older used to send longer-than-needed
+ * xsave regions. Permit loading the record if the extra data
+ * is all zero.
+ */
+ for ( i = size; i < desc->length; i++ )
+ {
+ if ( h->data[desc_start + i] )
+ {
+ printk(XENLOG_G_WARNING
+ "HVM%d.%u restore mismatch: xsave length %#x > %#x (non-zero data at %#x)\n",
+ d->domain_id, vcpuid, desc->length, size, i);
+ return -EOPNOTSUPP;
+ }
+ }
printk(XENLOG_G_WARNING
- "HVM%d.%d restore mismatch: xsave length %u > %u\n",
+ "HVM%d.%u restore mismatch: xsave length %#x > %#x\n",
d->domain_id, vcpuid, desc->length, size);
- return -EOPNOTSUPP;
}
/* Checking finished */
@@ -2049,8 +2057,8 @@ static int hvm_load_cpu_xsave_states(struct domain *d, hvm_domain_context_t *h)
v->arch.xcr0_accum = ctxt->xcr0_accum;
if ( ctxt->xcr0_accum & XSTATE_NONLAZY )
v->arch.nonlazy_xstate_used = 1;
- memcpy(v->arch.xsave_area, &ctxt->save_area,
- desc->length - offsetof(struct hvm_hw_cpu_xsave, save_area));
+ memcpy(v->arch.xsave_area, &ctxt->save_area, min(desc->length, size)
+ - offsetof(struct hvm_hw_cpu_xsave, save_area));
return 0;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: [PATCH v7] Sanity check xsave area when migrating or restoring from older Xen verions
2014-10-23 20:49 [PATCH v7] Sanity check xsave area when migrating or restoring from older Xen verions Don Koch
@ 2014-10-23 21:59 ` Andrew Cooper
0 siblings, 0 replies; 2+ messages in thread
From: Andrew Cooper @ 2014-10-23 21:59 UTC (permalink / raw)
To: Don Koch, xen-devel; +Cc: Keir Fraser, Jan Beulich
On 23/10/2014 21:49, Don Koch wrote:
> Xen 4.3.0, 4.2.3 and older transferred a maximum sized xsave area (as
> if all the available XCR0 bits were set); the new version only
> transfers based on the actual XCR0 bits. This may result in a smaller
> area if the last sections were missing (e.g., the LWP area from an AMD
> machine). If the size doesn't match the XCR0 derived size, the size is
> checked against the maximum size and the part of the xsave area
> between the actual and maximum used size is checked for zero data. If
> either the max size check or any part of the overflow area is
> non-zero, we return with an error.
>
> Signed-off-by: Don Koch <dkoch@verizon.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> Changes in V7:
> - Note more specific older versions
> - Correct copy length
>
> Changes since v6:
> - Error message shuffle.
>
> Changes in V5:
> - Back to one message before non-zero check, on in case of non-zero
> - Change vcpuid format to %u
> - Change offsets to %#x
> - Added better comment regarding bug (using Andrew's suggestion)
>
> Changes in V4:
> - Removed check of size base on xfeature_mask.
> - Unsign some ints.
> - Change %d to %u for unsigned ints.
> - Move printk to only print if non-zero data found.
>
> Changes in V3:
> - use h->data for zero check
> - remove max size check (use size that was sent)
> - fix error message (drop first byte value)
> - fix "for" issues
>
> Changes in V2:
> - Add check for size.
> - Add check for non-zero data in unused part of block.
>
> xen/arch/x86/hvm/hvm.c | 32 ++++++++++++++++++++------------
> 1 file changed, 20 insertions(+), 12 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index f0e1edc..8d27128 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -1971,6 +1971,7 @@ static int hvm_load_cpu_xsave_states(struct domain *d, hvm_domain_context_t *h)
> struct vcpu *v;
> struct hvm_hw_cpu_xsave *ctxt;
> struct hvm_save_descriptor *desc;
> + unsigned int i, desc_start;
>
> /* Which vcpu is this? */
> vcpuid = hvm_load_instance(h);
> @@ -2011,15 +2012,8 @@ static int hvm_load_cpu_xsave_states(struct domain *d, hvm_domain_context_t *h)
> save_area) + XSTATE_AREA_MIN_SIZE);
> return -EINVAL;
> }
> - size = HVM_CPU_XSAVE_SIZE(xfeature_mask);
> - if ( desc->length > size )
> - {
> - printk(XENLOG_G_WARNING
> - "HVM%d.%d restore mismatch: xsave length %u > %u\n",
> - d->domain_id, vcpuid, desc->length, size);
> - return -EOPNOTSUPP;
> - }
> h->cur += sizeof (*desc);
> + desc_start = h->cur;
>
> ctxt = (struct hvm_hw_cpu_xsave *)&h->data[h->cur];
> h->cur += desc->length;
> @@ -2038,10 +2032,24 @@ static int hvm_load_cpu_xsave_states(struct domain *d, hvm_domain_context_t *h)
> size = HVM_CPU_XSAVE_SIZE(ctxt->xcr0_accum);
> if ( desc->length > size )
> {
> + /*
> + * Xen 4.3.0, 4.2.3 and older used to send longer-than-needed
> + * xsave regions. Permit loading the record if the extra data
> + * is all zero.
> + */
> + for ( i = size; i < desc->length; i++ )
> + {
> + if ( h->data[desc_start + i] )
> + {
> + printk(XENLOG_G_WARNING
> + "HVM%d.%u restore mismatch: xsave length %#x > %#x (non-zero data at %#x)\n",
> + d->domain_id, vcpuid, desc->length, size, i);
> + return -EOPNOTSUPP;
> + }
> + }
> printk(XENLOG_G_WARNING
> - "HVM%d.%d restore mismatch: xsave length %u > %u\n",
> + "HVM%d.%u restore mismatch: xsave length %#x > %#x\n",
> d->domain_id, vcpuid, desc->length, size);
> - return -EOPNOTSUPP;
> }
> /* Checking finished */
>
> @@ -2049,8 +2057,8 @@ static int hvm_load_cpu_xsave_states(struct domain *d, hvm_domain_context_t *h)
> v->arch.xcr0_accum = ctxt->xcr0_accum;
> if ( ctxt->xcr0_accum & XSTATE_NONLAZY )
> v->arch.nonlazy_xstate_used = 1;
> - memcpy(v->arch.xsave_area, &ctxt->save_area,
> - desc->length - offsetof(struct hvm_hw_cpu_xsave, save_area));
> + memcpy(v->arch.xsave_area, &ctxt->save_area, min(desc->length, size)
> + - offsetof(struct hvm_hw_cpu_xsave, save_area));
>
> return 0;
> }
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2014-10-23 21:59 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-10-23 20:49 [PATCH v7] Sanity check xsave area when migrating or restoring from older Xen verions Don Koch
2014-10-23 21:59 ` Andrew Cooper
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.