All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
To: linux-mm@kvack.org, linux-kernel@vger.kernel.org
Cc: x86@kernel.org, Borislav Petkov <bp@suse.de>,
	Andy Lutomirski <luto@kernel.org>, Rik van Riel <riel@redhat.com>,
	Mel Gorman <mgorman@suse.de>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>
Subject: [PATCH] x86/mm: disable preemption during CR3 read+write
Date: Fri,  5 Aug 2016 15:37:39 +0200	[thread overview]
Message-ID: <1470404259-26290-1-git-send-email-bigeasy@linutronix.de> (raw)

Usually current->mm (and therefore mm->pgd) stays the same during the
lifetime of a task so it does not matter if a task gets preempted during
the read and write of the CR3.

But then, there is this scenario on x86-UP:
TaskA is in do_exit() and exit_mm() sets current->mm = NULL followed by
mmput() -> exit_mmap() -> tlb_finish_mmu() -> tlb_flush_mmu() ->
tlb_flush_mmu_tlbonly() -> tlb_flush() -> flush_tlb_mm_range() ->
__flush_tlb_up() -> __flush_tlb() ->  __native_flush_tlb().

At this point current->mm is NULL but current->active_mm still points to
the "old" mm.
Let's preempt taskA _after_ native_read_cr3() by taskB. TaskB has its
own mm so CR3 has changed.
Now preempt back to taskA. TaskA has no ->mm set so it borrows taskB's
mm and so CR3 remains unchanged. Once taskA gets active it continues
where it was interrupted and that means it writes its old CR3 value
back. Everything is fine because userland won't need its memory
anymore.

Now the fun part. Let's preempt taskA one more time and get back to
taskB. This time switch_mm() won't do a thing because oldmm
(->active_mm) is the same as mm (as per context_switch()). So we remain
with a bad CR3 / pgd and return to userland.
The next thing that happens is handle_mm_fault() with an address for the
execution of its code in userland. handle_mm_fault() realizes that it
has a PTE with proper rights so it returns doing nothing. But the CPU
looks at the wrong pgd and insists that something is wrong and faults
again. And again. And one more time…

This pagefault circle continues until the scheduler gets tired of it and
puts another task on the CPU. It gets little difficult if the task is a
RT task with a high priority. The system will either freeze or it gets
fixed by the software watchdog thread which usually runs at RT-max prio.
But waiting for the watchdog will increase the latency of the RT task
which is no good.

Cc: stable@vger.kernel.org
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 arch/x86/include/asm/tlbflush.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/x86/include/asm/tlbflush.h b/arch/x86/include/asm/tlbflush.h
index 4e5be94e079a..1ee065954e24 100644
--- a/arch/x86/include/asm/tlbflush.h
+++ b/arch/x86/include/asm/tlbflush.h
@@ -135,7 +135,14 @@ static inline void cr4_set_bits_and_update_boot(unsigned long mask)
 
 static inline void __native_flush_tlb(void)
 {
+	/*
+	 * if current->mm == NULL then we borrow a mm which may change during a
+	 * task switch and therefore we must not be preempted while we write CR3
+	 * back.
+	 */
+	preempt_disable();
 	native_write_cr3(native_read_cr3());
+	preempt_enable();
 }
 
 static inline void __native_flush_tlb_global_irq_disabled(void)
-- 
2.8.1

WARNING: multiple messages have this Message-ID (diff)
From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
To: linux-mm@kvack.org, linux-kernel@vger.kernel.org
Cc: x86@kernel.org, Borislav Petkov <bp@suse.de>,
	Andy Lutomirski <luto@kernel.org>, Rik van Riel <riel@redhat.com>,
	Mel Gorman <mgorman@suse.de>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>
Subject: [PATCH] x86/mm: disable preemption during CR3 read+write
Date: Fri,  5 Aug 2016 15:37:39 +0200	[thread overview]
Message-ID: <1470404259-26290-1-git-send-email-bigeasy@linutronix.de> (raw)

Usually current->mm (and therefore mm->pgd) stays the same during the
lifetime of a task so it does not matter if a task gets preempted during
the read and write of the CR3.

But then, there is this scenario on x86-UP:
TaskA is in do_exit() and exit_mm() sets current->mm = NULL followed by
mmput() -> exit_mmap() -> tlb_finish_mmu() -> tlb_flush_mmu() ->
tlb_flush_mmu_tlbonly() -> tlb_flush() -> flush_tlb_mm_range() ->
__flush_tlb_up() -> __flush_tlb() ->  __native_flush_tlb().

At this point current->mm is NULL but current->active_mm still points to
the "old" mm.
Let's preempt taskA _after_ native_read_cr3() by taskB. TaskB has its
own mm so CR3 has changed.
Now preempt back to taskA. TaskA has no ->mm set so it borrows taskB's
mm and so CR3 remains unchanged. Once taskA gets active it continues
where it was interrupted and that means it writes its old CR3 value
back. Everything is fine because userland won't need its memory
anymore.

Now the fun part. Let's preempt taskA one more time and get back to
taskB. This time switch_mm() won't do a thing because oldmm
(->active_mm) is the same as mm (as per context_switch()). So we remain
with a bad CR3 / pgd and return to userland.
The next thing that happens is handle_mm_fault() with an address for the
execution of its code in userland. handle_mm_fault() realizes that it
has a PTE with proper rights so it returns doing nothing. But the CPU
looks at the wrong pgd and insists that something is wrong and faults
again. And again. And one more timea?|

This pagefault circle continues until the scheduler gets tired of it and
puts another task on the CPU. It gets little difficult if the task is a
RT task with a high priority. The system will either freeze or it gets
fixed by the software watchdog thread which usually runs at RT-max prio.
But waiting for the watchdog will increase the latency of the RT task
which is no good.

Cc: stable@vger.kernel.org
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 arch/x86/include/asm/tlbflush.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/x86/include/asm/tlbflush.h b/arch/x86/include/asm/tlbflush.h
index 4e5be94e079a..1ee065954e24 100644
--- a/arch/x86/include/asm/tlbflush.h
+++ b/arch/x86/include/asm/tlbflush.h
@@ -135,7 +135,14 @@ static inline void cr4_set_bits_and_update_boot(unsigned long mask)
 
 static inline void __native_flush_tlb(void)
 {
+	/*
+	 * if current->mm == NULL then we borrow a mm which may change during a
+	 * task switch and therefore we must not be preempted while we write CR3
+	 * back.
+	 */
+	preempt_disable();
 	native_write_cr3(native_read_cr3());
+	preempt_enable();
 }
 
 static inline void __native_flush_tlb_global_irq_disabled(void)
-- 
2.8.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

             reply	other threads:[~2016-08-05 13:37 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-05 13:37 Sebastian Andrzej Siewior [this message]
2016-08-05 13:37 ` [PATCH] x86/mm: disable preemption during CR3 read+write Sebastian Andrzej Siewior
2016-08-05 13:53 ` Peter Zijlstra
2016-08-05 13:53   ` Peter Zijlstra
2016-08-05 14:38 ` Rik van Riel
2016-08-05 15:42 ` Andy Lutomirski
2016-08-05 15:42   ` Andy Lutomirski
2016-08-05 15:52   ` Sebastian Andrzej Siewior
2016-08-05 15:52     ` Sebastian Andrzej Siewior
2016-08-10 18:10 ` [tip:x86/urgent] x86/mm: Disable " tip-bot for Sebastian Andrzej Siewior
2017-10-17 15:57 [PATCH] " Sebastian Andrzej Siewior
2017-10-18 15:01 ` Bernhard Kaindl
2017-10-18 16:51   ` Greg KH

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1470404259-26290-1-git-send-email-bigeasy@linutronix.de \
    --to=bigeasy@linutronix.de \
    --cc=a.p.zijlstra@chello.nl \
    --cc=bp@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=luto@kernel.org \
    --cc=mgorman@suse.de \
    --cc=riel@redhat.com \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.