KEXEC: fix kexec_get_range_compat to fail vocally

KEXEC: fix kexec_get_range_compat to fail vocally

[Andrew Cooper]

[-- Attachment #1: Type: text/plain, Size: 250 bytes --]

I am not sure that this is the only instance, but it is really not
acceptable to hand truncated pointers or sizes for physical memory to dom0.

-- 
Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer
T: +44 (0)1223 225 900, http://www.citrix.com


[-- Attachment #2: KEXEC-fix-kexec_get_range_compat.patch --]
[-- Type: text/x-patch, Size: 820 bytes --]

KEXEC: fix kexec_get_range_compat to fail vocally.

Fail with -ERANGE rather than silently truncating 64bit values (a
physical address and size) into 32bit integers for dom0 to consume.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

diff -r df7cec2c6c03 xen/common/kexec.c
--- a/xen/common/kexec.c
+++ b/xen/common/kexec.c
@@ -395,6 +395,12 @@ static int kexec_get_range_compat(XEN_GU
 
     ret = kexec_get_range_internal(&range);
 
+#define RANGE_MASK (((unsigned long)-1) & ~((unsigned int)-1))
+    /* Dont silently truncate physical addresses or sizes. */
+    if ( range.start & RANGE_MASK || range.size & RANGE_MASK )
+        return -ERANGE;
+#undef RANGE_MASK
+
     if ( ret == 0 ) {
         XLAT_kexec_range(&compat_range, &range);
         if ( unlikely(copy_to_guest(uarg, &compat_range, 1)) )

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: KEXEC: fix kexec_get_range_compat to fail vocally

[Jan Beulich]

>>> On 05.12.11 at 12:35, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> I am not sure that this is the only instance, but it is really not
> acceptable to hand truncated pointers or sizes for physical memory to dom0.

While I agree to the change, I think we also ought to add a new flavor
of KEXEC_CMD_kexec_get_range that is 64-bit capable even for a
32-bit Dom0.

Both under the assumption that apart from the truncation nothing
prevents addresses above the 4G boundary from being usable with
a 32-bit kernel for at least one of the ranges (probably only
KEXEC_RANGE_MA_CPU and KEXEC_RANGE_MA_VMCOREINFO are
candidates for this on x86).

Jan

Re: KEXEC: fix kexec_get_range_compat to fail vocally

[Andrew Cooper]

On 05/12/11 12:58, Jan Beulich wrote:
>>>> On 05.12.11 at 12:35, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>> I am not sure that this is the only instance, but it is really not
>> acceptable to hand truncated pointers or sizes for physical memory to dom0.
> While I agree to the change, I think we also ought to add a new flavor
> of KEXEC_CMD_kexec_get_range that is 64-bit capable even for a
> 32-bit Dom0.

Agreed - I am working on introducing a new hypercall right now.

> Both under the assumption that apart from the truncation nothing
> prevents addresses above the 4G boundary from being usable with
> a 32-bit kernel for at least one of the ranges (probably only
> KEXEC_RANGE_MA_CPU and KEXEC_RANGE_MA_VMCOREINFO are
> candidates for this on x86).

Will those ranges not be covered by my patch?

> Jan
>

-- 
Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer
T: +44 (0)1223 225 900, http://www.citrix.com

Re: KEXEC: fix kexec_get_range_compat to fail vocally

[Jan Beulich]

>>> On 05.12.11 at 14:01, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> On 05/12/11 12:58, Jan Beulich wrote:
>> Both under the assumption that apart from the truncation nothing
>> prevents addresses above the 4G boundary from being usable with
>> a 32-bit kernel for at least one of the ranges (probably only
>> KEXEC_RANGE_MA_CPU and KEXEC_RANGE_MA_VMCOREINFO are
>> candidates for this on x86).
> 
> Will those ranges not be covered by my patch?

That wasn't the question. My point was that your change and the new
sub-hypercall are useful only if any of these ranges can validly live
above 4G (i.e. if we don't need to instead constrain the allocations).

Jan

Re: KEXEC: fix kexec_get_range_compat to fail vocally

[Andrew Cooper]

On 05/12/11 13:18, Jan Beulich wrote:
>>>> On 05.12.11 at 14:01, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>> On 05/12/11 12:58, Jan Beulich wrote:
>>> Both under the assumption that apart from the truncation nothing
>>> prevents addresses above the 4G boundary from being usable with
>>> a 32-bit kernel for at least one of the ranges (probably only
>>> KEXEC_RANGE_MA_CPU and KEXEC_RANGE_MA_VMCOREINFO are
>>> candidates for this on x86).
>> Will those ranges not be covered by my patch?
> That wasn't the question. My point was that your change and the new
> sub-hypercall are useful only if any of these ranges can validly live
> above 4G (i.e. if we don't need to instead constrain the allocations).
>
> Jan

Ah ok - I misunderstood.  As part of my planned changes to the kexec
path, there will be a command line parameter to request all kexec
related allocs are below a specified address.  My intention is that this
would typically be set to either 64GB if you use a 32bit PAE crash
kernel, or 4G for using a 32bit non-PAE kernel.  Certain other
allocations which would be covered by this parameter would be the
console ring, so the crash kernel can pull it out of memory and dump it
to disk.

We need this functionality for XenServer as we use a 32bit PAE crash
kernel, but it does have an impact on how many 32bit PV guests you can
boot on a large memory machine.  At the moment, there is a hack^H "fix"
from before my time in the XS patch queue to allocate crash notes in
Xen's BSS so it falls into the Xen mapped region of dom0 memory, but has
the problem of needing all the notes to be compile time constant
lengths, which reduces flexibility.  (It was also to work around an old
bug in kexec tools where it would consider the "Crash Notes" invalid if
they were not explicitly in a System Ram section - this appears fixed now.)

-- 
Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer
T: +44 (0)1223 225 900, http://www.citrix.com

Re: KEXEC: fix kexec_get_range_compat to fail vocally

[Keir Fraser]

On 05/12/2011 11:35, "Andrew Cooper" <andrew.cooper3@citrix.com> wrote:

> I am not sure that this is the only instance, but it is really not
> acceptable to hand truncated pointers or sizes for physical memory to dom0.

I applied as c/s 24358 but I changed the mask-checking arithmetic a bit. Not
only is it now shorter, I'm also more certain that it is correct: It looks
to me like ~((unsigned int)-1) performs the ~ before promotion to unsigned
long, hence this term ends up as 0 rather than 0xffffffff00000000.

 -- Keir