[v2,1/4] powerpc/64: Add INTERRUPT_SANITIZE_REGISTERS Kconfig

Add Kconfig option for enabling clearing of registers on arrival in an
interrupt handler. This reduces the speculation influence of registers
on kernel internals. The option will be consumed by 64-bit systems that
feature speculation and wish to implement this mitigation.

This patch only introduces the Kconfig option, no actual mitigations.

The primary overhead of this mitigation lies in an increased number of
registers that must be saved and restored by interrupt handlers on
Book3S systems. Enable by default on Book3E systems, which prior to
this patch eagerly save and restore register state, meaning that the
mitigation when implemented will have minimal overhead.

Acked-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
---
Resubmitting patches as their own series after v6 partially merged:
Link: https://lore.kernel.org/all/166488988686.779920.13794870102696416283.b4-ty@ellerman.id.au/t/
---
 arch/powerpc/Kconfig | 9 +++++++++
 1 file changed, 9 insertions(+)

Le 07/11/2022 à 04:31, Rohan McLure a écrit :
> Add Kconfig option for enabling clearing of registers on arrival in an
> interrupt handler. This reduces the speculation influence of registers
> on kernel internals. The option will be consumed by 64-bit systems that
> feature speculation and wish to implement this mitigation.
> 
> This patch only introduces the Kconfig option, no actual mitigations.

If that has to do with speculation, do we need a new Kconfig option ? 
Can't we use CONFIG_PPC_BARRIER_NOSPEC for that ?

> 
> The primary overhead of this mitigation lies in an increased number of
> registers that must be saved and restored by interrupt handlers on
> Book3S systems. Enable by default on Book3E systems, which prior to
> this patch eagerly save and restore register state, meaning that the
> mitigation when implemented will have minimal overhead.
> 
> Acked-by: Nicholas Piggin <npiggin@gmail.com>
> Signed-off-by: Rohan McLure <rmclure@linux.ibm.com>
> ---
> Resubmitting patches as their own series after v6 partially merged:
> Link: https://lore.kernel.org/all/166488988686.779920.13794870102696416283.b4-ty@ellerman.id.au/t/
> ---
>   arch/powerpc/Kconfig | 9 +++++++++
>   1 file changed, 9 insertions(+)
> 
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 2ca5418457ed..9d3d20c6f365 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -529,6 +529,15 @@ config HOTPLUG_CPU
>   
>   	  Say N if you are unsure.
>   
> +config INTERRUPT_SANITIZE_REGISTERS
> +	bool "Clear gprs on interrupt arrival"
> +	depends on PPC64 && ARCH_HAS_SYSCALL_WRAPPER
> +	default PPC_BOOK3E_64
> +	help
> +	  Reduce the influence of user register state on interrupt handlers and
> +	  syscalls through clearing user state from registers before handling
> +	  the exception.
> +
>   config PPC_QUEUED_SPINLOCKS
>   	bool "Queued spinlocks" if EXPERT
>   	depends on SMP

Hi!

On Mon, Nov 07, 2022 at 02:31:59PM +1100, Rohan McLure wrote:
> Add Kconfig option for enabling clearing of registers on arrival in an
> interrupt handler. This reduces the speculation influence of registers
> on kernel internals.

Assuming you are talking about existing PowerPC CPUs from the last 30
years:

There is no data speculation.  At all.  Ever.

There is branch prediction, but that is not influenced by register
contents, either (for any current CPUs at least).  (Except when you get
a flush because of a mispredict, but if this zeroing changes anything,
we will have used wild (but user controlled) values in the old
non-zeroing situation, and that is a much bigger problem itself already,
also for security!  This can be an unlikely kernel bug, or a very
unlikely compiler bug.)

All GPRs are renamed, always.  If you zero all GPRs on interrupt entry
(which is context synchronising, importantly), this will guarantee there
can be no timing influence from the GPRs, because all of the physical
registers depend on nothing that happened before.  So that is good, at
least it can give some peace of mind.  Except that this makes 30 new
registers in just a few cycles, which *itself* can cause stalls, if the
renaming things are still busy.  Context synchronising does not
necessarily help there, the renaming machinery can do stuff *after* an
insn completes.

I don't see how this helps anything.  If it does, "reduces speculation
influence" is not a good description of what it does, afaics?


Segher

On Tue Nov 8, 2022 at 2:39 AM AEST, Segher Boessenkool wrote:
> Hi!
>
> On Mon, Nov 07, 2022 at 02:31:59PM +1100, Rohan McLure wrote:
> > Add Kconfig option for enabling clearing of registers on arrival in an
> > interrupt handler. This reduces the speculation influence of registers
> > on kernel internals.
>
> Assuming you are talking about existing PowerPC CPUs from the last 30
> years:
>
> There is no data speculation.  At all.  Ever.
>
> There is branch prediction, but that is not influenced by register
> contents, either (for any current CPUs at least).  (Except when you get
> a flush because of a mispredict, but if this zeroing changes anything,
> we will have used wild (but user controlled) values in the old
> non-zeroing situation, and that is a much bigger problem itself already,
> also for security!  This can be an unlikely kernel bug, or a very
> unlikely compiler bug.)

This is not about data speculation, it is about speculative execution
in kernel mode that uses architected value that were set by user, and
that value being used to influence a speculative gadget that can expose
data to user via a side channel.

> All GPRs are renamed, always.  If you zero all GPRs on interrupt entry
> (which is context synchronising, importantly), this will guarantee there
> can be no timing influence from the GPRs, because all of the physical
> registers depend on nothing that happened before. So that is good, at
> least it can give some peace of mind.  Except that this makes 30 new
> registers in just a few cycles, which *itself* can cause stalls, if the
> renaming things are still busy.

It will have some pipeline effect like any instruction I suppose. At
least in latest processors, zeroing idiom should release registers
AFAIK.

> Context synchronising does not
> necessarily help there, the renaming machinery can do stuff *after* an
> insn completes.

Possibly context synchronization does not push everything prior to
completion, certainly it does not drain prior stores from store queues
even if they had previously completed, so there can be things going on
there. Software does not really have the ability to do anything about
that though, so that's more of a hardware problem if that exposes a
security issue IMO. Or at least a separate issue if there is some
architecture that could deal with it.

Thanks,
Nick

On Tue Nov 8, 2022 at 12:28 AM AEST, Christophe Leroy wrote:
>
>
> Le 07/11/2022 à 04:31, Rohan McLure a écrit :
> > Add Kconfig option for enabling clearing of registers on arrival in an
> > interrupt handler. This reduces the speculation influence of registers
> > on kernel internals. The option will be consumed by 64-bit systems that
> > feature speculation and wish to implement this mitigation.
> > 
> > This patch only introduces the Kconfig option, no actual mitigations.
>
> If that has to do with speculation, do we need a new Kconfig option ? 
> Can't we use CONFIG_PPC_BARRIER_NOSPEC for that ?

NOSPEC barrier adds runtime-patchable hardware barrier and that config
is a build implementation detail. Also that spec barrier is for bounds
checks speculation that is easy to get the kernel to do something like
speculatively branch to arbitrary address.

Interrupt/syscall register sanitization is more handwavy. It could be
a bandaid for cases where the above speculation barrier was missed
for exampel. But at some point, at least for syscalls, registers have to
contain some values influenced by userspace so if we were paranoid
we would have to put barriers before every branch while any registers
contained a value from userspace.

A security option menu might be a good idea though. There's some other
build time options like rop protection that we might want to add.

Thanks,
Nick