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

From: Nam Cao <namcao@linutronix.de>
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>,
	Jiakai Xu <xujiakai2025@iscas.ac.cn>,
	Matthew Bystrin <dev.mbstr@gmail.com>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Paul Walmsley <pjw@kernel.org>, Rui Qi <qirui.001@bytedance.com>,
	Samuel Holland <samuel.holland@sifive.com>
Subject: Re: [PATCH v4] riscv: stacktrace: fix stack-out-of-bounds in walk_stackframe()
Date: Thu, 02 Jul 2026 10:05:52 +0200	[thread overview]
Message-ID: <87ik6xvmsf.fsf@yellow.woof> (raw)
In-Reply-To: <20260630090535.4103888-1-xujiakai2025@iscas.ac.cn>

Jiakai Xu <xujiakai2025@iscas.ac.cn> writes:
> +	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 (task != current) {
> +		pr_warn("%s: sp (%lx) is not in task stack of %s\n",
> +			__func__, sp, task->comm);
> +		return;
> +	}
> +#ifdef CONFIG_VMAP_STACK
> +	else {
> +		unsigned long ovf_base =
> +			(unsigned long)this_cpu_ptr(overflow_stack);
> +
> +		if (sp >= ovf_base && sp < ovf_base + OVERFLOW_STACK_SIZE)
> +			high = ovf_base + OVERFLOW_STACK_SIZE;
> +	}
> +#endif

Looks functionally correct to me. But this #ifdef goes against the
kernel's coding style. See:
https://docs.kernel.org/process/coding-style.html

Can we
    else if (IS_ENABLED(CONFIG_VMAP_STACK))
just like the irq stack thing?

> +	if (IS_ENABLED(CONFIG_IRQ_STACKS) && !high) {
> +		unsigned long irq_base =
> +			(unsigned long)this_cpu_read(irq_stack_ptr);
> +
> +		if (sp >= irq_base && sp < irq_base + IRQ_STACK_SIZE)
> +			high = irq_base + IRQ_STACK_SIZE;
> +	}