On 18.02.26 09:21, Juergen Gross wrote:
> Today alt_replace_call() requires the initial indirect call not to be
> followed by any further instructions, including padding NOPs. In case
> any replacement is longer than 6 bytes, a subsequent replacement of
> the indirect call with a direct one will result in a crash.
>
> Fix that by crashing only if the original instruction is less than
> 6 bytes long or not a known indirect call.
>
> Signed-off-by: Juergen Gross <jgross@suse.com>
Any chance to get some feedback on this one? I believe it should go in,
even if the rest of the series isn't taken.
Juergen
> ---
> V3:
> - new patch
>
> arch/x86/kernel/alternative.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
> index 693b59b2f7d0..6d37672ba71f 100644
> --- a/arch/x86/kernel/alternative.c
> +++ b/arch/x86/kernel/alternative.c
> @@ -534,6 +534,7 @@ noinstr void BUG_func(void)
> }
> EXPORT_SYMBOL(BUG_func);
>
> +#define CALL_RIP_INSTRLEN 6
> #define CALL_RIP_REL_OPCODE 0xff
> #define CALL_RIP_REL_MODRM 0x15
>
> @@ -551,7 +552,7 @@ static unsigned int alt_replace_call(u8 *instr, u8 *insn_buff, struct alt_instr
> BUG();
> }
>
> - if (a->instrlen != 6 ||
> + if (a->instrlen < CALL_RIP_INSTRLEN ||
> instr[0] != CALL_RIP_REL_OPCODE ||
> instr[1] != CALL_RIP_REL_MODRM) {
> pr_err("ALT_FLAG_DIRECT_CALL set for unrecognized indirect call\n");
> @@ -563,7 +564,7 @@ static unsigned int alt_replace_call(u8 *instr, u8 *insn_buff, struct alt_instr
> #ifdef CONFIG_X86_64
> /* ff 15 00 00 00 00 call *0x0(%rip) */
> /* target address is stored at "next instruction + disp". */
> - target = *(void **)(instr + a->instrlen + disp);
> + target = *(void **)(instr + CALL_RIP_INSTRLEN + disp);
> #else
> /* ff 15 00 00 00 00 call *0x0 */
> /* target address is stored at disp. */