Message ID | 92120cfd-0b0b-0152-5296-9c6889d21687@suse.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | x86/traps: improve show_trace()'s top-of-stack handling | expand |
On 15/07/2019 16:00, Jan Beulich wrote: > Nothing guarantees that the original frame's stack pointer points at > readable memory. Avoid a (likely nested) crash by attaching exception > recovery to the read (making it a single read at the same time). Don't > even invoke _show_trace() in case of a non-readable top slot. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> > Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
On 23.09.2019 16:20, Andrew Cooper wrote: > On 15/07/2019 16:00, Jan Beulich wrote: >> Nothing guarantees that the original frame's stack pointer points at >> readable memory. Avoid a (likely nested) crash by attaching exception >> recovery to the read (making it a single read at the same time). Don't >> even invoke _show_trace() in case of a non-readable top slot. >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> >> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> With this, ... > Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> ... was this perhaps meant for patch 2 of this short series? Jan
On 23/09/2019 16:12, Jan Beulich wrote: > On 23.09.2019 16:20, Andrew Cooper wrote: >> On 15/07/2019 16:00, Jan Beulich wrote: >>> Nothing guarantees that the original frame's stack pointer points at >>> readable memory. Avoid a (likely nested) crash by attaching exception >>> recovery to the read (making it a single read at the same time). Don't >>> even invoke _show_trace() in case of a non-readable top slot. >>> >>> Signed-off-by: Jan Beulich <jbeulich@suse.com> >>> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> > With this, ... > >> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> > ... was this perhaps meant for patch 2 of this short series? No. I didn't spot my R-b tag and only had enough free time to read the first email. ~Andrew
--- a/xen/arch/x86/traps.c +++ b/xen/arch/x86/traps.c @@ -486,17 +486,31 @@ static void _show_trace(unsigned long sp static void show_trace(const struct cpu_user_regs *regs) { - unsigned long *sp = ESP_BEFORE_EXCEPTION(regs); + unsigned long *sp = ESP_BEFORE_EXCEPTION(regs), tos = 0; + bool fault = false; printk("Xen call trace:\n"); + /* Guarded read of the stack top. */ + asm ( "1: mov %[data], %[tos]; 2:\n" + ".pushsection .fixup,\"ax\"\n" + "3: movb $1, %[fault]; jmp 2b\n" + ".popsection\n" + _ASM_EXTABLE(1b, 3b) + : [tos] "+r" (tos), [fault] "+qm" (fault) : [data] "m" (*sp) ); + /* * If RIP looks sensible, or the top of the stack doesn't, print RIP at * the top of the stack trace. */ if ( is_active_kernel_text(regs->rip) || - !is_active_kernel_text(*sp) ) + !is_active_kernel_text(tos) ) printk(" [<%p>] %pS\n", _p(regs->rip), _p(regs->rip)); + else if ( fault ) + { + printk(" [Fault on access]\n"); + return; + } /* * Else RIP looks bad but the top of the stack looks good. Perhaps we * followed a wild function pointer? Lets assume the top of the stack is a @@ -505,7 +519,7 @@ static void show_trace(const struct cpu_ */ else { - printk(" [<%p>] %pS\n", _p(*sp), _p(*sp)); + printk(" [<%p>] %pS\n", _p(tos), _p(tos)); sp++; }