Re: [PATCH v1 2/2] riscv: envcfg save and restore on trap entry/exit

From: Andrew Jones <ajones@ventanamicro.com>
To: Deepak Gupta <debug@rivosinc.com>
Cc: Palmer Dabbelt <palmer@dabbelt.com>,
	 Paul Walmsley <paul.walmsley@sifive.com>,
	aou@eecs.berkeley.edu, apatel@ventanamicro.com,
	 guoren@kernel.org, mchitale@ventanamicro.com,
	waylingii@gmail.com,  greentime.hu@sifive.com,
	samitolvanen@google.com, Bjorn Topel <bjorn@rivosinc.com>,
	 Conor Dooley <conor.dooley@microchip.com>,
	jeeheng.sia@starfivetech.com, Heiko Stuebner <heiko@sntech.de>,
	 Evan Green <evan@rivosinc.com>,
	jszhang@kernel.org, cleger@rivosinc.com,
	 linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v1 2/2] riscv: envcfg save and restore on trap entry/exit
Date: Wed, 13 Dec 2023 13:24:18 +0100	[thread overview]
Message-ID: <20231213-707b4e8b5a91ceedd557eb12@orel> (raw)
In-Reply-To: <ZXkCs9ypok5X/Wx2@debug.ba.rivosinc.com>

On Tue, Dec 12, 2023 at 05:02:43PM -0800, Deepak Gupta wrote:
> On Tue, Dec 12, 2023 at 04:53:48PM -0800, Palmer Dabbelt wrote:
> > On Tue, 12 Dec 2023 15:49:25 PST (-0800), debug@rivosinc.com wrote:
> > > envcfg CSR defines enabling bits for cache management instructions and soon
> > > will control enabling for control flow integrity and pointer masking features.
> > > 
> > > Control flow integrity and pointer masking features need to be enabled on per
> > > thread basis. Additionally, I believe cache management instructions need to be
> > > enabled on per thread basis. As an example a seccomped task on riscv may be
> > > restricted to not use cache management instructions
> > 
> > Do we have anything in the kernel that actually does that?  Generally we
> > need some use, I couldn't find any user-mode writable envcfg bits in any
> > extesions I looked at (admittidly just CFI and pointer masking), and
> > unless I'm missing something there's no per-thread state in the kernel.
> > 
> 
> Cache management operations?
> As of now kernel blindly enables that for all the user mode. It will be good if
> that is enabled on per-thread basis. Sure, all threads can have it enabled by
> default. But if strict seccomp is enabled, I would argue that cache management
> operations for that thread to be disabled as is done on other arches. As an
> example x86 disable rdtsc on strict seccomp. RISCV allows this CMO extension
> and I expect CMO to leverage this (currently it
> doesn't).
> 
> I was being opportunistic here so that I can reduce number of patches on CFI
> enabling patchset.
> 
> Will it be okay if I revise this patch to include with a usecase to restrict CMO
> (say for case of strict seccomp on risc-v)?

I opted to only expose cache block zero since giving userspace the
ability to invalidate cache blocks seems risky from a side-channel attack
perspective.

I'm no security expert, so feedback welcome, but I don't see a risk with
userspace being granted cbo.zero, even for strict seccomp processes.

Thanks,
drew

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