Re: [PATCH v4 0/5] Next revision of the L1D flush patches

Re: [PATCH v4 0/5] Next revision of the L1D flush patches

[Kees Cook]

*thread necromancy*
https://lore.kernel.org/lkml/20210108121056.21940-1-sblbir@amazon.com/

On Mon, Jan 25, 2021 at 09:27:38AM +0000, Singh, Balbir wrote:
> On Fri, 2021-01-08 at 23:10 +1100, Balbir Singh wrote:
> > Implement a mechanism that allows tasks to conditionally flush
> > their L1D cache (mitigation mechanism suggested in [2]). The previous
> > posts of these patches were sent for inclusion (see [3]) and were not
> > included due to the concern for the need for additional checks,
> > those checks were:
> > 
> > 1. Implement this mechanism only for CPUs affected by the L1TF bug
> > 2. Disable the software fallback
> > 3. Provide an override to enable this mechanism
> > 4. Be SMT aware in the implementation
> > [...]
> Ping on any review comments? Suggested refactoring?

Hi!

I'd still really like to see this -- it's a big hammer, but that's the
point for cases where some new flaw appears and we can point to the
toolbox and say "you can mitigate it with this while you wait for new
kernel/CPU."

Any further thoughts from x86 maintainers? This seems like it addressed
all of tglx's review comments.

-- 
Kees Cook

Re: [PATCH v4 0/5] Next revision of the L1D flush patches

[Thomas Gleixner]

On Mon, Apr 26 2021 at 10:31, Thomas Gleixner wrote:
> On Thu, Apr 08 2021 at 13:23, Kees Cook wrote:
>>
>> I'd still really like to see this -- it's a big hammer, but that's the
>> point for cases where some new flaw appears and we can point to the
>> toolbox and say "you can mitigate it with this while you wait for new
>> kernel/CPU."
>>
>> Any further thoughts from x86 maintainers? This seems like it addressed
>> all of tglx's review comments.
>
> Sorry for dropping the ball on this. It's in my list of things to deal
> with. Starting to look at it now.

So I went through the pile and for remorse I sat down and made the
tweaks I think are necessary myself.

I've pushed out the result to

  git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git x86/l1dflush

The only thing I did not address yet is that the documentation lacks any
mentioning of the SIGBUS mechanism which is invoked when a task which
asked for L1D flush protection ends up on a SMT sibling for whatever
reason. That's essential to have because it's part of the contract of
that prctl.

Balbir, can you please double check the result and prepare an updated
version from there?

If you don't have cycles, please let me know.

Thanks,

        tglx

Re: [PATCH v4 0/5] Next revision of the L1D flush patches

[Kees Cook]

On Tue, Apr 27, 2021 at 12:24:16AM +0200, Thomas Gleixner wrote:
> On Mon, Apr 26 2021 at 10:31, Thomas Gleixner wrote:
> > On Thu, Apr 08 2021 at 13:23, Kees Cook wrote:
> >>
> >> I'd still really like to see this -- it's a big hammer, but that's the
> >> point for cases where some new flaw appears and we can point to the
> >> toolbox and say "you can mitigate it with this while you wait for new
> >> kernel/CPU."
> >>
> >> Any further thoughts from x86 maintainers? This seems like it addressed
> >> all of tglx's review comments.
> >
> > Sorry for dropping the ball on this. It's in my list of things to deal
> > with. Starting to look at it now.
> 
> So I went through the pile and for remorse I sat down and made the
> tweaks I think are necessary myself.
> 
> I've pushed out the result to
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git x86/l1dflush

Oh excellent; thank you for doing this!

-- 
Kees Cook

Re: [PATCH v4 0/5] Next revision of the L1D flush patches

[Balbir Singh]

On Tue, Apr 27, 2021 at 12:24:16AM +0200, Thomas Gleixner wrote:
> On Mon, Apr 26 2021 at 10:31, Thomas Gleixner wrote:
> > On Thu, Apr 08 2021 at 13:23, Kees Cook wrote:
> >>
> >> I'd still really like to see this -- it's a big hammer, but that's the
> >> point for cases where some new flaw appears and we can point to the
> >> toolbox and say "you can mitigate it with this while you wait for new
> >> kernel/CPU."
> >>
> >> Any further thoughts from x86 maintainers? This seems like it addressed
> >> all of tglx's review comments.
> >
> > Sorry for dropping the ball on this. It's in my list of things to deal
> > with. Starting to look at it now.
> 
> So I went through the pile and for remorse I sat down and made the
> tweaks I think are necessary myself.
> 
> I've pushed out the result to
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git x86/l1dflush
>

