2018-03-02 22:03:53

by Tim Chen

[permalink] [raw]
Subject: [PATCH 0/2] Backport IBPB on context switch to non-dumpable process

Greg,

I will like to propose backporting "x86/speculation: Use Indirect Branch
Prediction Barrier on context switch" from commit 18bf3c3e in upstream
to 4.9 and 4.4 stable. The patch has already been ported to 4.14 and
4.15 stable. The patch needs mm context id that Andy added in commit
f39681ed. I have lifted the mm context id change from Andy's upstream
patch and included it here.

Thanks.

Tim

Tim Chen (2):
x86/mm: Give each mm a unique ID
x86/speculation: Use Indirect Branch Prediction Barrier in context
switch

arch/x86/include/asm/mmu.h | 15 +++++++++++++--
arch/x86/include/asm/mmu_context.h | 5 +++++
arch/x86/include/asm/tlbflush.h | 2 ++
arch/x86/mm/tlb.c | 33 +++++++++++++++++++++++++++++++++
4 files changed, 53 insertions(+), 2 deletions(-)

--
2.9.4



2018-03-02 22:04:12

by Tim Chen

[permalink] [raw]
Subject: [PATCH 1/2] x86/mm: Give each mm a unique ID

From: Andy Lutomirski <[email protected]>
commit: f39681ed0f48498b80455095376f11535feea332

This adds a new variable to mmu_context_t: ctx_id.
ctx_id uniquely identifies the mm_struct and will never be reused.

Signed-off-by: Andy Lutomirski <[email protected]>
Reviewed-by: Nadav Amit <[email protected]>
Reviewed-by: Thomas Gleixner <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: Arjan van de Ven <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Dave Hansen <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Mel Gorman <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Rik van Riel <[email protected]>
Cc: [email protected]
Link: http://lkml.kernel.org/r/413a91c24dab3ed0caa5f4e4d017d87b0857f920.1498751203.git.luto@kernel.org
Signed-off-by: Ingo Molnar <[email protected]>
Signed-off-by: Tim Chen <[email protected]>
---
arch/x86/include/asm/mmu.h | 15 +++++++++++++--
arch/x86/include/asm/mmu_context.h | 5 +++++
arch/x86/mm/tlb.c | 2 ++
3 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/mmu.h b/arch/x86/include/asm/mmu.h
index 8b272a0..e2e0934 100644
--- a/arch/x86/include/asm/mmu.h
+++ b/arch/x86/include/asm/mmu.h
@@ -3,12 +3,18 @@

#include <linux/spinlock.h>
#include <linux/mutex.h>
+#include <linux/atomic.h>

/*
- * The x86 doesn't have a mmu context, but
- * we put the segment information here.
+ * x86 has arch-specific MMU state beyond what lives in mm_struct.
*/
typedef struct {
+ /*
+ * ctx_id uniquely identifies this mm_struct. A ctx_id will never
+ * be reused, and zero is not a valid ctx_id.
+ */
+ u64 ctx_id;
+
#ifdef CONFIG_MODIFY_LDT_SYSCALL
struct ldt_struct *ldt;
#endif
@@ -33,6 +39,11 @@ typedef struct {
#endif
} mm_context_t;

+#define INIT_MM_CONTEXT(mm) \
+ .context = { \
+ .ctx_id = 1, \
+ }
+
void leave_mm(int cpu);

#endif /* _ASM_X86_MMU_H */
diff --git a/arch/x86/include/asm/mmu_context.h b/arch/x86/include/asm/mmu_context.h
index d23e355..5a295bb 100644
--- a/arch/x86/include/asm/mmu_context.h
+++ b/arch/x86/include/asm/mmu_context.h
@@ -12,6 +12,9 @@
#include <asm/tlbflush.h>
#include <asm/paravirt.h>
#include <asm/mpx.h>
+
+extern atomic64_t last_mm_ctx_id;
+
#ifndef CONFIG_PARAVIRT
static inline void paravirt_activate_mm(struct mm_struct *prev,
struct mm_struct *next)
@@ -106,6 +109,8 @@ static inline void enter_lazy_tlb(struct mm_struct *mm, struct task_struct *tsk)
static inline int init_new_context(struct task_struct *tsk,
struct mm_struct *mm)
{
+ mm->context.ctx_id = atomic64_inc_return(&last_mm_ctx_id);
+
#ifdef CONFIG_X86_INTEL_MEMORY_PROTECTION_KEYS
if (cpu_feature_enabled(X86_FEATURE_OSPKE)) {
/* pkey 0 is the default and always allocated */
diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
index 578973a..fa74bf5 100644
--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -29,6 +29,8 @@
* Implement flush IPI by CALL_FUNCTION_VECTOR, Alex Shi
*/

+atomic64_t last_mm_ctx_id = ATOMIC64_INIT(1);
+
struct flush_tlb_info {
struct mm_struct *flush_mm;
unsigned long flush_start;
--
2.9.4


2018-03-03 03:34:07

by Tim Chen

[permalink] [raw]
Subject: [PATCH 2/2] x86/speculation: Use Indirect Branch Prediction Barrier in context switch

commit: 18bf3c3ea8ece8f03b6fc58508f2dfd23c7711c7

Flush indirect branches when switching into a process that marked itself
non dumpable. This protects high value processes like gpg better,
without having too high performance overhead.

If done naïvely, we could switch to a kernel idle thread and then back
to the original process, such as:

process A -> idle -> process A

In such scenario, we do not have to do IBPB here even though the process
is non-dumpable, as we are switching back to the same process after a
hiatus.

To avoid the redundant IBPB, which is expensive, we track the last mm
user context ID. The cost is to have an extra u64 mm context id to track
the last mm we were using before switching to the init_mm used by idle.
Avoiding the extra IBPB is probably worth the extra memory for this
common scenario.

For those cases where tlb_defer_switch_to_init_mm() returns true (non
PCID), lazy tlb will defer switch to init_mm, so we will not be changing
the mm for the process A -> idle -> process A switch. So IBPB will be
skipped for this case.

Thanks to the reviewers and Andy Lutomirski for the suggestion of
using ctx_id which got rid of the problem of mm pointer recycling.

Signed-off-by: Tim Chen <[email protected]>
Signed-off-by: David Woodhouse <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Link: https://lkml.kernel.org/r/[email protected]
---
arch/x86/include/asm/tlbflush.h | 2 ++
arch/x86/mm/tlb.c | 31 +++++++++++++++++++++++++++++++
2 files changed, 33 insertions(+)

diff --git a/arch/x86/include/asm/tlbflush.h b/arch/x86/include/asm/tlbflush.h
index 94146f6..99185a0 100644
--- a/arch/x86/include/asm/tlbflush.h
+++ b/arch/x86/include/asm/tlbflush.h
@@ -68,6 +68,8 @@ static inline void invpcid_flush_all_nonglobals(void)
struct tlb_state {
struct mm_struct *active_mm;
int state;
+ /* last user mm's ctx id */
+ u64 last_ctx_id;

/*
* Access to this CR4 shadow and to H/W CR4 is protected by
diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
index fa74bf5..eac92e2 100644
--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -10,6 +10,7 @@

#include <asm/tlbflush.h>
#include <asm/mmu_context.h>
+#include <asm/nospec-branch.h>
#include <asm/cache.h>
#include <asm/apic.h>
#include <asm/uv/uv.h>
@@ -106,6 +107,28 @@ void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next,
unsigned cpu = smp_processor_id();

if (likely(prev != next)) {
+ u64 last_ctx_id = this_cpu_read(cpu_tlbstate.last_ctx_id);
+
+ /*
+ * Avoid user/user BTB poisoning by flushing the branch
+ * predictor when switching between processes. This stops
+ * one process from doing Spectre-v2 attacks on another.
+ *
+ * As an optimization, flush indirect branches only when
+ * switching into processes that disable dumping. This
+ * protects high value processes like gpg, without having
+ * too high performance overhead. IBPB is *expensive*!
+ *
+ * This will not flush branches when switching into kernel
+ * threads. It will also not flush if we switch to idle
+ * thread and back to the same process. It will flush if we
+ * switch to a different non-dumpable process.
+ */
+ if (tsk && tsk->mm &&
+ tsk->mm->context.ctx_id != last_ctx_id &&
+ get_dumpable(tsk->mm) != SUID_DUMP_USER)
+ indirect_branch_prediction_barrier();
+
if (IS_ENABLED(CONFIG_VMAP_STACK)) {
/*
* If our current stack is in vmalloc space and isn't
@@ -120,6 +143,14 @@ void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next,
set_pgd(pgd, init_mm.pgd[stack_pgd_index]);
}

+ /*
+ * Record last user mm's context id, so we can avoid
+ * flushing branch buffer with IBPB if we switch back
+ * to the same user.
+ */
+ if (next != &init_mm)
+ this_cpu_write(cpu_tlbstate.last_ctx_id, next->context.ctx_id);
+
this_cpu_write(cpu_tlbstate.state, TLBSTATE_OK);
this_cpu_write(cpu_tlbstate.active_mm, next);

--
2.9.4


2018-03-03 08:58:00

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 0/2] Backport IBPB on context switch to non-dumpable process

On Fri, Mar 02, 2018 at 01:32:08PM -0800, Tim Chen wrote:
> Greg,
>
> I will like to propose backporting "x86/speculation: Use Indirect Branch
> Prediction Barrier on context switch" from commit 18bf3c3e in upstream
> to 4.9 and 4.4 stable. The patch has already been ported to 4.14 and
> 4.15 stable. The patch needs mm context id that Andy added in commit
> f39681ed. I have lifted the mm context id change from Andy's upstream
> patch and included it here.

What does this patch "fix" in those older kernels? Is this a
performance improvement or something else?

thanks,

greg k-h

2018-03-03 11:42:39

by Woodhouse, David

[permalink] [raw]
Subject: Re: [PATCH 0/2] Backport IBPB on context switch to non-dumpable process

On Sat, 2018-03-03 at 09:54 +0100, Greg Kroah-Hartman wrote:
> On Fri, Mar 02, 2018 at 01:32:08PM -0800, Tim Chen wrote:
> >
> > Greg,
> >
> > I will like to propose backporting "x86/speculation: Use Indirect Branch
> > Prediction Barrier on context switch" from commit 18bf3c3e in upstream
> > to 4.9 and 4.4 stable.  The patch has already been ported to 4.14 and
> > 4.15 stable.  The patch needs mm context id that Andy added in commit
> > f39681ed. I have lifted the mm context id change from Andy's upstream
> > patch and included it here.
>
> What does this patch "fix" in those older kernels?  Is this a
> performance improvement or something else?

It's part of the Spectre variant 2 mitigation — a full flush of the
branch prediction on context switch to a sensitive¹ process. It was the
one I called out as "needs more attention" when I did the rest of the
retpoline etc backportingk, and Tim has now fixed it up. (Thanks).




¹ for now, "sensitive" means non-dumpable. This isn't perfect but it's
a reasonable approximation for now; it would be too expensive to do it
on *every* context switch. And for your purposes, the important part is
that it's what's upstream.


Attachments:
smime.p7s (5.09 kB)

2018-03-07 17:32:10

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 1/2] x86/mm: Give each mm a unique ID

On Fri, Mar 02, 2018 at 01:32:09PM -0800, Tim Chen wrote:
> From: Andy Lutomirski <[email protected]>
> commit: f39681ed0f48498b80455095376f11535feea332
>
> This adds a new variable to mmu_context_t: ctx_id.
> ctx_id uniquely identifies the mm_struct and will never be reused.
>
> Signed-off-by: Andy Lutomirski <[email protected]>
> Reviewed-by: Nadav Amit <[email protected]>
> Reviewed-by: Thomas Gleixner <[email protected]>
> Cc: Andrew Morton <[email protected]>
> Cc: Arjan van de Ven <[email protected]>
> Cc: Borislav Petkov <[email protected]>
> Cc: Dave Hansen <[email protected]>
> Cc: Linus Torvalds <[email protected]>
> Cc: Mel Gorman <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Rik van Riel <[email protected]>
> Cc: [email protected]
> Link: http://lkml.kernel.org/r/413a91c24dab3ed0caa5f4e4d017d87b0857f920.1498751203.git.luto@kernel.org
> Signed-off-by: Ingo Molnar <[email protected]>
> Signed-off-by: Tim Chen <[email protected]>
> ---
> arch/x86/include/asm/mmu.h | 15 +++++++++++++--
> arch/x86/include/asm/mmu_context.h | 5 +++++
> arch/x86/mm/tlb.c | 2 ++
> 3 files changed, 20 insertions(+), 2 deletions(-)
>

Does not apply to 4.4.y :(

Can you provide a working backport for that tree?

thanks,

greg k-h

2018-03-07 17:33:26

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 0/2] Backport IBPB on context switch to non-dumpable process

On Fri, Mar 02, 2018 at 01:32:08PM -0800, Tim Chen wrote:
> Greg,
>
> I will like to propose backporting "x86/speculation: Use Indirect Branch
> Prediction Barrier on context switch" from commit 18bf3c3e in upstream
> to 4.9 and 4.4 stable. The patch has already been ported to 4.14 and
> 4.15 stable. The patch needs mm context id that Andy added in commit
> f39681ed. I have lifted the mm context id change from Andy's upstream
> patch and included it here.

Applied to 4.9.y, but I need a separate set of patches for 4.4.y in
order for them to work there. Can you provide that?

thanks,

greg k-h

2018-03-08 18:24:54

by Tim Chen

[permalink] [raw]
Subject: Re: [PATCH 1/2] x86/mm: Give each mm a unique ID

On 03/07/2018 09:30 AM, Greg Kroah-Hartman wrote:
> On Fri, Mar 02, 2018 at 01:32:09PM -0800, Tim Chen wrote:
>> From: Andy Lutomirski <[email protected]>
>> commit: f39681ed0f48498b80455095376f11535feea332
>>
>> This adds a new variable to mmu_context_t: ctx_id.
>> ctx_id uniquely identifies the mm_struct and will never be reused.
>>
>> Signed-off-by: Andy Lutomirski <[email protected]>
>> Reviewed-by: Nadav Amit <[email protected]>
>> Reviewed-by: Thomas Gleixner <[email protected]>
>> Cc: Andrew Morton <[email protected]>
>> Cc: Arjan van de Ven <[email protected]>
>> Cc: Borislav Petkov <[email protected]>
>> Cc: Dave Hansen <[email protected]>
>> Cc: Linus Torvalds <[email protected]>
>> Cc: Mel Gorman <[email protected]>
>> Cc: Peter Zijlstra <[email protected]>
>> Cc: Rik van Riel <[email protected]>
>> Cc: [email protected]
>> Link: http://lkml.kernel.org/r/413a91c24dab3ed0caa5f4e4d017d87b0857f920.1498751203.git.luto@kernel.org
>> Signed-off-by: Ingo Molnar <[email protected]>
>> Signed-off-by: Tim Chen <[email protected]>
>> ---
>> arch/x86/include/asm/mmu.h | 15 +++++++++++++--
>> arch/x86/include/asm/mmu_context.h | 5 +++++
>> arch/x86/mm/tlb.c | 2 ++
>> 3 files changed, 20 insertions(+), 2 deletions(-)
>>
>
> Does not apply to 4.4.y :(
>
> Can you provide a working backport for that tree?
>

Okay. Will do. Thanks.

Tim

2018-03-10 01:05:52

by Tim Chen

[permalink] [raw]
Subject: Re: [PATCH 1/2] x86/mm: Give each mm a unique ID

On 03/08/2018 10:23 AM, Tim Chen wrote:
> On 03/07/2018 09:30 AM, Greg Kroah-Hartman wrote:
>> On Fri, Mar 02, 2018 at 01:32:09PM -0800, Tim Chen wrote:
>>> From: Andy Lutomirski <[email protected]>
>>> commit: f39681ed0f48498b80455095376f11535feea332
>>>
>>> This adds a new variable to mmu_context_t: ctx_id.
>>> ctx_id uniquely identifies the mm_struct and will never be reused.
>>>
>>> Signed-off-by: Andy Lutomirski <[email protected]>
>>> Reviewed-by: Nadav Amit <[email protected]>
>>> Reviewed-by: Thomas Gleixner <[email protected]>
>>> Cc: Andrew Morton <[email protected]>
>>> Cc: Arjan van de Ven <[email protected]>
>>> Cc: Borislav Petkov <[email protected]>
>>> Cc: Dave Hansen <[email protected]>
>>> Cc: Linus Torvalds <[email protected]>
>>> Cc: Mel Gorman <[email protected]>
>>> Cc: Peter Zijlstra <[email protected]>
>>> Cc: Rik van Riel <[email protected]>
>>> Cc: [email protected]
>>> Link: http://lkml.kernel.org/r/413a91c24dab3ed0caa5f4e4d017d87b0857f920.1498751203.git.luto@kernel.org
>>> Signed-off-by: Ingo Molnar <[email protected]>
>>> Signed-off-by: Tim Chen <[email protected]>
>>> ---
>>> arch/x86/include/asm/mmu.h | 15 +++++++++++++--
>>> arch/x86/include/asm/mmu_context.h | 5 +++++
>>> arch/x86/mm/tlb.c | 2 ++
>>> 3 files changed, 20 insertions(+), 2 deletions(-)
>>>
>>
>> Does not apply to 4.4.y :(
>>
>> Can you provide a working backport for that tree?
>>
>
> Okay. Will do. Thanks.
>


Greg,

I actually found that there are a number of dependent IBPB related patches that haven't been
backported yet to 4.4:

x86/cpufeatures: Add AMD feature bits for Speculation Control
(cherry picked from commit 5d10cbc91d9eb5537998b65608441b592eec65e7)

x86/msr: Add definitions for new speculation control MSRs
(cherry picked from commit 1e340c60d0dd3ae07b5bedc16a0469c14b9f3410)

x86/speculation: Add basic IBPB (Indirect Branch Prediction Barrier) support
(cherry picked from commit 20ffa1caecca4db8f79fe665acdeaa5af815a24d)

x86/cpufeatures: Clean up Spectre v2 related CPUID flags
(cherry picked from commit 2961298efe1ea1b6fc0d7ee8b76018fa6c0bcef2)

And probably and a few more.
You have plans to backport these patches?

Tim


2018-03-10 01:15:56

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 1/2] x86/mm: Give each mm a unique ID

On Fri, Mar 09, 2018 at 05:04:39PM -0800, Tim Chen wrote:
> On 03/08/2018 10:23 AM, Tim Chen wrote:
> > On 03/07/2018 09:30 AM, Greg Kroah-Hartman wrote:
> >> On Fri, Mar 02, 2018 at 01:32:09PM -0800, Tim Chen wrote:
> >>> From: Andy Lutomirski <[email protected]>
> >>> commit: f39681ed0f48498b80455095376f11535feea332
> >>>
> >>> This adds a new variable to mmu_context_t: ctx_id.
> >>> ctx_id uniquely identifies the mm_struct and will never be reused.
> >>>
> >>> Signed-off-by: Andy Lutomirski <[email protected]>
> >>> Reviewed-by: Nadav Amit <[email protected]>
> >>> Reviewed-by: Thomas Gleixner <[email protected]>
> >>> Cc: Andrew Morton <[email protected]>
> >>> Cc: Arjan van de Ven <[email protected]>
> >>> Cc: Borislav Petkov <[email protected]>
> >>> Cc: Dave Hansen <[email protected]>
> >>> Cc: Linus Torvalds <[email protected]>
> >>> Cc: Mel Gorman <[email protected]>
> >>> Cc: Peter Zijlstra <[email protected]>
> >>> Cc: Rik van Riel <[email protected]>
> >>> Cc: [email protected]
> >>> Link: http://lkml.kernel.org/r/413a91c24dab3ed0caa5f4e4d017d87b0857f920.1498751203.git.luto@kernel.org
> >>> Signed-off-by: Ingo Molnar <[email protected]>
> >>> Signed-off-by: Tim Chen <[email protected]>
> >>> ---
> >>> arch/x86/include/asm/mmu.h | 15 +++++++++++++--
> >>> arch/x86/include/asm/mmu_context.h | 5 +++++
> >>> arch/x86/mm/tlb.c | 2 ++
> >>> 3 files changed, 20 insertions(+), 2 deletions(-)
> >>>
> >>
> >> Does not apply to 4.4.y :(
> >>
> >> Can you provide a working backport for that tree?
> >>
> >
> > Okay. Will do. Thanks.
> >
>
>
> Greg,
>
> I actually found that there are a number of dependent IBPB related patches that haven't been
> backported yet to 4.4:
>
> x86/cpufeatures: Add AMD feature bits for Speculation Control
> (cherry picked from commit 5d10cbc91d9eb5537998b65608441b592eec65e7)
>
> x86/msr: Add definitions for new speculation control MSRs
> (cherry picked from commit 1e340c60d0dd3ae07b5bedc16a0469c14b9f3410)
>
> x86/speculation: Add basic IBPB (Indirect Branch Prediction Barrier) support
> (cherry picked from commit 20ffa1caecca4db8f79fe665acdeaa5af815a24d)
>
> x86/cpufeatures: Clean up Spectre v2 related CPUID flags
> (cherry picked from commit 2961298efe1ea1b6fc0d7ee8b76018fa6c0bcef2)
>
> And probably and a few more.
> You have plans to backport these patches?

I don't, but I think someone from Amazon was looking into it, but I
haven't heard from them in a few weeks. I'll gladly take patches if you
have them, or at the worse case, a list like above of the git commits
that are missing.

thanks,

greg k-h