The NOP functionality will NOP any of the code at the 'old_addr' or at 'name' if the 'new_addr' and 'new_size' are both zero. The purpose of this is to NOP out calls, such as:
e9 <4-bytes-offset> (5 byte insn), or on ARM a 4 byte insn for branching. We need the EIP of where we need to the NOP, and that can be provided via the `old_addr` or `name`. If the `old_addr` is provided we will NOP 5 instructions (on x86) at that location. If `name` is provided with the symbol+0x<offset/<len> we make sure that <len> is 5 (on x86) and upon retrieving the EIP based on `name` will NOP that location. While at it, also unify the code on x86 patching so it is a bit simpler (instead of two seperate writes just make it one memcpy). Signed-off-by: Konrad Rzeszutek Wilk <konrad.w...@oracle.com> --- Cc: Konrad Rzeszutek Wilk <konrad.w...@oracle.com> Cc: Ross Lagerwall <ross.lagerw...@citrix.com> Cc: Jan Beulich <jbeul...@suse.com> Cc: Andrew Cooper <andrew.coop...@citrix.com> v3: First submission --- docs/misc/livepatch.markdown | 6 ++++-- xen/arch/x86/alternative.c | 2 +- xen/arch/x86/livepatch.c | 23 ++++++++++++++--------- xen/common/livepatch.c | 12 ++++++++---- xen/include/asm-x86/alternative.h | 1 + 5 files changed, 28 insertions(+), 16 deletions(-) diff --git a/docs/misc/livepatch.markdown b/docs/misc/livepatch.markdown index f1a5147..590757d 100644 --- a/docs/misc/livepatch.markdown +++ b/docs/misc/livepatch.markdown @@ -320,10 +320,12 @@ The size of the structure is 64 bytes on 64-bit hypervisors. It will be * `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. + either the address of the new function in the file, or zero if we are NOPing out + at `old_addr` (and `new_size` **MUST** be zero). * `old_size` and `new_size` contain the sizes of the respective functions in bytes. - The value of `old_size` **MUST** not be zero. + The value of `old_size` **MUST** not be zero. If the value of `new_size` is _zero_ + the code at location of `old_addr` (or `name` when resolved) will be NOPed out. * `version` is to be one. diff --git a/xen/arch/x86/alternative.c b/xen/arch/x86/alternative.c index be40b13..fd8528e 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 df64b00..cabd0c1 100644 --- a/xen/arch/x86/livepatch.c +++ b/xen/arch/x86/livepatch.c @@ -34,10 +34,6 @@ int arch_verify_insn_length(unsigned long len) int arch_livepatch_verify_func(const struct livepatch_func *func) { - /* No NOP patching yet. */ - if ( !func->new_size ) - return -EOPNOTSUPP; - if ( func->old_size < PATCH_INSN_SIZE ) return -EINVAL; @@ -46,18 +42,27 @@ 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; + uint8_t insn[PATCH_INSN_SIZE]; BUILD_BUG_ON(PATCH_INSN_SIZE > sizeof(func->opaque)); - BUILD_BUG_ON(PATCH_INSN_SIZE != (1 + sizeof(val))); old_ptr = func->old_addr; memcpy(func->opaque, old_ptr, PATCH_INSN_SIZE); - *old_ptr++ = 0xe9; /* Relative jump */ - val = func->new_addr - func->old_addr - PATCH_INSN_SIZE; - memcpy(old_ptr, &val, sizeof(val)); + if ( func->new_size ) + { + int32_t val; + + BUILD_BUG_ON(PATCH_INSN_SIZE != (1 + sizeof(val))); + + insn[0] = 0xe9; + val = func->new_addr - func->old_addr - PATCH_INSN_SIZE; + memcpy(&insn[1], &val, sizeof(val)); + } else + add_nops(&insn, PATCH_INSN_SIZE); + + memcpy(old_ptr, insn, PATCH_INSN_SIZE); } void arch_livepatch_revert_jmp(const struct livepatch_func *func) diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c index e752949..6f82a9e 100644 --- a/xen/common/livepatch.c +++ b/xen/common/livepatch.c @@ -561,11 +561,15 @@ static int prepare_payload(struct payload *payload, return -EOPNOTSUPP; } - if ( !f->new_addr || !f->new_size ) + /* If both are zero then we are NOPing. */ + if ( (!f->new_addr || !f->new_size) ) { - dprintk(XENLOG_ERR, LIVEPATCH "%s: Address or size fields are zero!\n", - elf->name); - return -EINVAL; + if ( f->new_addr || f->new_size ) + { + dprintk(XENLOG_ERR, LIVEPATCH "%s: Address or size fields are zero!\n", + elf->name); + return -EINVAL; + } } rc = arch_livepatch_verify_func(f); diff --git a/xen/include/asm-x86/alternative.h b/xen/include/asm-x86/alternative.h index bce959f..acaeded 100644 --- a/xen/include/asm-x86/alternative.h +++ b/xen/include/asm-x86/alternative.h @@ -23,6 +23,7 @@ struct alt_instr { u8 replacementlen; /* length of new instruction, <= instrlen */ }; +extern void add_nops(void *insns, unsigned int len); /* Similar to apply_alternatives except it can be run with IRQs enabled. */ extern void apply_alternatives_nocheck(struct alt_instr *start, struct alt_instr *end); -- 2.4.11 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel