diff --git a/MAINTAINERS b/MAINTAINERS index 97720a82b8..9b3060085a 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -270,6 +270,7 @@ F: docs/misc/livepatch.markdown F: tools/misc/xen-livepatch.c F: xen/arch/*/livepatch* F: xen/common/livepatch* +F: xen/include/asm-*/livepatch.h F: xen/include/xen/livepatch* MACHINE CHECK (MCA) & RAS diff --git a/docs/misc/livepatch.markdown b/docs/misc/livepatch.markdown index a674037e15..f22eeabe62 100644 --- a/docs/misc/livepatch.markdown +++ b/docs/misc/livepatch.markdown @@ -318,13 +318,24 @@ The size of the structure is 64 bytes on 64-bit hypervisors. It will be payload generation time if hypervisor function address is known. If unknown, the value *MUST* be zero and the hypervisor will attempt to resolve the address. -* `new_addr` is the address of the function that is replacing the old - function. The address is filled in during relocation. The value **MUST** be - the address of the new function in the file. +* `new_addr` can either have a non-zero value or be zero. + * If there is a non-zero value, then it is the address of the function that is + replacing the old function and the address is recomputed during relocation. + The value **MUST** be the address of the new function in the payload file. -* `old_size` and `new_size` contain the sizes of the respective functions in bytes. + * If the value is zero, then we NOPing out at the `old_addr` location + `new_size` bytes. + +* `old_size` contains the sizes of the respective `old_addr` function in bytes. The value of `old_size` **MUST** not be zero. +* `new_size` depends on what `new_addr` contains: + * If `new_addr` contains an non-zero value, then `new_size` has the size of + the new function (which will replace the one at `old_addr`) in bytes. + * If the value of `new_addr` is zero then `new_size` determines how many + instruction bytes to NOP (up to opaque size modulo smallest platform + instruction - 1 byte x86 and 4 bytes on ARM). + * `version` is to be one. * `opaque` **MUST** be zero. @@ -1087,7 +1098,8 @@ limit that calls the next trampoline. Please note there is a small limitation for trampolines in function entries: The target function (+ trailing padding) must be able to accomodate the trampoline. On x86 with +-2 GB relative jumps, -this means 5 bytes are required. +this means 5 bytes are required which means that `old_size` **MUST** be +at least five bytes if patching in trampoline. Depending on compiler settings, there are several functions in Xen that are smaller (without inter-function padding). diff --git a/xen/arch/x86/alternative.c b/xen/arch/x86/alternative.c index 05e3eb8cf8..6eaa10f858 100644 --- a/xen/arch/x86/alternative.c +++ b/xen/arch/x86/alternative.c @@ -101,7 +101,7 @@ static void __init arch_init_ideal_nops(void) } /* Use this to add nops to a buffer, then text_poke the whole buffer. */ -static void init_or_livepatch add_nops(void *insns, unsigned int len) +void init_or_livepatch add_nops(void *insns, unsigned int len) { while ( len > 0 ) { diff --git a/xen/arch/x86/livepatch.c b/xen/arch/x86/livepatch.c index 56da1545f2..d5e7174fbc 100644 --- a/xen/arch/x86/livepatch.c +++ b/xen/arch/x86/livepatch.c @@ -12,8 +12,7 @@ #include #include - -#define PATCH_INSN_SIZE 5 +#include int arch_livepatch_quiesce(void) { @@ -31,11 +30,17 @@ void arch_livepatch_revive(void) int arch_livepatch_verify_func(const struct livepatch_func *func) { - /* No NOP patching yet. */ - if ( !func->new_size ) - return -EOPNOTSUPP; + /* If NOPing.. */ + if ( !func->new_addr ) + { + /* Only do up to maximum amount we can put in the ->opaque. */ + if ( func->new_size > sizeof(func->opaque) ) + return -EOPNOTSUPP; - if ( func->old_size < PATCH_INSN_SIZE ) + if ( func->old_size < func->new_size ) + return -EINVAL; + } + else if ( func->old_size < ARCH_PATCH_INSN_SIZE ) return -EINVAL; return 0; @@ -43,23 +48,36 @@ int arch_livepatch_verify_func(const struct livepatch_func *func) void arch_livepatch_apply_jmp(struct livepatch_func *func) { - int32_t val; uint8_t *old_ptr; - - BUILD_BUG_ON(PATCH_INSN_SIZE > sizeof(func->opaque)); - BUILD_BUG_ON(PATCH_INSN_SIZE != (1 + sizeof(val))); + uint8_t insn[sizeof(func->opaque)]; + unsigned int len; old_ptr = func->old_addr; - memcpy(func->opaque, old_ptr, PATCH_INSN_SIZE); + len = livepatch_insn_len(func); + if ( !len ) + return; - *old_ptr++ = 0xe9; /* Relative jump */ - val = func->new_addr - func->old_addr - PATCH_INSN_SIZE; - memcpy(old_ptr, &val, sizeof(val)); + memcpy(func->opaque, old_ptr, len); + if ( func->new_addr ) + { + int32_t val; + + BUILD_BUG_ON(ARCH_PATCH_INSN_SIZE != (1 + sizeof(val))); + + insn[0] = 0xe9; /* Relative jump. */ + val = func->new_addr - func->old_addr - ARCH_PATCH_INSN_SIZE; + + memcpy(&insn[1], &val, sizeof(val)); + } + else + add_nops(insn, len); + + memcpy(old_ptr, insn, len); } void arch_livepatch_revert_jmp(const struct livepatch_func *func) { - memcpy(func->old_addr, func->opaque, PATCH_INSN_SIZE); + memcpy(func->old_addr, func->opaque, livepatch_insn_len(func)); } /* Serialise the CPU pipeline. */ diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c index f5ce28c726..13346d7956 100644 --- a/xen/common/livepatch.c +++ b/xen/common/livepatch.c @@ -523,7 +523,8 @@ static int prepare_payload(struct payload *payload, return -EOPNOTSUPP; } - if ( !f->new_addr || !f->new_size ) + /* 'old_addr', 'new_addr', 'new_size' can all be zero. */ + if ( !f->old_size ) { dprintk(XENLOG_ERR, LIVEPATCH "%s: Address or size fields are zero!\n", elf->name); diff --git a/xen/include/asm-x86/alternative.h b/xen/include/asm-x86/alternative.h index 67fc0d2418..db4f08e0e7 100644 --- a/xen/include/asm-x86/alternative.h +++ b/xen/include/asm-x86/alternative.h @@ -27,6 +27,7 @@ struct alt_instr { #define ALT_ORIG_PTR(a) __ALT_PTR(a, instr_offset) #define ALT_REPL_PTR(a) __ALT_PTR(a, repl_offset) +extern void add_nops(void *insns, unsigned int len); /* Similar to alternative_instructions except it can be run with IRQs enabled. */ extern void apply_alternatives(const struct alt_instr *start, const struct alt_instr *end); diff --git a/xen/include/asm-x86/livepatch.h b/xen/include/asm-x86/livepatch.h new file mode 100644 index 0000000000..5e04aa1afb --- /dev/null +++ b/xen/include/asm-x86/livepatch.h @@ -0,0 +1,21 @@ +/* + * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved. + * + */ + +#ifndef __XEN_X86_LIVEPATCH_H__ +#define __XEN_X86_LIVEPATCH_H__ + +#define ARCH_PATCH_INSN_SIZE 5 + +#endif /* __XEN_X86_LIVEPATCH_H__ */ + +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * tab-width: 4 + * indent-tabs-mode: nil + * End: + */ diff --git a/xen/include/xen/livepatch.h b/xen/include/xen/livepatch.h index 29c9b3141b..174af06103 100644 --- a/xen/include/xen/livepatch.h +++ b/xen/include/xen/livepatch.h @@ -68,7 +68,17 @@ int arch_livepatch_secure(const void *va, unsigned int pages, enum va_type types void arch_livepatch_init(void); #include /* For struct livepatch_func. */ +#include int arch_livepatch_verify_func(const struct livepatch_func *func); + +static inline +unsigned int livepatch_insn_len(const struct livepatch_func *func) +{ + if ( !func->new_addr ) + return func->new_size; + + return ARCH_PATCH_INSN_SIZE; +} /* * These functions are called around the critical region patching live code, * for an architecture to take make appropratie global state adjustments.