Messages in this thread | | | Date | Mon, 15 Aug 2016 10:05:58 -0500 | From | Josh Poimboeuf <> | Subject | Re: [PATCH v3 09/51] x86/dumpstack: fix x86_32 kernel_stack_pointer() previous stack access |
| |
On Sun, Aug 14, 2016 at 12:26:29AM -0700, Andy Lutomirski wrote: > On Fri, Aug 12, 2016 at 7:28 AM, Josh Poimboeuf <jpoimboe@redhat.com> wrote: > > On x86_32, when an interrupt happens from kernel space, SS and SP aren't > > pushed and the existing stack is used. So pt_regs is effectively two > > words shorter, and the previous stack pointer is normally the memory > > after the shortened pt_regs, aka '®s->sp'. > > > > But in the rare case where the interrupt hits right after the stack > > pointer has been changed to point to an empty stack, like for example > > when call_on_stack() is used, the address immediately after the > > shortened pt_regs is no longer on the stack. In that case, instead of > > '®s->sp', the previous stack pointer should be retrieved from the > > beginning of the current stack page. > > > > kernel_stack_pointer() wants to do that, but it forgets to dereference > > the pointer. So instead of returning a pointer to the previous stack, > > it returns a pointer to the beginning of the current stack. > > > > Fixes: 0788aa6a23cb ("x86: Prepare removal of previous_esp from i386 thread_info structure") > > Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> > > This seems like a valid fix, but I'm not sure I agree with the intent > of the code. ®s->sp really is the previous stack pointer in the > sense that the stack pointer was ®s->sp when the entry happened. > From an unwinder's perspective, how is: > > movl [whatever], $esp > <-- interrupt > > any different from: > > movl [whatever], $esp > pushl [something] > <-- interrupt
In the first case, the stack is empty, so reading the value pointed to by %esp would result in accessing outside the bounds of the stack.
-- Josh
| |