Re: [PATCH 02/10] x86: add support for relative CALL and JMP in alternatives

[Masami Hiramatsu <mhiramat@redhat.com>]

Luca Barbieri wrote:
> Currently CALL and JMP cannot be used in alternatives because the
> relative offset would be wrong.
> 
> This patch uses the existing x86 instruction parser to parse the
> alternative sequence and fix up the displacements.
> 
> This allows to implement this feature with minimal code.
> 
> Signed-off-by: Luca Barbieri <luca@luca-barbieri.com>
> ---
>  arch/x86/kernel/alternative.c |   22 ++++++++++++++++++++++
>  1 files changed, 22 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
> index de7353c..7464c7e 100644
> --- a/arch/x86/kernel/alternative.c
> +++ b/arch/x86/kernel/alternative.c
> @@ -17,6 +17,7 @@
>  #include <asm/tlbflush.h>
>  #include <asm/io.h>
>  #include <asm/fixmap.h>
> +#include <asm/insn.h>
>  
>  #define MAX_PATCH_LEN (255-1)
>  
> @@ -195,6 +196,26 @@ extern struct alt_instr __alt_instructions[], __alt_instructions_end[];
>  extern u8 *__smp_locks[], *__smp_locks_end[];
>  static void *text_poke_early(void *addr, const void *opcode, size_t len);
>  

Hmm, first of all, if those alternative codes are hand-written, I think
you don't need to use insn-decoder because you can expect what code
will be there. (or, as Peter said, it should be done in compile-time, because
the adjustment can be determined on each site...)
Anyway, I have some comments on it.

> +/* Fix call instruction displacements */
> +static void __init_or_module fixup_relative_addresses(char *buf, size_t size, size_t adj)
> +{
> +	struct insn insn;
> +	char *p = buf;
> +	char *end = p + size;
> +	while (p < end) {
> +		kernel_insn_init(&insn, p);
> +		insn_get_opcode(&insn);

Here, if you need to know the destination address encoded as an imm operand,
you can use insn_get_immediate() for that.

> +
> +		/* CALL or JMP near32 */
> +		if (insn.opcode.bytes[0] == 0xe8 || insn.opcode.bytes[0] == 0xe9) {
> +			size_t disp_off = insn.next_byte - insn.kaddr;
> +			*(unsigned *)(p + disp_off) += adj;

And also, you can use insn_offset_immediate() instead of disp_off, here.
Please see fix_riprel() in arch/x86/kernel/kprobes.c.

Thank you,

-- 
Masami Hiramatsu
e-mail: mhiramat@redhat.com