Re: [PATCH v3] riscv: stacktrace: fix stack-out-of-bounds in walk_stackframe()

[Jiakai Xu]

> Jiakai Xu <xujiakai2025@xxxxxxxxxxx> writes:
> > +#ifdef CONFIG_IRQ_STACKS
> > +DECLARE_PER_CPU(ulong *, irq_stack_ptr);
> > +#endif
> 
> Instead of this, shouldn't we
>   #include <asm/irq_stack.h>
> ?
> 
> > +	if (sp >= (unsigned long)task_stack_page(task) &&
> > +	    sp < (unsigned long)task_stack_page(task) + THREAD_SIZE) {
> > +		high = (unsigned long)task_pt_regs(task);
> > +	} else if (IS_ENABLED(CONFIG_IRQ_STACKS)) {
> > +		high = (unsigned long)this_cpu_read(irq_stack_ptr) +
> > +		       IRQ_STACK_SIZE;
> 
> I suspect this fails to build if CONFIG_IRQ_STACKS=n, which would be
> resolved if we do the suggested #include above.
> 
> > +	} else {
> > +		high = (unsigned long)task_pt_regs(task);
> 
> We only get to this branch if the stack pointer is broken, right? If so,
> I think printing a warning and returning is more appropriate.

Thanks for the review!  All three points make sense. I'll switch to
#include <asm/irq_stack.h>, and change the fallback else branch to
warn and return instead of silently using task_pt_regs().  Will send
a v4 with these fixed.

Jiakai