Messages in this thread Patch in this message | | | From | Alexandre Chartre <> | Subject | [PATCH] x86/alternatives: check int3 breakpoint physical addresses | Date | Fri, 25 Jan 2019 15:57:29 +0100 |
| |
Modifying multi-byte instructions is achieved by temporarily replacing the first instruction to patch with an int3 instruction. Then, if an int3 interrupt is raised, the int3 handler will check if the interrupt was caused by this temporary patch by comparing the address that raises the interrupt (the interrupt address) and the address that was patched (the patched address).
The int3 handler compares virtual addresses of the interrupt and patched addresses. However, this doesn't work if the code to modify is mapped to multiple virtual addresses because, in that case, the patched virtual address can be different from the interrupt virtual address.
A simple solution is then to also compare physical addresses when virtual addresses do not match. Note that we still compare virtual addresses because that's a faster comparison than having to fully resolve and compare physical addresses.
Signed-off-by: Alexandre Chartre <alexandre.chartre@oracle.com> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: Ingo Molnar <mingo@kernel.org> Cc: Borislav Petkov <bp@alien8.de> Cc: H. Peter Anvin <hpa@zytor.com> Cc: Josh Poimboeuf <jpoimboe@redhat.com> Cc: x86@kernel.org ---
This is a potential issue and I don't know if it can be triggered with the current kernel: is there any code mapped to multiple virtual addresses which can be updated with text_poke_bp()? However as the patch is small and simple, it might be worth applying it anyway to prevent any such issue.
Note that this issue has been observed and reproduced with a custom kernel with some code mapped to different virtual addresses and using jump labels As jump labels use text_poke_bp(), crashes were sometimes observed when updating jump labels.
arch/x86/kernel/alternative.c | 13 ++++++++++++- 1 files changed, 12 insertions(+), 1 deletions(-)
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c index ebeac48..e563573 100644 --- a/arch/x86/kernel/alternative.c +++ b/arch/x86/kernel/alternative.c @@ -757,7 +757,18 @@ int poke_int3_handler(struct pt_regs *regs) if (likely(!bp_patching_in_progress)) return 0; - if (user_mode(regs) || regs->ip != (unsigned long)bp_int3_addr) + if (user_mode(regs)) + return 0; + + /* + * If virtual addresses are different, check if physical addresses + * are the same in case we have the same code mapped to different + * virtual addresses. Note that we could just compare physical + * addresses, however we first compare virtual addresses because + * this is much faster and very likely to succeed. + */ + if (regs->ip != (unsigned long)bp_int3_addr && + slow_virt_to_phys((void *)regs->ip) != slow_virt_to_phys(bp_int3_addr)) return 0; /* set up the specified breakpoint handler */ -- 1.7.1
| |