On Tue, Aug 23, 2016 at 10:22:12PM -0400, Konrad Rzeszutek Wilk wrote:
> From: Ross Lagerwall
>
> Add hook functions which run during patch apply and patch revert.
> Hook functions are used by livepatch payloads to manipulate data
> structures during patching, etc.
>
> One use case is the XSA91. As Martin mentions it:
> "If we have shadow variables, we also need an unload hook to garbage
> collect all the variables introduced by a hotpatch to prevent memory
> leaks. Potentially, we also want to pre-reserve memory for static or
> existing dynamic objects in the load-hook instead of on the fly.
>
> For testing and debugging, various applications are possible.
>
> In general, the hooks provide flexibility when having to deal with
> unforeseen cases, but their application should be rarely required (<
> 10%)."
>
> Furthermore include a test-case for it.
>
> Signed-off-by: Ross Lagerwall
> Signed-off-by: Konrad Rzeszutek Wilk
So... anybody willing to review it :-)
>
> ---
> Cc: Andrew Cooper
> Cc: George Dunlap
> Cc: Ian Jackson
> Cc: Jan Beulich
> Cc: Stefano Stabellini
> Cc: Tim Deegan
> Cc: Wei Liu
In regards to this going in v4.8 my recollection is that:
George: 0
Andrew: +1
Jan: 0 (with a slight leaning towards -1)?
I think that means folks are OK or 'don't care'.
And the livepatch maintainers:
Ross: +1 (obviously since he wrote it)
Konrad: +1
>
> v0.4..v0.11: Defered for v4.8
> v0.12: s/xsplice/livepatch/
>
> v3: Clarify the comments about spin_debug_enable
> Rename one of the hooks to lower-case (Z->z) to guarantee it being
> called last.
> Learned a lot of about 'const'
> v4: Do the actual const of the hooks.
> Remove the requirement for the tests-case hooks to be sorted
> (they never were to start with).
> Fix up the comment about spin_debug_enable so more.
> ---
> docs/misc/livepatch.markdown| 23 +
> xen/arch/x86/test/xen_hello_world.c | 34 +
> xen/common/livepatch.c | 50
> -
> xen/include/xen/livepatch_payload.h | 49
> 4 files changed, 155 insertions(+), 1 deletion(-)
> create mode 100644 xen/include/xen/livepatch_payload.h
>
> diff --git a/docs/misc/livepatch.markdown b/docs/misc/livepatch.markdown
> index ad0df26..9f52aeb 100644
> --- a/docs/misc/livepatch.markdown
> +++ b/docs/misc/livepatch.markdown
> @@ -343,6 +343,13 @@ When reverting a patch, the hypervisor iterates over
> each `livepatch_func`
> and the core code copies the data from the undo buffer (private internal
> copy)
> to `old_addr`.
>
> +It optionally may contain the address of functions to be called right before
> +being applied and after being reverted:
> +
> + * `.livepatch.hooks.load` - an array of function pointers.
> + * `.livepatch.hooks.unload` - an array of function pointers.
> +
> +
> ### Example of .livepatch.funcs
>
> A simple example of what a payload file can be:
> @@ -380,6 +387,22 @@ struct livepatch_func livepatch_hello_world = {
>
> Code must be compiled with -fPIC.
>
> +### .livepatch.hooks.load and .livepatch.hooks.unload
> +
> +This section contains an array of function pointers to be executed
> +before payload is being applied (.livepatch.funcs) or after reverting
> +the payload. This is useful to prepare data structures that need to
> +be modified patching.
> +
> +Each entry in this array is eight bytes.
> +
> +The type definition of the function are as follow:
> +
> +
> +typedef void (*livepatch_loadcall_t)(void);
> +typedef void (*livepatch_unloadcall_t)(void);
> +
> +
> ### .livepatch.depends and .note.gnu.build-id
>
> To support dependencies checking and safe loading (to load the
> diff --git a/xen/arch/x86/test/xen_hello_world.c
> b/xen/arch/x86/test/xen_hello_world.c
> index 422bdf1..cb5e27a 100644
> --- a/xen/arch/x86/test/xen_hello_world.c
> +++ b/xen/arch/x86/test/xen_hello_world.c
> @@ -4,14 +4,48 @@
> */
>
> #include "config.h"
> +#include
> #include
> #include
> #include
> +#include
>
> #include
>
> static char hello_world_patch_this_fnc[] = "xen_extra_version";
> extern const char *xen_hello_world(void);
> +static unsigned int cnt;
> +
> +static void apply_hook(void)
> +{
> +printk(KERN_DEBUG "Hook executing.\n");
> +}
> +
> +static void revert_hook(void)
> +{
> +printk(KERN_DEBUG "Hook unloaded.\n");
> +}
> +
> +static void hi_func(void)
> +{
> +printk(KERN_DEBUG "%s: Hi! (called %u times)\n", __func__, ++cnt);
> +};
> +
> +static void check_fnc(void)
> +{
> +printk(KERN_DEBUG "%s: Hi func called %u times\n", __func__, cnt);
> +BUG_ON(cnt == 0 || cnt > 2);
> +}
> +
> +LIVEPATCH_LOAD_HOOK(apply_hook);
> +LIVEPATCH_UNLOAD_HOOK(revert_hook);
> +
> +/* Imbalance here. Two load and three unload. */
> +
> +LIVEPATCH_LOAD_HOOK(hi_func);
> +LIVEPATCH_UNLOAD_HOOK(hi_func);
> +
> +LIVEPATCH_UNLOAD_HOOK(check_fnc);
>
> struct livepatch_func __s
