Re: [PATCH v0] RISC-V: Use Zkr to seed KASLR base address

From: Deepak Gupta <debug@rivosinc.com>
To: Conor Dooley <conor@kernel.org>
Cc: "Clément Léger" <cleger@rivosinc.com>,
	"Conor Dooley" <conor.dooley@microchip.com>,
	"Alexandre Ghiti" <alex@ghiti.fr>,
	"Jesse Taube" <jesse@rivosinc.com>,
	linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org,
	llvm@lists.linux.dev, "Alexandre Ghiti" <alexghiti@rivosinc.com>,
	"Palmer Dabbelt" <palmer@dabbelt.com>,
	"Albert Ou" <aou@eecs.berkeley.edu>,
	"Björn Töpel" <bjorn@rivosinc.com>,
	"Paul Walmsley" <paul.walmsley@sifive.com>,
	"Nathan Chancellor" <nathan@kernel.org>,
	"Nick Desaulniers" <ndesaulniers@google.com>,
	"Masahiro Yamada" <masahiroy@kernel.org>,
	"Atish Patra" <atishp@rivosinc.com>
Subject: Re: [PATCH v0] RISC-V: Use Zkr to seed KASLR base address
Date: Tue, 11 Jun 2024 08:32:10 -0700	[thread overview]
Message-ID: <Zmht+h75GgxP3ocz@debug.ba.rivosinc.com> (raw)
In-Reply-To: <20240610-earplugs-anybody-ebd04a5fa777@spud>

On Mon, Jun 10, 2024 at 10:56:35PM +0100, Conor Dooley wrote:
>On Mon, Jun 10, 2024 at 02:06:50PM -0700, Deepak Gupta wrote:
>> On Mon, Jun 10, 2024 at 11:16:42AM +0200, Clément Léger wrote:
>> > On 10/06/2024 11:02, Conor Dooley wrote:
>> > > On Mon, Jun 10, 2024 at 10:33:34AM +0200, Clément Léger wrote:
>> > > > On 07/06/2024 20:51, Deepak Gupta wrote:
>> > > > > On Mon, Jun 03, 2024 at 01:47:40PM +0100, Conor Dooley wrote:
>> > > > > > On Mon, Jun 03, 2024 at 11:14:49AM +0200, Alexandre Ghiti wrote:
>> > > > > I don't know all the details but on first glance it seems like instead
>> > > > > of ACPI,
>> > > > > may be FWFT is a better place for discovery ?
>> > > > > https://lists.riscv.org/g/tech-prs/topic/patch_v12_add_firmware/106479571
>> > > >
>> > > > IMHO, doing discovery in FWFT is not the goal of this extension. I think
>> > > > the "real" solution would be to wait for the unified discovery task
>> > > > group to come up with something for that (which is their goal I think) [1]
>>
>> Yeah I understand the conundrum here.
>>
>> > >
>> > > I'm curious to see how that works out. The proposal documents an m-mode
>> > > csr, so we'd have to smuggle the information to s-mode somehow...
>> >
>> > Ahem, yeah, I spoke a bit too fast. Looked at the proposal and the
>> > mconfigptr CSR will be accessible by M-mode only so I guess we will have
>> > to find another way...
>>
>> That's not the only problem. Even if you get mconfigptr access, parsing the format
>> is another thing that has to be done. This is early in boot. Although its strictly (pun
>> intended) not a firmware feature extension, I think it's much easier to ask underlying
>> firmware if platform is `Sstrict`. or may be expose something like below
>>
>> `ENABLE_SSTRICT`.
>> Platforms which support `Sstrict` can return success for this while platforms which don't
>> have `Sstrict` can return error code (not supported or not implemented).
>> This way its not feature discovery.
>
>I mean, it's feature discovery in all but name. You're calling it
>enable, but the behaviour you describe is feature discovery - not that I
>am against this being feature discovery since it gets us out of an
>annoying bind.

Yes I know it's cheating but at least for this case, it seems like easy solution which
doesn't break anything. Neither I see it creating any future problems (except FWFT becoming
to look like discovery mechanism :-) and Clement/Atish hating me for that)

Another solution to this could be introducing a riscv config `CONFIG_RISCV_SSTRICT`.
By default always select `CONFIG_RISCV_SSTRICT` and any platform owner who are not
sstrict, they can build their own.
I expect distro (ubuntu, red hat, etc) would want by default `CONFIG_RISCV_SSTRICT`.

>
>I forget which extension Alex and I discussed previously, but there's
>some mm-related things that you're gonna have to probe super early and
>we need to figure out if we can get that info from ACPI or not. I know
>we discussed it w.r.t. one of the T-Head vendor extensions, but I think
>another standard one got mentioned too.
>
>> It seems like arm64 parses fdt and it reads certain CSRs too
>> (`arch/arm64/kernel/pi/kaslr_early.c`). Although it doesn't look like it has to do any
>> discovery for them.
>
>A decree from the Palmer that we don't support things that do not conform
>in this manner would allow us to do what arm64 does. I brought this up
>originally because it's been discussed before that we cannot rely on
>conformance because we want to support people's platforms, whether they
>comply or not. I'd be wary of making this an exception now, and then
>later on someone makes a platform we want to support that doesn't
>conform and hey presto, we regress KASLR support - even if I think it is
>pretty unlikely that someone is going to repurpose the Zkr CSRs.
>
>One of the problems with only supporting conforming platforms is that
>the definition of conforming changes over time! This has happened with
>the Andes PMU for example, which I believe uses an interrupt number that
>was later co-opted by AIA spec. That conformed at the time, but doesn't
>anymore - do they get to mark themselves as Sstrict?
>
>Maybe we can do this on a case-by-case basis, but it's up to Palmer
>whether or not we can do that.

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