[Crash-utility] [PATCH] do not check sp if ip points to user space

[Dave Anderson]

----- Original Message -----
> At 09/23/2011 09:41 PM, Dave Anderson Write:
> > 
> > 
> > ----- Original Message -----
> >> If the task is a user program, the sp can be points to anywhere,
> >> because we can modify sp in assembly.
> >> For example:
> >>
> >> .globl main
> >>         .type   main, @function
> >> main:
> >>
> >>         finit
> >>         subq $16, (%rsp)
> >>         movq $0, (%rsp)
> >> .loop:
> >>         jmp .loop
> >>
> >>
> > 
> > Why would any user task do that?
> > 
> > And what happens when a backtrace is attempted on such a task?
> > 
> > Since the current code would not set BT_USER_SPACE, I'm guessing that it
> > would run into this (at least on x86_64):
> > 
> >         if (!(bt->flags & BT_USER_SPACE) && (!rsp || !accessible(rsp))) {
> >                 error(INFO, "cannot determine starting stack pointer\n");
> >                 return;
> >         }
> 
> Yes, crash will run into this on x86_64.

OK, so why not change the above to do something like this:

        if (!(bt->flags & BT_USER_SPACE) && (!rsp || !accessible(rsp))) {
                fprintf(ofp, "cannot determine starting stack pointer\n");
                if(KVMDUMP_DUMPFILE())
                        kvmdump_display_regs(bt->tc->processor, ofp);
                else if (ELF_NOTES_VALID() && DISKDUMP_DUMPFILE())
                        diskdump_display_regs(bt->tc->processor, ofp);
                else if (SADUMP_DUMPFILE()) {
                        sadump_display_regs(bt->tc->processor, ofp);
                return;
        }

Dave
 
> > 
> > I do believe that I put the additional in_user_stack() checks in those
> > locations for a reason.  Consider a task running in kernel mode that
> > corrupts its return address stack location with a non-kernel  address,
> > or called a function indirectly that had a NULL pointer in it.  That
> > would cause a kernel crash with a non-kernel RIP in its exception frame,
> > and your patch would mistake it for user-space.
> 
> I know the reason why you check if sp is in user stack.
> What about this:
> if !is_kernel_text(ip) && (sp is in kernel stack(include irq))
>     try to backtrace according to sp
> else
>     display registers
>
> If both ip and sp is corrupted, and we can not determine sp according to
> the content in the stack, I think we should display registers.
>
> > 
> > In any case, you're going to have to come up with a more compelling
> > reason to change all of these locations.  (And for that matter, I wonder
> > why you didn't patch Fujitsu's get_sadump_regs() the same way?)
> 
> No only for Fujitsu's sadump, I think kvmdump has the same problem.
> 
> By the way, crash try to use the register when the dump format is diskdump.
> I think we should use the register value when the dump format is Fujitsu's
> sadump.
> 
> Thanks
> Wen Congyang
> 
> > 
> > Dave