On 22.04.2024 20:14, Andrew Cooper wrote:
> --- a/xen/arch/x86/alternative.c
> +++ b/xen/arch/x86/alternative.c
> @@ -244,10 +244,31 @@ static void init_or_livepatch
> _apply_alternatives(struct alt_instr *start,
>
> memcpy(buf, repl, a->repl_len);
>
> + /* Walk buf[] and adjust any insn-relative operands. */
> + if ( a->repl_len )
> {
> - /* 0xe8/0xe9 are relative branches; fix the offset. */
> - if ( a->repl_len >= 5 && (*buf & 0xfe) == 0xe8 )
> + uint8_t *ip = buf, *end = ip + a->repl_len;
> +
> + for ( x86_decode_lite_t res; ip < end; ip += res.len )
> {
> + int32_t *d32;
> + uint8_t *target;
> +
> + res = x86_decode_lite(ip, end);
> +
> + if ( res.len <= 0 )
> + {
> + printk("Alternative for %ps [%*ph]\n",
> + ALT_ORIG_PTR(a), a->repl_len, repl);
> + printk("Unable to decode instruction in alternative -
> ignoring.\n");
> + goto skip_this_alternative;
Can this really be just a log message? There are cases where patching has
to happen for things to operate correctly. Hence if not panic()ing, I'd
say we at least want to taint the hypervisor.
> @@ -317,14 +338,23 @@ static void init_or_livepatch
> _apply_alternatives(struct alt_instr *start,
> */
> goto skip_this_alternative;
> }
> +
> + continue;
> }
> - else if ( force && system_state < SYS_STATE_active )
> - ASSERT_UNREACHABLE();
This (and the other one below) is related to altcall patching, which you
say you mean to leave alone: During the 2nd pass, no un-processed CALL /
JMP should occur anymore that aren't altcall related.
Jan