Thank you I'll take a look and test it.
 
> The only thing I did not address yet is that the documentation lacks any
> mentioning of the SIGBUS mechanism which is invoked when a task which
> asked for L1D flush protection ends up on a SMT sibling for whatever
> reason. That's essential to have because it's part of the contract of
> that prctl.

IIRC I documented it, I'll double check.

> 
> Balbir, can you please double check the result and prepare an updated
> version from there?
> 
> If you don't have cycles, please let me know.
>

I might have some cycles for testing and re-review. Thanks for all the
hard work on this

Balbir Singh. 

Re: [PATCH v4 0/5] Next revision of the L1D flush patches

[Balbir Singh]

On Wed, Apr 28, 2021 at 01:08:05PM -0700, Kees Cook wrote:
> On Tue, Apr 27, 2021 at 12:24:16AM +0200, Thomas Gleixner wrote:
> > On Mon, Apr 26 2021 at 10:31, Thomas Gleixner wrote:
> > > On Thu, Apr 08 2021 at 13:23, Kees Cook wrote:
> > >>
> > >> I'd still really like to see this -- it's a big hammer, but that's the
> > >> point for cases where some new flaw appears and we can point to the
> > >> toolbox and say "you can mitigate it with this while you wait for new
> > >> kernel/CPU."
> > >>
> > >> Any further thoughts from x86 maintainers? This seems like it addressed
> > >> all of tglx's review comments.
> > >
> > > Sorry for dropping the ball on this. It's in my list of things to deal
> > > with. Starting to look at it now.
> > 
> > So I went through the pile and for remorse I sat down and made the
> > tweaks I think are necessary myself.
> > 
> > I've pushed out the result to
> > 
> >   git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git x86/l1dflush
> 
> Oh excellent; thank you for doing this!
>

Thanks again Thomas!

I no longer have access to the bare metal hardware, but I was able to test
this under qemu with some emulation changes. The changes worked as expected.

Folks on the list/cc, appreciate any tested-by or additional reviewed-by
tags if you do happen to review/test.

Thanks,
Balbir Singh. 

Re: [PATCH v4 0/5] Next revision of the L1D flush patches

[Kees Cook]

On Fri, Jun 04, 2021 at 08:06:14PM +1000, Balbir Singh wrote:
> On Wed, Apr 28, 2021 at 01:08:05PM -0700, Kees Cook wrote:
> > On Tue, Apr 27, 2021 at 12:24:16AM +0200, Thomas Gleixner wrote:
> > > On Mon, Apr 26 2021 at 10:31, Thomas Gleixner wrote:
> > > > On Thu, Apr 08 2021 at 13:23, Kees Cook wrote:
> > > >>
> > > >> I'd still really like to see this -- it's a big hammer, but that's the
> > > >> point for cases where some new flaw appears and we can point to the
> > > >> toolbox and say "you can mitigate it with this while you wait for new
> > > >> kernel/CPU."
> > > >>
> > > >> Any further thoughts from x86 maintainers? This seems like it addressed
> > > >> all of tglx's review comments.
> > > >
> > > > Sorry for dropping the ball on this. It's in my list of things to deal
> > > > with. Starting to look at it now.
> > > 
> > > So I went through the pile and for remorse I sat down and made the
> > > tweaks I think are necessary myself.
> > > 
> > > I've pushed out the result to
> > > 
> > >   git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git x86/l1dflush
> > 
> > Oh excellent; thank you for doing this!
> >
> 
> Thanks again Thomas!
> 
> I no longer have access to the bare metal hardware, but I was able to test
> this under qemu with some emulation changes. The changes worked as expected.
> 
> Folks on the list/cc, appreciate any tested-by or additional reviewed-by
> tags if you do happen to review/test.

I can't test the behavior (no access to CPU), but I wrote a simple prctl
tester. Perhaps this can be expanded on?


diff --git a/tools/testing/selftests/x86/Makefile b/tools/testing/selftests/x86/Makefile
index 333980375bc7..50c150d35962 100644
--- a/tools/testing/selftests/x86/Makefile
+++ b/tools/testing/selftests/x86/Makefile
@@ -13,7 +13,7 @@ CAN_BUILD_WITH_NOPIE := $(shell ./check_cc.sh $(CC) trivial_program.c -no-pie)
 TARGETS_C_BOTHBITS := single_step_syscall sysret_ss_attrs syscall_nt test_mremap_vdso \
 			check_initial_reg_state sigreturn iopl ioperm \
 			test_vsyscall mov_ss_trap \
