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

From: "Matthew Bystrin" <dev.mbstr@gmail.com>
To: "Jiakai Xu" <xujiakai2025@iscas.ac.cn>,
	<linux-kernel@vger.kernel.org>, <linux-riscv@lists.infradead.org>
Cc: "Albert Ou" <aou@eecs.berkeley.edu>,
	"Alexandre Ghiti" <alex@ghiti.fr>,
	"Chunyan Zhang" <zhangchunyan@iscas.ac.cn>,
	"Matthew Bystrin" <dev.mbstr@gmail.com>,
	"Palmer Dabbelt" <palmer@dabbelt.com>,
	"Paul Walmsley" <pjw@kernel.org>,
	"Samuel Holland" <samuel.holland@sifive.com>
Subject: Re: [PATCH] riscv: stacktrace: fix stack-out-of-bounds in walk_stackframe
Date: Thu, 14 May 2026 17:21:36 +0300	[thread overview]
Message-ID: <DIIGT2B04NER.LLGB7WGGPC2N@gmail.com> (raw)
In-Reply-To: <20260514100711.838895-1-xujiakai2025@iscas.ac.cn>

Hi, Jiakai!

Thanks for your valid correction!

Jiakai Xu, May 14, 2026 at 13:07:
> -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.

>  	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)

>  	return !(fp < low || fp > high || fp & 0x07);
>  }

-- 
Best regards,
Matt

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv