Received-SPF: pass (google.com: domain of linux-kernel+bounces-39178-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Thu, 25 Jan 2024 11:26:47 -0800
From: Deepak Gupta <debug@rivosinc.com>
To: Conor Dooley <conor@kernel.org>
Cc: rick.p.edgecombe@intel.com, broonie@kernel.org, Szabolcs.Nagy@arm.com,
	kito.cheng@sifive.com, keescook@chromium.org,
	ajones@ventanamicro.com, paul.walmsley@sifive.com,
	palmer@dabbelt.com, conor.dooley@microchip.com, cleger@rivosinc.com,
	atishp@atishpatra.org, alex@ghiti.fr, bjorn@rivosinc.com,
	alexghiti@rivosinc.com, corbet@lwn.net, aou@eecs.berkeley.edu,
	oleg@redhat.com, akpm@linux-foundation.org, arnd@arndb.de,
	ebiederm@xmission.com, shuah@kernel.org, brauner@kernel.org,
	guoren@kernel.org, samitolvanen@google.com, evan@rivosinc.com,
	xiao.w.wang@intel.com, apatel@ventanamicro.com,
	mchitale@ventanamicro.com, waylingii@gmail.com,
	greentime.hu@sifive.com, heiko@sntech.de, jszhang@kernel.org,
	shikemeng@huaweicloud.com, david@redhat.com, charlie@rivosinc.com,
	panqinglin2020@iscas.ac.cn, willy@infradead.org,
	vincent.chen@sifive.com, andy.chiu@sifive.com, gerg@kernel.org,
	jeeheng.sia@starfivetech.com, mason.huo@starfivetech.com,
	ancientmodern4@gmail.com, mathis.salmen@matsal.de,
	cuiyunhui@bytedance.com, bhe@redhat.com, chenjiahao16@huawei.com,
	ruscur@russell.cc, bgray@linux.ibm.com, alx@kernel.org,
	baruch@tkos.co.il, zhangqing@loongson.cn, catalin.marinas@arm.com,
	revest@chromium.org, josh@joshtriplett.org, joey.gouly@arm.com,
	shr@devkernel.io, omosnace@redhat.com, ojeda@kernel.org,
	jhubbard@nvidia.com, linux-doc@vger.kernel.org,
	linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org,
	linux-mm@kvack.org, linux-arch@vger.kernel.org,
	linux-kselftest@vger.kernel.org
Subject: Re: [RFC PATCH v1 24/28] riscv: select config for shadow stack and
 landing pad instr support
Message-ID: <ZbK198Vovbw2CwrR@debug.ba.rivosinc.com>
References: <20240125062739.1339782-1-debug@rivosinc.com>
 <20240125062739.1339782-25-debug@rivosinc.com>
 <20240125-snitch-boogieman-5b4a0b142e61@spud>
 <ZbKkgNX7xfU5KO8X@debug.ba.rivosinc.com>
 <20240125-implement-coagulant-3058e743a098@spud>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Disposition: inline
In-Reply-To: <20240125-implement-coagulant-3058e743a098@spud>

On Thu, Jan 25, 2024 at 06:44:48PM +0000, Conor Dooley wrote:
>On Thu, Jan 25, 2024 at 10:12:16AM -0800, Deepak Gupta wrote:
>> On Thu, Jan 25, 2024 at 06:04:26PM +0000, Conor Dooley wrote:
>> > On Wed, Jan 24, 2024 at 10:21:49PM -0800, debug@rivosinc.com wrote:
>> > > From: Deepak Gupta <debug@rivosinc.com>
>> > >
>> > > This patch selects config shadow stack support and landing pad instr
>> > > support. Shadow stack support and landing instr support is hidden behind
>> > > `CONFIG_RISCV_USER_CFI`. Selecting `CONFIG_RISCV_USER_CFI` wires up path
>> > > to enumerate CPU support and if cpu support exists, kernel will support
>> > > cpu assisted user mode cfi.
>> > >
>> > > Signed-off-by: Deepak Gupta <debug@rivosinc.com>
>> > > ---
>> > >  arch/riscv/Kconfig | 15 +++++++++++++++
>> > >  1 file changed, 15 insertions(+)
>> > >
>> > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
>> > > index 9d386e9edc45..437b2f9abf3e 100644
>> > > --- a/arch/riscv/Kconfig
>> > > +++ b/arch/riscv/Kconfig
>> > > @@ -163,6 +163,7 @@ config RISCV
>> > >  	select SYSCTL_EXCEPTION_TRACE
>> > >  	select THREAD_INFO_IN_TASK
>> > >  	select TRACE_IRQFLAGS_SUPPORT
>> > > +	select RISCV_USER_CFI
>> >
>> > This select makes no sense to me, it will unconditionally enable
>> > RISCV_USER_CFI. I don't think that that is your intent, since you have a
>> > detailed option below that allows the user to turn it on or off.
>> >
>> > If you remove it, the commit message will need to change too FYI.
>> >
>>
>> Selecting this config puts support in Kernel so that it can run tasks who wants
>> to enable hardware assisted control flow integrity for themselves. But apps still
>> always need to optin using `prctls`. Those prctls are stubs and return EINVAL when
>> this config is not selected. Not selecting this config means, kernel will not support
>> enabling this feature for user mode.
>
>I don't think you understand me. "select RISCV_USER_CFI" will
>unconditionally build it into the kernel, making stubs etc useless.
>You're talking like (and the rest of your commit implements it!) that
>this feature can be enabled in menuconfig etc. Having this select
>will always enable the config option, rendering the choice below
>redundant. Try turning it off in menuconfig.

Aah got it now. Thanks.
I'll fix this messaging and select in next version.

>
>Oh and if it were valid to have here, you put it in out of order. That's
>an alphanumerically sorted list :)

Thanks for pointing that out. Will fix it.
>
>Cheers,
>Conor.
>