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

[Jiakai Xu]

Hi, Matt!

Thank you for your review and valuable suggestions!

> > -static inline int fp_is_valid(unsigned long fp, unsigned long sp)
> > +static inline int fp_is_valid(unsigned long fp, unsigned long sp,
> > +			      struct task_struct *task)
> >  {
> >  	unsigned long low, high;
> >  
> > +	if (!task)
> > +		task = current;
> > +
> 
> I would suggest to move this `if` into walk_stackframe() function in order to do
> this only once before walking loop.

Moving the `if (!task)` check into `walk_stackframe()` is a reasonable 
micro-optimization. I'll adopt this in v2.

> 
> >  	low = sp + sizeof(struct stackframe);
> > -	high = ALIGN(sp, THREAD_SIZE);
> > +	high = (unsigned long)task_stack_page(task) + THREAD_SIZE;
> 
> Also after grepping `task_stack_page` I've noticed that pt_regs structure is
> located at the end of stack. Maybe it is a good idea to adjust border even
> "lower" to check that sp does not points inside pt_regs? (see task_pt_regs
> macro)

Using `task_pt_regs(task)` as the upper bound provides a tighter and more 
precise boundary. I'll update the high bound accordingly in v2.

I’m planning to submit the v2 patch in two days. I’d like to wait and see 
whether others have any suggestions for this patch.

Thanks again for your time and suggestions!

Best regards,
Jiakai Xu