-			syscall_arg_fault fsgsbase_restore
+			syscall_arg_fault fsgsbase_restore l1d_flush
 TARGETS_C_32BIT_ONLY := entry_from_vm86 test_syscall_vdso unwind_vdso \
 			test_FCMOV test_FCOMI test_FISTTP \
 			vdso_restorer
diff --git a/tools/testing/selftests/x86/l1d_flush.c b/tools/testing/selftests/x86/l1d_flush.c
new file mode 100644
index 000000000000..ef4e73679d58
--- /dev/null
+++ b/tools/testing/selftests/x86/l1d_flush.c
@@ -0,0 +1,66 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * l1d_flush.c: Exercise the L1D flushing behaviors
+ */
+#define _GNU_SOURCE
+
+#include <stdlib.h>
+#include <stddef.h>
+#include <stdio.h>
+#include <string.h>
+#include <errno.h>
+#include <sys/prctl.h>
+
+#ifndef PR_SET_SPECULATION_CTRL
+# define PR_GET_SPECULATION_CTRL	52
+# define PR_SET_SPECULATION_CTRL	53
+#  define PR_SPEC_ENABLE		(1UL << 1)
+#  define PR_SPEC_DISABLE		(1UL << 2)
+#endif
+
+#ifndef PR_SPEC_L1D_FLUSH
+# define PR_SPEC_L1D_FLUSH		2
+#endif
+
+#include "../kselftest_harness.h"
+
+TEST(toggle)
+{
+	int ret;
+
+	ret = prctl(PR_GET_SPECULATION_CTRL, PR_SPEC_L1D_FLUSH, 0, 0, 0);
+	ASSERT_GE(ret, 0) {
+		TH_LOG("PR_GET_SPECULATION_CTRL, PR_SPEC_L1D_FLUSH failed: %d (%s)", errno, strerror(errno));
+	}
+	EXPECT_EQ((ret & (PR_SPEC_DISABLE | PR_SPEC_ENABLE)), 0) {
+		TH_LOG("PR_SPEC_L1D_FLUSH is already enabled!?");
+	}
+
+	/* Enable */
+	ret = prctl(PR_SET_SPECULATION_CTRL, PR_SPEC_L1D_FLUSH, PR_SPEC_ENABLE, 0, 0);
+	EXPECT_EQ(ret, 0) {
+		if (errno == EPERM)
+			SKIP(return, "Kernel does not support PR_SPEC_L1D_FLUSH (boot with l1d_flush=on with a supported CPU)");
+		TH_LOG("PR_SET_SPECULATION_CTRL, PR_SPEC_L1D_FLUSH, PR_SPEC_ENABLE failed: %d (%s)", errno, strerror(errno));
+	}
+
+	/* Check Enable */
+	ret = prctl(PR_GET_SPECULATION_CTRL, PR_SPEC_L1D_FLUSH, 0, 0, 0);
+	EXPECT_EQ((ret & (PR_SPEC_DISABLE | PR_SPEC_ENABLE)), PR_SPEC_ENABLE) {
+		TH_LOG("PR_SPEC_L1D_FLUSH did not stay enabled");
+	}
+
+	/* Disable */
+	ret = prctl(PR_SET_SPECULATION_CTRL, PR_SPEC_L1D_FLUSH, PR_SPEC_DISABLE, 0, 0);
+	EXPECT_EQ(ret, 0) {
+		TH_LOG("PR_SET_SPECULATION_CTRL, PR_SPEC_L1D_FLUSH, PR_SPEC_DISABLE failed: %d (%s)", errno, strerror(errno));
+	}
+
+	/* Check Disable */
+	ret = prctl(PR_GET_SPECULATION_CTRL, PR_SPEC_L1D_FLUSH, 0, 0, 0);
+	EXPECT_EQ((ret & (PR_SPEC_DISABLE | PR_SPEC_ENABLE)), PR_SPEC_DISABLE) {
+		TH_LOG("PR_SPEC_L1D_FLUSH did not stay disabled");
+	}
+}
+
+TEST_HARNESS_MAIN

-- 
Kees Cook