Re: [PATCH v1 01/11] xsplice: Design document (v2).

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Tue, Nov 03, 2015 at 06:15:58PM +0000, Ross Lagerwall wrote:
> From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> 
> A mechanism is required to binarily patch the running hypervisor with new
> opcodes that have come about due to primarily security updates.
> 
> This document describes the design of the API that would allow us to
> upload to the hypervisor binary patches.
> 
> This document has been shaped by the input from:
>   Martin Pohlack <mpohlack@amazon.de>
>   Jan Beulich <jbeulich@suse.com>
> 
> Thank you!
> 
> Input-from: Martin Pohlack <mpohlack@amazon.de>
> Input-from: Jan Beulich <jbeulich@suse.com>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
> ---
>  docs/misc/xsplice.markdown | 999 +++++++++++++++++++++++++++++++++++++++++++++


9999 :-) What a nice number!

.. sniup..
> +## Design of payload format
> +
> +The payload **MUST** contain enough data to allow us to apply the update
> +and also safely reverse it. As such we **MUST** know:
> +
> + * The locations in memory to be patched. This can be determined dynamically
> +   via symbols or via virtual addresses.
> + * The new code that will be patched in.
> + * Signature to verify the payload.

Argh. We need to move the 'Signature to verify' in the 'v2' section
as I don't think we can get that done in time.

> +
> +This binary format can be constructed using an custom binary format but
> +there are severe disadvantages of it:
> +
> + * The format might need to be changed and we need an mechanism to accommodate
> +   that.
> + * It has to be platform agnostic.
> + * Easily constructed using existing tools.
> +
> +As such having the payload in an ELF file is the sensible way. We would be
> +carrying the various sets of structures (and data) in the ELF sections under
> +different names and with definitions. The prefix for the ELF section name
> +would always be: *.xsplice* to match up to the names of the structures.
> +
> +Note that every structure has padding. This is added so that the hypervisor
> +can re-use those fields as it sees fit.
> +
> +Earlier design attempted to ineptly explain the relations of the ELF sections
> +to each other without using proper ELF mechanism (sh_info, sh_link, data
> +structures using Elf types, etc). This design will explain in detail
> +the structures and how they are used together and not dig in the ELF
> +format - except mention that the section names should match the
> +structure names.
> +
> +The xSplice payload is a relocatable ELF binary. A typical binary would have:
> +
> + * One or more .text sections
> + * Zero or more read-only data sections
> + * Zero or more data sections
> + * Relocations for each of these sections
> +
> +It may also have some architecture-specific sections. For example:
> +
> + * Alternatives instructions
> + * Bug frames
> + * Exception tables
> + * Relocations for each of these sections
> +
> +The xSplice core code loads the payload as a standard ELF binary, relocates it
> +and handles the architecture-specifc sections as needed. This process is much
> +like what the Linux kernel module loader does. It contains no xSplice-specific
> +details and thus will not be discussed further.

What is 'it'? The 'process of what module loader does'?

> +
> +Importantly, the payload also contains a section with an array of structures
> +describing the functions to be patched:
> +<pre>
> +struct xsplice_patch_func {
> +    unsigned long new_addr;
> +    unsigned long new_size;
> +    unsigned long old_addr;
> +    unsigned long old_size;
> +    char *name;
> +    uint8_t pad[64];
> +};
> +<pre>

Uh, so 104 bytes ? Or did you mean to s/64/24/ so the structure is nicely
padded to 64-bytes?

I think that is what you meant.
> +
> +* `old_addr` is the address of the function to be patched and is filled in at
> +  compile time if the payload is statically linked and at run time if the
> +  payload is dynamically linked.
> +* `new_addr` is the address of the function that is replacing the old
> +  function. The address is filled in during relocation.
> +* `old_size` and `new_size` contain the sizes of the respective functions.
> +* `name` is used for looking up the old function address during dynamic
> +  linking.
> +
> +The size of the `xsplice_patch_func` array is determined from the ELF section
> +size.
> +
> +During patch apply, for each `xsplice_patch_func`, the core code inserts a
> +trampoline at `old_addr` to `new_addr`. During patch revert, for each
> +`xsplice_patch_func`, the core code copies the data from the undo buffer to
> +`old_addr`.
> +