RE:(3) [PATCH 1/1] arm: get task_stack reference before dump_backtrace

[Maninder Singh]

Hi,

> Hi,
> 
> > >"otherwise if someone calls show_stack() for task" ... and the stack
> > >trace given stops at show_stack() and doesn't show the "someone".
> > >
> > >I'd like to know _how_ this happens, and why ARM64 and now 32-bit ARM
> > >are different from x86.
> > 
> > I tried to simulate same thing on x86_64, it is also crashing.
> > 
> > Just a dummy code to save task_struct to reproduce the race:
> > 
> > +       rcu_read_lock();
> > +       for_each_process(p) {
> > +               if (!strcmp(p->comm, "sleep")) {
> > +                       check_task = p;
> > +                       get_task_struct(p);
> > +                       pr_emerg("get done for %s %d\n", p->comm, p->pid);
> > +               }
> > +       }
> > +       rcu_read_unlock();
> > 
> > // in mean time here sleep binary will be exited.
> > 
> > +       show_stack(check_task,  NULL, KERN_EMERG);
> 
> The task's stack is released on its final schedule() invocation.
> Therefore holding task_struct does not hold the stack of the task if it
> is separated out of task_struct and can be gone if the tasks quits.
> 
> Therefore holding a reference to the stack while accessing it, like
> during a backtrace, makes sense and is required if the task is not
> current.
> 
> Let me add this to my list and tackle it later today for x86. Then we
> get probably Russell on board for ARM.
> 

Thanks :)

I have checked more details about missing this handling from x86 and found the reason.
Originally show_stack() was calling show_stack_log_lvl()
which had proper call to try_get_task_stack().

It was replaced with show_trace_log_lvl() later. Which removed this check.

Below commit did that.

commit 0ee1dd9f5e7eae4e55f95935b72d4beecb03de9c (HEAD)
Author: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>
Date:   Tue Oct 25 09:51:13 2016 -0500

    x86/dumpstack: Remove raw stack dump
...
...


-void show_stack_log_lvl(struct task_struct *task, struct pt_regs *regs,
-                       unsigned long *sp, char *log_lvl)
-{
...
...
-       if (!try_get_task_stack(task))
-               return;
-
...

@@ -171,12 +170,12 @@ void show_stack(struct task_struct *task, unsigned long *sp)
        if (!sp && task == current)
                sp = get_stack_pointer(current, NULL);

-       show_stack_log_lvl(task, NULL, sp, KERN_DEFAULT);
+       show_trace_log_lvl(task, NULL, sp, KERN_DEFAULT);
 }



So I think x86 patch shall also be merged to kernel:
https://lkml.org/lkml/2026/3/11/317


Thanks and regrads
Maninder singh