|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 1 of 3] KEXEC: Add new hypercall to fix 64/32bit truncation errors. v2
>>> On 22.12.11 at 18:36, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
> Currently, KEXEC_CMD_kexec_get_range in compat mode, or with 32bit
> Xen, will truncate 64bit pointers to 32bits, leading to problems on
> machines with more than 4GiB of RAM. As 32bit dom0 kernels tend to be
> PAE capable, this is especially wasteful, as most structures currently
> limited to <4GiB could easily be <64GiB instead.
>
> Therefore, introduce a new hypercall 'KEXEC_CMD_kexec64_get_range'
> which passes similar information as KEXEC_CMD_kexec_get_range, but
> which uses a structure with explicitly specified field widths, causing
> it to be identical in the compat and regular case. This new
> structure, xen_kexec64_range_t, will be the same as xen_kexec_range_t
> if Xen is compiled for x86_64, but will still use 64bit integers if
> Xen is compiled for x86_32.
>
> To fix 32bit Xen which uses 32bit integers for addresses and sizes,
> change the internals to use xen_kexec64_range_t which will use 64bit
> integers instead. This also involves changing several casts to
> explicitly use uint64_ts rather than unsigned longs.
Just for the record - I continue to be opposed to doing this for the
32-bit hypervisor. All relevant allocations are below 4G there, so
there's simply no need to decrease code readability for no benefit.
Jan
> In addition, the hypercall entry points need updating to be able to
> cater for all possibilities.
>
> |Xen/dom0 bits| Bit width of addresses in structure for |
> +------+------+---------------------------+-----------------------------+
> | Xen | dom0 | KEXEC_CMD_kexec_get_range | KEXEC_CMD_kexec64_get_range |
> +------+------+---------------------------+-----------------------------+
> | 32 | 32 | 32 | 64 |
> | 64 | 32 | 32 | 64 |
> | 64 | 64 | 64 | 64 |
> +------+------+---------------------------+-----------------------------+
>
> This has been solved by splitting do_kexec_op_internal back into
> do_kexec_op{,_compat} and changing kexec_get_range{,_compat} to
> kexec_get_range{32,64} which now exist irrespective of CONFIG_COMPAT
> or not.
>
> The knock-on effect of removing do_kexec_op_internal means that
> kexec_load_unload_compat is only ever called from inside an #ifdef
> CONFIG_COMPAT codepath, which does not exist on Xen x86_32.
> Therefore, move the #ifdef CONFIG_COMPAT guards to include the entire
> function.
>
> Finally, and a little unrelatedly, fix KEXEC_CMD_kexec_{load,unload}
> to return -EBUSY instead of EOK if a kexec is in progress.
>
> Changes since v1:
> * Fix check for pointer truncation to work when Xen is compiled for
> 32 bit mode as well.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>
> diff -r e56500f95b6a -r f571dc8e4368 xen/arch/ia64/xen/machine_kexec.c
> --- a/xen/arch/ia64/xen/machine_kexec.c
> +++ b/xen/arch/ia64/xen/machine_kexec.c
> @@ -102,10 +102,10 @@ void machine_reboot_kexec(xen_kexec_imag
> machine_kexec(image);
> }
>
> -int machine_kexec_get_xen(xen_kexec_range_t *range)
> +int machine_kexec_get_xen(xen_kexec64_range_t *range)
> {
> range->start = ia64_tpa(_text);
> - range->size = (unsigned long)_end - (unsigned long)_text;
> + range->size = (uint64_t)_end - (uint64_t)_text;
> return 0;
> }
>
> @@ -113,31 +113,31 @@ int machine_kexec_get_xen(xen_kexec_rang
> #define ELF_PAGE_SIZE (__IA64_UL_CONST(1) << ELF_PAGE_SHIFT)
> #define ELF_PAGE_MASK (~(ELF_PAGE_SIZE - 1))
>
> -static int machine_kexec_get_xenheap(xen_kexec_range_t *range)
> +static int machine_kexec_get_xenheap(xen_kexec64_range_t *range)
> {
> range->start = (ia64_tpa(_end) + (ELF_PAGE_SIZE - 1)) & ELF_PAGE_MASK;
> range->size =
> - (((unsigned long)range->start + KERNEL_TR_PAGE_SIZE) &
> + (((uint64_t)range->start + KERNEL_TR_PAGE_SIZE) &
> ~(KERNEL_TR_PAGE_SIZE - 1))
> - - (unsigned long)range->start;
> + - (uint64_t)range->start;
> return 0;
> }
>
> -static int machine_kexec_get_boot_param(xen_kexec_range_t *range)
> +static int machine_kexec_get_boot_param(xen_kexec64_range_t *range)
> {
> range->start = __pa(ia64_boot_param);
> range->size = sizeof(*ia64_boot_param);
> return 0;
> }
>
> -static int machine_kexec_get_efi_memmap(xen_kexec_range_t *range)
> +static int machine_kexec_get_efi_memmap(xen_kexec64_range_t *range)
> {
> range->start = ia64_boot_param->efi_memmap;
> range->size = ia64_boot_param->efi_memmap_size;
> return 0;
> }
>
> -int machine_kexec_get(xen_kexec_range_t *range)
> +int machine_kexec_get(xen_kexec64_range_t *range)
> {
> switch (range->range) {
> case KEXEC_RANGE_MA_XEN:
> diff -r e56500f95b6a -r f571dc8e4368 xen/arch/x86/machine_kexec.c
> --- a/xen/arch/x86/machine_kexec.c
> +++ b/xen/arch/x86/machine_kexec.c
> @@ -120,7 +120,7 @@ void machine_kexec(xen_kexec_image_t *im
> }
> }
>
> -int machine_kexec_get(xen_kexec_range_t *range)
> +int machine_kexec_get(xen_kexec64_range_t *range)
> {
> if (range->range != KEXEC_RANGE_MA_XEN)
> return -EINVAL;
> diff -r e56500f95b6a -r f571dc8e4368 xen/arch/x86/x86_32/machine_kexec.c
> --- a/xen/arch/x86/x86_32/machine_kexec.c
> +++ b/xen/arch/x86/x86_32/machine_kexec.c
> @@ -11,11 +11,11 @@
> #include <asm/page.h>
> #include <public/kexec.h>
>
> -int machine_kexec_get_xen(xen_kexec_range_t *range)
> +int machine_kexec_get_xen(xen_kexec64_range_t *range)
> {
> range->start = virt_to_maddr(_start);
> - range->size = (unsigned long)xenheap_phys_end -
> - (unsigned long)range->start;
> + range->size = (uint64_t)xenheap_phys_end -
> + (uint64_t)range->start;
> return 0;
> }
>
> diff -r e56500f95b6a -r f571dc8e4368 xen/arch/x86/x86_64/machine_kexec.c
> --- a/xen/arch/x86/x86_64/machine_kexec.c
> +++ b/xen/arch/x86/x86_64/machine_kexec.c
> @@ -11,10 +11,10 @@
> #include <asm/page.h>
> #include <public/kexec.h>
>
> -int machine_kexec_get_xen(xen_kexec_range_t *range)
> +int machine_kexec_get_xen(xen_kexec64_range_t *range)
> {
> range->start = virt_to_maddr(_start);
> - range->size = virt_to_maddr(_end) - (unsigned long)range->start;
> + range->size = virt_to_maddr(_end) - (uint64_t)range->start;
> return 0;
> }
>
> diff -r e56500f95b6a -r f571dc8e4368 xen/common/kexec.c
> --- a/xen/common/kexec.c
> +++ b/xen/common/kexec.c
> @@ -56,6 +56,14 @@ static struct {
> unsigned long size;
> } ranges[16] __initdata;
>
> +/* This XLAT macro is required now even without CONFIG_COMPAT. */
> +#define TRANSLATE_kexec_range(_d_, _s_) do { \
> + (_d_)->range = (_s_)->range; \
> + (_d_)->nr = (_s_)->nr; \
> + (_d_)->size = (_s_)->size; \
> + (_d_)->start = (_s_)->start; \
> +} while (0)
> +
> /*
> * Parse command lines in the format
> *
> @@ -280,7 +288,7 @@ static int sizeof_note(const char *name,
> ELFNOTE_ALIGN(descsz));
> }
>
> -static int kexec_get_reserve(xen_kexec_range_t *range)
> +static int kexec_get_reserve(xen_kexec64_range_t *range)
> {
> if ( kexec_crash_area.size > 0 && kexec_crash_area.start > 0) {
> range->start = kexec_crash_area.start;
> @@ -291,7 +299,7 @@ static int kexec_get_reserve(xen_kexec_r
> return 0;
> }
>
> -static int kexec_get_cpu(xen_kexec_range_t *range)
> +static int kexec_get_cpu(xen_kexec64_range_t *range)
> {
> int nr = range->nr;
> int nr_bytes = 0;
> @@ -335,14 +343,14 @@ static int kexec_get_cpu(xen_kexec_range
> return 0;
> }
>
> -static int kexec_get_vmcoreinfo(xen_kexec_range_t *range)
> +static int kexec_get_vmcoreinfo(xen_kexec64_range_t *range)
> {
> range->start = __pa((unsigned long)vmcoreinfo_data);
> range->size = VMCOREINFO_BYTES;
> return 0;
> }
>
> -static int kexec_get_range_internal(xen_kexec_range_t *range)
> +static int kexec_get_range_internal(xen_kexec64_range_t *range)
> {
> int ret = -EINVAL;
>
> @@ -365,9 +373,14 @@ static int kexec_get_range_internal(xen_
> return ret;
> }
>
> -static int kexec_get_range(XEN_GUEST_HANDLE(void) uarg)
> +/* This function can be invoked from either a KEXEC_CMD_kexec64_get_range
> + * hypercall, or from a KEXEC_CMD_kexec_get_range hypercall with 64bit dom0
> + * on 64bit Xen. In the first case, the guest structure is a
> + * xen_kexec64_range_t, and in the second case, the xen_kexec_range_t guest
> + * structure is identical to xen_kexec64_range_t. */
> +static int kexec_get_range64(XEN_GUEST_HANDLE(void) uarg)
> {
> - xen_kexec_range_t range;
> + xen_kexec64_range_t range;
> int ret = -EINVAL;
>
> if ( unlikely(copy_from_guest(&range, uarg, 1)) )
> @@ -381,34 +394,40 @@ static int kexec_get_range(XEN_GUEST_HAN
> return ret;
> }
>
> -static int kexec_get_range_compat(XEN_GUEST_HANDLE(void) uarg)
> +/* This function can be invoked from either a KEXEC_CMD_kexec_get_range
> + * compat hypercall for 32bit dom0 on 64bit Xen, or from the same hypercall
> + * on 32bit Xen. In both cases, the guest argument uses 32bit integers, so
>
> + * translate them to 64bit for use by kexec_get_range_internal. The
> + * preprocessor guards are to choose the correct 32bit structure, as the
> + * compat_* structures dont exist in 32bit Xen. */
> +static int kexec_get_range32(XEN_GUEST_HANDLE(void) uarg)
> {
> + xen_kexec64_range_t range64;
> #ifdef CONFIG_COMPAT
> - xen_kexec_range_t range;
> - compat_kexec_range_t compat_range;
> + compat_kexec_range_t range32;
> +#else
> + xen_kexec_range_t range32;
> +#endif
> int ret = -EINVAL;
>
> - if ( unlikely(copy_from_guest(&compat_range, uarg, 1)) )
> + if ( unlikely(copy_from_guest(&range32, uarg, 1)) )
> return -EFAULT;
>
> - XLAT_kexec_range(&range, &compat_range);
> + TRANSLATE_kexec_range(&range64, &range32);
>
> - ret = kexec_get_range_internal(&range);
> + ret = kexec_get_range_internal(&range64);
>
> /* Dont silently truncate physical addresses or sizes. */
> - if ( (range.start | range.size) & ~(unsigned long)(~0u) )
> + if ( (range64.start | range64.size) & 0xffffffff00000000ULL )
> return -ERANGE;
>
> if ( ret == 0 ) {
> - XLAT_kexec_range(&compat_range, &range);
> - if ( unlikely(copy_to_guest(uarg, &compat_range, 1)) )
> + TRANSLATE_kexec_range(&range32, &range64);
> + if ( unlikely(copy_to_guest(uarg, &range32, 1)) )
> return -EFAULT;
> }
>
> return ret;
> -#else /* CONFIG_COMPAT */
> - return 0;
> -#endif /* CONFIG_COMPAT */
> }
>
> static int kexec_load_get_bits(int type, int *base, int *bit)
> @@ -543,10 +562,10 @@ static int kexec_load_unload(unsigned lo
> return kexec_load_unload_internal(op, &load);
> }
>
> +#ifdef CONFIG_COMPAT
> static int kexec_load_unload_compat(unsigned long op,
> XEN_GUEST_HANDLE(void) uarg)
> {
> -#ifdef CONFIG_COMPAT
> compat_kexec_load_t compat_load;
> xen_kexec_load_t load;
>
> @@ -564,10 +583,8 @@ static int kexec_load_unload_compat(unsi
> XLAT_kexec_image(&load.image, &compat_load.image);
>
> return kexec_load_unload_internal(op, &load);
> -#else /* CONFIG_COMPAT */
> - return 0;
> +}
> #endif /* CONFIG_COMPAT */
> -}
>
> static int kexec_exec(XEN_GUEST_HANDLE(void) uarg)
> {
> @@ -601,8 +618,51 @@ static int kexec_exec(XEN_GUEST_HANDLE(v
> return -EINVAL; /* never reached */
> }
>
> -int do_kexec_op_internal(unsigned long op, XEN_GUEST_HANDLE(void) uarg,
> - int compat)
> +long do_kexec_op(unsigned long op, XEN_GUEST_HANDLE(void) uarg)
> +{
> + unsigned long flags;
> + int ret = -EINVAL;
> +
> + if ( !IS_PRIV(current->domain) )
> + return -EPERM;
> +
> + ret = xsm_kexec();
> + if ( ret )
> + return ret;
> +
> + switch ( op )
> + {
> +#ifdef __i386__
> + case KEXEC_CMD_kexec_get_range:
> + ret = kexec_get_range32(uarg);
> + break;
> +#else
> + case KEXEC_CMD_kexec_get_range:
> +#endif
> + case KEXEC_CMD_kexec64_get_range:
> + ret = kexec_get_range64(uarg);
> + break;
> +
> + case KEXEC_CMD_kexec_load:
> + case KEXEC_CMD_kexec_unload:
> + spin_lock_irqsave(&kexec_lock, flags);
> + if (!test_bit(KEXEC_FLAG_IN_PROGRESS, &kexec_flags))
> + ret = kexec_load_unload(op, uarg);
> + else
> + ret = -EBUSY;
> + spin_unlock_irqrestore(&kexec_lock, flags);
> + break;
> +
> + case KEXEC_CMD_kexec:
> + ret = kexec_exec(uarg);
> + break;
> + }
> +
> + return ret;
> +}
> +
> +#ifdef CONFIG_COMPAT
> +int compat_kexec_op(unsigned long op, XEN_GUEST_HANDLE(void) uarg)
> {
> unsigned long flags;
> int ret = -EINVAL;
> @@ -617,23 +677,21 @@ int do_kexec_op_internal(unsigned long o
> switch ( op )
> {
> case KEXEC_CMD_kexec_get_range:
> - if (compat)
> - ret = kexec_get_range_compat(uarg);
> - else
> - ret = kexec_get_range(uarg);
> + ret = kexec_get_range32(uarg);
> + break;
> + case KEXEC_CMD_kexec64_get_range:
> + ret = kexec_get_range64(uarg);
> break;
> case KEXEC_CMD_kexec_load:
> case KEXEC_CMD_kexec_unload:
> spin_lock_irqsave(&kexec_lock, flags);
> if (!test_bit(KEXEC_FLAG_IN_PROGRESS, &kexec_flags))
> - {
> - if (compat)
> - ret = kexec_load_unload_compat(op, uarg);
> - else
> - ret = kexec_load_unload(op, uarg);
> - }
> + ret = kexec_load_unload_compat(op, uarg);
> + else
> + ret = -EBUSY;
> spin_unlock_irqrestore(&kexec_lock, flags);
> break;
> +
> case KEXEC_CMD_kexec:
> ret = kexec_exec(uarg);
> break;
> @@ -641,17 +699,6 @@ int do_kexec_op_internal(unsigned long o
>
> return ret;
> }
> -
> -long do_kexec_op(unsigned long op, XEN_GUEST_HANDLE(void) uarg)
> -{
> - return do_kexec_op_internal(op, uarg, 0);
> -}
> -
> -#ifdef CONFIG_COMPAT
> -int compat_kexec_op(unsigned long op, XEN_GUEST_HANDLE(void) uarg)
> -{
> - return do_kexec_op_internal(op, uarg, 1);
> -}
> #endif
>
> /*
> diff -r e56500f95b6a -r f571dc8e4368 xen/include/public/kexec.h
> --- a/xen/include/public/kexec.h
> +++ b/xen/include/public/kexec.h
> @@ -155,6 +155,15 @@ typedef struct xen_kexec_range {
> unsigned long start;
> } xen_kexec_range_t;
>
> +#define KEXEC_CMD_kexec64_get_range 4
> +/* xen_kexec_range_t using explicit sizes for fields. */
> +typedef struct xen_kexec64_range {
> + int32_t range;
> + int32_t nr;
> + uint64_t size;
> + uint64_t start;
> +} xen_kexec64_range_t;
> +
> #endif /* _XEN_PUBLIC_KEXEC_H */
>
> /*
> diff -r e56500f95b6a -r f571dc8e4368 xen/include/xen/kexec.h
> --- a/xen/include/xen/kexec.h
> +++ b/xen/include/xen/kexec.h
> @@ -34,8 +34,8 @@ void kexec_crash(void);
> void kexec_crash_save_cpu(void);
> crash_xen_info_t *kexec_crash_save_info(void);
> void machine_crash_shutdown(void);
> -int machine_kexec_get(xen_kexec_range_t *range);
> -int machine_kexec_get_xen(xen_kexec_range_t *range);
> +int machine_kexec_get(xen_kexec64_range_t *range);
> +int machine_kexec_get_xen(xen_kexec64_range_t *range);
>
> void compat_machine_kexec(unsigned long rnk,
> unsigned long indirection_page,
> diff -r e56500f95b6a -r f571dc8e4368 xen/include/xlat.lst
> --- a/xen/include/xlat.lst
> +++ b/xen/include/xlat.lst
> @@ -50,9 +50,7 @@
> ? grant_entry_v1 grant_table.h
> ? grant_entry_header grant_table.h
> ? grant_entry_v2 grant_table.h
> -? kexec_exec kexec.h
> ! kexec_image kexec.h
> -! kexec_range kexec.h
> ! add_to_physmap memory.h
> ! foreign_memory_map memory.h
> ! memory_exchange memory.h
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |