Message ID | 20221107033202.1375238-1-rmclure@linux.ibm.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | [v2,1/4] powerpc/64: Add INTERRUPT_SANITIZE_REGISTERS Kconfig | expand |
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
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