Re: [PATCH 1/2] arm64: entry: fix ARM64_WORKAROUND_SPECULATIVE_UNPRIV_LOAD

From: Mark Rutland <mark.rutland@arm.com>
To: Rob Herring <robh@kernel.org>
Cc: linux-arm-kernel@lists.infradead.org, catalin.marinas@arm.com,
	james.morse@arm.com, stable@vger.kernel.org, will@kernel.org
Subject: Re: [PATCH 1/2] arm64: entry: fix ARM64_WORKAROUND_SPECULATIVE_UNPRIV_LOAD
Date: Mon, 22 Jan 2024 11:16:10 +0000	[thread overview]
Message-ID: <Za5OekA_bYooPXxz@FVFF77S0Q05N> (raw)
In-Reply-To: <CAL_JsqJGWHj_adgvX_XwRuh+xvQGw2p-e9ZxxJ_qd19nWZ_daQ@mail.gmail.com>

Hi Rob,

Sorry for the confusion here; I should have synced up with you before sending
this out.

On Fri, Jan 19, 2024 at 09:11:33AM -0600, Rob Herring wrote:
> On Tue, Jan 16, 2024 at 5:02 AM Mark Rutland <mark.rutland@arm.com> wrote:
> >
> > Currently the ARM64_WORKAROUND_SPECULATIVE_UNPRIV_LOAD workaround isn't
> > quite right, as it is supposed to be applied after the last explicit
> > memory access, but is immediately followed by an LDR.
> 
> This isn't necessary. The LDR in question is an unprivileged load from
> the EL0 stack. The erratum write-up is not really clear in that
> regard.

I see from internal notes that the rationale is that the LDR in question only
loads data that EL0 already has in its registers, and hence it doesn't matter
if that data is leaked to EL0. That's reasonable, but we didn't note that
anywhere (i.e. neither in the commit message nor in any comments).

To avoid confusion, the LDR in question *is* a privileged load (whereas an LDTR
at EL1 would be an unprivileged load); for memory accesses the architecture
uses the terms privileged and unprivileged to distinguish the way those are
handled by the MMU.

I agree that given the rationale above this patch isn't strictly necessary, but
I would prefer result of these two patches as it's less likely that we'll add
loads of sensitive information in future as this code is changed.

> It's the same as the KPTI case. After switching the page tables, there
> are unprivileged loads from the EL0 stack.

I'm not sure what you mean here; maybe I'm missing something?

AFAICT we don't do any loads within the kernel after switching the translation
tables. In tramp_exit we follow tramp_unmap_kernel with:

	MRS; ERET; SB

... and in __sdei_asm_exit_trampoline we follow tramp_unmap_kernel with

	CMP; B.NE; {HVC,SMC}; B .

... so there are no explicit loads at EL1 before the ERET to EL0. In the SDEI
case any loads at a higher EL don't matter because they're in a different
translation regime.

Thanks,
Mark.

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