Re: [PATCH 4/9] kexec: extend hypercall with improved load/unload ops

[David Vrabel <david.vrabel@citrix.com>]

On 04/10/13 22:23, Daniel Kiper wrote:
> On Fri, Sep 20, 2013 at 02:10:50PM +0100, David Vrabel wrote:
>> --- /dev/null
>> +++ b/xen/arch/x86/x86_64/kexec_reloc.S
>> @@ -0,0 +1,208 @@
[...]
>> +ENTRY(kexec_reloc)
>> +        /* %rdi - code page maddr */
>> +        /* %rsi - page table maddr */
>> +        /* %rdx - indirection page maddr */
>> +        /* %rcx - entry maddr */
>> +        /* %r8 - flags */
>> +
>> +        movq %rdx, %rbx
> 
> Delete movq %rdx, %rbx

We avoid using %rdx in case we need to re-add the UART debugging.

>> +        /* Need to switch to 32-bit mode? */
>> +        testq $KEXEC_RELOC_FLAG_COMPAT, %r8
>> +        jnz call_32_bit
>> +
>> +call_64_bit:
>> +        /* Call the image entry point.  This should never return. */
> 
> I think that all general purpose registers (including %rsi, %rdi, %rbp
> and %rsp) should be zeroed here. We should leave as little as possible
> info about previous system. Especially in kexec case. Just in case.
> Please look into linux/arch/x86/kernel/relocate_kernel_64.S
> for more details.

Not initializing the registers is a deliberate design decision so exec'd
images cannot mistakenly rely on the register values.

Clearing a handful of words when all of host memory is accessible by the
exec'd image does nothing for security (as you suggest in a later email).

>> +        callq *%rcx
> 
> Maybe we should use retq to jump into image entry point. If not
> I think that we should store image entry point address in %rax
> (just to the order).

With call if an image does try to return it will fault at a well defined
point (the following ud2) making the failure a bit easier to diagnose.
With your suggestion it will fault somewhere random.

Linux uses call.

David