linux-kernel - [tip: x86/urgent] Revert "x86/mm/64: Do not sync vmalloc/ioremap mappings"

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <159673380949.3192.14983535678351816282.tip-bot2@tip-bot2>
Date:   Thu, 06 Aug 2020 17:10:09 -0000
From:   "tip-bot2 for Ingo Molnar" <tip-bot2@...utronix.de>
To:     linux-tip-commits@...r.kernel.org
Cc:     "Jason A. Donenfeld" <Jason@...c4.com>,
        Joerg Roedel <jroedel@...e.de>, Ingo Molnar <mingo@...nel.org>,
        x86 <x86@...nel.org>, LKML <linux-kernel@...r.kernel.org>
Subject: [tip: x86/urgent] Revert "x86/mm/64: Do not sync vmalloc/ioremap mappings"

The following commit has been merged into the x86/urgent branch of tip:

Commit-ID:     f17506e2f14bfa8a6a2de9b8b6a3ccc6b6f7c9b6
Gitweb:        https://git.kernel.org/tip/f17506e2f14bfa8a6a2de9b8b6a3ccc6b6f7c9b6
Author:        Ingo Molnar <mingo@...nel.org>
AuthorDate:    Thu, 06 Aug 2020 15:11:03 +02:00
Committer:     Ingo Molnar <mingo@...nel.org>
CommitterDate: Thu, 06 Aug 2020 15:11:03 +02:00

Revert "x86/mm/64: Do not sync vmalloc/ioremap mappings"

This reverts commit 8bb9bf242d1fee925636353807c511d54fde8986.

Jason reported that this causes a new oops in process_one_work(),
and bisected it to this commit.

Linus suspects that it was caused by missing pagetable synchronization:

> > BUG: unable to handle page fault for address: ffffe8ffffd00608
> > #PF: supervisor read access in kernel mode
> > #PF: error_code(0x0000) - not-present page
> > PGD 0 P4D 0
>
> Yeah, missing page table because it wasn't copied.
>
> Presumably because that kthread is using the active_mm of some random
> user space process that didn't get sync'ed.
>
> And the sync_global_pgds() may have ended up being sufficient
> synchronization with whoever allocated thigns, even if it wasn't about
> the TLB contents themselves.
>
> So apparently the "the page-table pages are all pre-allocated now" is
> simply not true.

Revert the commit for now.

Reported-by: "Jason A. Donenfeld" <Jason@...c4.com>
Analyzed-by: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Joerg Roedel <jroedel@...e.de>
Signed-off-by: Ingo Molnar <mingo@...nel.org>
---
 arch/x86/include/asm/pgtable_64_types.h | 2 ++
 arch/x86/mm/init_64.c                   | 5 +++++
 2 files changed, 7 insertions(+)

diff --git a/arch/x86/include/asm/pgtable_64_types.h b/arch/x86/include/asm/pgtable_64_types.h
index 52e5f5f..8f63efb 100644
--- a/arch/x86/include/asm/pgtable_64_types.h
+++ b/arch/x86/include/asm/pgtable_64_types.h
@@ -159,4 +159,6 @@ extern unsigned int ptrs_per_p4d;
 
 #define PGD_KERNEL_START	((PAGE_SIZE / 2) / sizeof(pgd_t))
 
+#define ARCH_PAGE_TABLE_SYNC_MASK	(pgtable_l5_enabled() ?	PGTBL_PGD_MODIFIED : PGTBL_P4D_MODIFIED)
+
 #endif /* _ASM_X86_PGTABLE_64_DEFS_H */
diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
index e65b96f..3f4e29a 100644
--- a/arch/x86/mm/init_64.c
+++ b/arch/x86/mm/init_64.c
@@ -217,6 +217,11 @@ static void sync_global_pgds(unsigned long start, unsigned long end)
 		sync_global_pgds_l4(start, end);
 }
 
+void arch_sync_kernel_mappings(unsigned long start, unsigned long end)
+{
+	sync_global_pgds(start, end);
+}
+
 /*
  * NOTE: This function is marked __ref because it calls __init function
  * (alloc_bootmem_pages). It's safe to do it ONLY when after_bootmem == 0.