2019-03-26 00:19:34

by Ralph Campbell

[permalink] [raw]
Subject: [PATCH v2 0/1] x86/mm: Fix limit mmap() of /dev/mem to valid physical

From: Ralph Campbell <[email protected]>

I was debugging with v5.1.0-rc1 and while booting I hit a
kernel BUG at arch/x86/mm/physaddr.c:27
which I fixed with the following patch but now I can't seem
to reproduce the exact setup that triggered it.
Still, it seems like a valid problem and maybe my difficulty
in reproducing it explains why others haven't seen it earlier.

Changes from v1 to v2:
* Updated the change log and patch as suggested by Thomas Gleixner.

Ralph Campbell (1):
x86/mm: Fix limit mmap() of /dev/mem to valid physical addresses

arch/x86/mm/mmap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

--
2.20.1



2019-03-26 00:19:44

by Ralph Campbell

[permalink] [raw]
Subject: [PATCH v2 1/1] x86/mm: Fix limit mmap() of /dev/mem to valid physical addresses

From: Ralph Campbell <[email protected]>

valid_phys_addr_range() is used to sanity check the physical address range
of an operation, e.g., access to /dev/mem. It uses __pa(high_memory)
internally.

If memory is populated at the end of the physical address space, then
__pa(high_memory) is outside of the physical address space because:

high_memory = (void *)__va(max_pfn * PAGE_SIZE - 1) + 1;

For the comparison in valid_phys_addr_range() this is not an issue, but if
CONFIG_DEBUG_VIRTUAL is enabled, __pa() maps to __phys_addr(), which
verifies that the resulting physical address is within the valid physical
address space of the CPU. So in the case that memory is populated at the
end of the physical address space, this is not true and triggers a
VIRTUAL_BUG_ON().

Use __pa(high_memory - 1) to prevent the conversion from going beyond
the end of valid physical addresses.

Fixes: be62a3204406 ("x86/mm: Limit mmap() of /dev/mem to valid physical addresses")
Signed-off-by: Ralph Campbell <[email protected]>
Cc: Craig Bergstrom <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Boris Ostrovsky <[email protected]>
Cc: Fengguang Wu <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Hans Verkuil <[email protected]>
Cc: Mauro Carvalho Chehab <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Sander Eikelenboom <[email protected]>
Cc: Sean Young <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
---
arch/x86/mm/mmap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/mm/mmap.c b/arch/x86/mm/mmap.c
index db3165714521..196bed43d5e6 100644
--- a/arch/x86/mm/mmap.c
+++ b/arch/x86/mm/mmap.c
@@ -230,7 +230,7 @@ bool mmap_address_hint_valid(unsigned long addr, unsigned long len)
/* Can we access it for direct reading/writing? Must be RAM: */
int valid_phys_addr_range(phys_addr_t addr, size_t count)
{
- return addr + count <= __pa(high_memory);
+ return addr + count - 1 <= __pa(high_memory - 1);
}

/* Can we access it through mmap? Must be a valid physical address: */
--
2.20.1


Subject: [tip:x86/urgent] x86/mm: Don't exceed the valid physical address space

Commit-ID: 92c77f7c4d5dfaaf45b2ce19360e69977c264766
Gitweb: https://git.kernel.org/tip/92c77f7c4d5dfaaf45b2ce19360e69977c264766
Author: Ralph Campbell <[email protected]>
AuthorDate: Mon, 25 Mar 2019 17:18:17 -0700
Committer: Thomas Gleixner <[email protected]>
CommitDate: Thu, 28 Mar 2019 14:13:51 +0100

x86/mm: Don't exceed the valid physical address space

valid_phys_addr_range() is used to sanity check the physical address range
of an operation, e.g., access to /dev/mem. It uses __pa(high_memory)
internally.

If memory is populated at the end of the physical address space, then
__pa(high_memory) is outside of the physical address space because:

high_memory = (void *)__va(max_pfn * PAGE_SIZE - 1) + 1;

For the comparison in valid_phys_addr_range() this is not an issue, but if
CONFIG_DEBUG_VIRTUAL is enabled, __pa() maps to __phys_addr(), which
verifies that the resulting physical address is within the valid physical
address space of the CPU. So in the case that memory is populated at the
end of the physical address space, this is not true and triggers a
VIRTUAL_BUG_ON().

Use __pa(high_memory - 1) to prevent the conversion from going beyond
the end of valid physical addresses.

Fixes: be62a3204406 ("x86/mm: Limit mmap() of /dev/mem to valid physical addresses")
Signed-off-by: Ralph Campbell <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Cc: Craig Bergstrom <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Boris Ostrovsky <[email protected]>
Cc: Fengguang Wu <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Hans Verkuil <[email protected]>
Cc: Mauro Carvalho Chehab <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Sander Eikelenboom <[email protected]>
Cc: Sean Young <[email protected]>

Link: https://lkml.kernel.org/r/[email protected]

---
arch/x86/mm/mmap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/mm/mmap.c b/arch/x86/mm/mmap.c
index db3165714521..dc726e07d8ba 100644
--- a/arch/x86/mm/mmap.c
+++ b/arch/x86/mm/mmap.c
@@ -230,7 +230,7 @@ bool mmap_address_hint_valid(unsigned long addr, unsigned long len)
/* Can we access it for direct reading/writing? Must be RAM: */
int valid_phys_addr_range(phys_addr_t addr, size_t count)
{
- return addr + count <= __pa(high_memory);
+ return addr + count - 1 <= __pa(high_memory - 1);
}

/* Can we access it through mmap? Must be a valid physical address: */