Re: [PATCH] riscv: ptrace: reject CFI regset access when extensions are absent

From: Chen Pei <cp0613@linux.alibaba.com>
To: Guo Ren <guoren@kernel.org>
Cc: Paul Walmsley <pjw@kernel.org>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Albert Ou <aou@eecs.berkeley.edu>,
	Oleg Nesterov <oleg@redhat.com>,
	Deepak Gupta <debug@rivosinc.com>,
	Alexandre Ghiti <alex@ghiti.fr>,
	linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] riscv: ptrace: reject CFI regset access when extensions are absent
Date: Thu, 20 Aug 2026 20:26:59 +0800	[thread overview]
Message-ID: <reply-guo-ren-cfi-ptrace-20260820@linux.alibaba.com> (raw)
In-Reply-To: <CAJF2gTQ-_kSrtV_rpoMK2pKPM3q-CmgDwyX111XvM9oJ=oyDxg@mail.gmail.com>

Hi Guo Ren,

Thanks for the review.

On 2026/8/20 19:45, Guo Ren wrote:
> On Thu, Aug 20, 2026 at 2:32 PM Chen Pei <cp0613@linux.alibaba.com> wrote:
>> +       if (!is_user_lpad_enabled() && !is_user_shstk_enabled())
> Do you want:
>
> /* If shadow stack is not supported or not enabled, nothing to ... */
> if (!is_user_shstk_enabled() || !is_shstk_enabled(tsk))

There are two separate points here, let me clarify both.

First, on "&&" vs "||" between the two features: the combined check is
intentional. Zicfilp and Zicfiss are independent extensions and the
kernel treats them as such everywhere (separate hwprobe keys, separate
prctls, separate riscv_nousercfi switches). Rejecting the regset as
soon as either one is missing would also take away the useful part: on
a system with only shadow stack, shstk_ptr is exactly what a debugger
needs for shadow stack unwinding, and symmetrically for landing pads.
With "&&", the regset stays available when at least one feature is
present, and the missing feature's section reads as all-zero, which is
accurate: its per-task enable flags can only be set when the feature
is available, since the prctl paths are gated by
is_user_lpad_enabled()/is_user_shstk_enabled() individually. The same
applies to riscv_cfi_set(): a "||" would wrongly block legitimate
shstk_ptr updates on shadow-stack-only systems.

Second, on the per-task check (is_shstk_enabled(target) and friends):
I would prefer not to reject the access when CFI is simply not enabled
on the traced task. The all-zero state is a meaningful answer -- it
tells the debugger that CFI is not enabled for this task. This matches
the arm64 GCS regset, which only checks system support in gcs_get()
and returns the task's (possibly zero) state unconditionally.

Best regards,
Pei

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