Messages in this thread | | | From | Andy Lutomirski <> | Date | Mon, 20 Nov 2017 13:07:16 -0800 | Subject | Re: [PATCH 02/16] x86/dumpstack: Add get_stack_info() support for the SYSENTER stack |
| |
On Mon, Nov 20, 2017 at 1:00 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: > On Mon, Nov 20, 2017 at 12:46:13PM -0800, Andy Lutomirski wrote: >> On Mon, Nov 20, 2017 at 12:42 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: >> > On Mon, Nov 20, 2017 at 09:07:33AM -0800, Andy Lutomirski wrote: >> >> +bool in_SYSENTER_stack(unsigned long *stack, struct stack_info *info) >> > >> > Can you make it lowercase for consistency with the other in_*_stack() >> > functions? For example, in_irq_stack() is all lowercase even though >> > "IRQ" is normally written in uppercase. >> > >> > But also, I'm wondering whether this get_stack_info() support is even >> > really needed. >> > >> > As currently written, the trampoline code doesn't have any ORC data >> > associated with it. So the unwinder would never have the need to >> > actually read the SYSENTER stack. >> > >> > You _could_ add an UNWIND_HINT_IRET_REGS annotation after the simulated >> > iret frame is written, which would allow the unwinder to dump those regs >> > when unwinding from an NMI. >> >> There's some ORC data in the non-trampoline SYSENTER path > > But that's *after* the stack switch to the real kernel stack, right?
Hmm, maybe you're right.
> >> but, more importantly, the OOPS unwinder will just bail without this >> patch. With the patch, we get a valid unwind, except that everything >> has a ? in front. > > Hm. I can't even fathom how that's possible. Are you talking about the > "unwind from NMI to SYSENTER stack" path? Or any unwind to a syscall? > Either way I'm baffled... If the unwinder only encounters the SYSENTER > stack at the end, how could that cause everything beforehand to have a > question mark?
I mean that, if I put a ud2 or other bug in the code that runs on the SYSENTER stack, without this patch, I get a totally blank call trace.
> >> > But there's only a tiny window where that would be possible: only a few >> > instructions. I'm not sure that would be worth the effort, unless we >> > got to the point where we expect to have 100% unwinder coverage. But >> > that's currently unrealistic anyway because of generated code and >> > runtime patching. >> >> I tripped it myself several times when debugging this code. > > Again I don't see how this patch would help if there's no ORC data for > the code which uses the SYSENTER stack. I must be missing something.
| |