linux-kernel - [tip:x86/urgent] x86/mm: Limit mmap() of /dev/mem to valid physical addresses

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>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <tip-be62a32044061cb4a3b70a10598e093f1319102e@git.kernel.org>
Date:   Thu, 16 Nov 2017 07:49:08 -0800
From:   tip-bot for Craig Bergstrom <tipbot@...or.com>
To:     linux-tip-commits@...r.kernel.org
Cc:     peterz@...radead.org, hpa@...or.com, torvalds@...ux-foundation.org,
        mchehab@...pensource.com, hans.verkuil@...co.com,
        boris.ostrovsky@...cle.com, sean@...s.org, fengguang.wu@...el.com,
        craigb@...gle.com, tglx@...utronix.de,
        linux-kernel@...r.kernel.org, gregkh@...uxfoundation.org,
        mingo@...nel.org, linux@...elenboom.it
Subject: [tip:x86/urgent] x86/mm: Limit mmap() of /dev/mem to valid physical
 addresses

Commit-ID:  be62a32044061cb4a3b70a10598e093f1319102e
Gitweb:     https://git.kernel.org/tip/be62a32044061cb4a3b70a10598e093f1319102e
Author:     Craig Bergstrom <craigb@...gle.com>
AuthorDate: Wed, 15 Nov 2017 15:29:51 -0700
Committer:  Ingo Molnar <mingo@...nel.org>
CommitDate: Thu, 16 Nov 2017 12:49:48 +0100

x86/mm: Limit mmap() of /dev/mem to valid physical addresses

One thing /dev/mem access APIs should verify is that there's no way
that excessively large pfn's can leak into the high bits of the
page table entry.

In particular, if people can use "very large physical page addresses"
through /dev/mem to set the bits past bit 58 - SOFTW4 and permission
key bits and NX bit, that could *really* confuse the kernel.

We had an earlier attempt:

  ce56a86e2ade ("x86/mm: Limit mmap() of /dev/mem to valid physical addresses")

... which turned out to be too restrictive (breaking mem=... bootups for example) and
had to be reverted in:

  90edaac62729 ("Revert "x86/mm: Limit mmap() of /dev/mem to valid physical addresses"")

This v2 attempt modifies the original patch and makes sure that mmap(/dev/mem)
limits the pfns so that it at least fits in the actual pteval_t architecturally:

 - Make sure mmap_mem() actually validates that the offset fits in phys_addr_t

    ( This may be indirectly true due to some other check, but it's not
      entirely obvious. )

 - Change valid_mmap_phys_addr_range() to just use phys_addr_valid()
   on the top byte

    ( Top byte is sufficient, because mmap_mem() has already checked that
      it cannot wrap. )

 - Add a few comments about what the valid_phys_addr_range() vs.
   valid_mmap_phys_addr_range() difference is.

Signed-off-by: Craig Bergstrom <craigb@...gle.com>
[ Fixed the checks and added comments. ]
Signed-off-by: Linus Torvalds <torvalds@...ux-foundation.org>
[ Collected the discussion and patches into a commit. ]
Cc: Boris Ostrovsky <boris.ostrovsky@...cle.com>
Cc: Fengguang Wu <fengguang.wu@...el.com>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: Hans Verkuil <hans.verkuil@...co.com>
Cc: Mauro Carvalho Chehab <mchehab@...pensource.com>
Cc: Peter Zijlstra <peterz@...radead.org>
Cc: Sander Eikelenboom <linux@...elenboom.it>
Cc: Sean Young <sean@...s.org>
Cc: Thomas Gleixner <tglx@...utronix.de>
Link: http://lkml.kernel.org/r/CA+55aFyEcOMb657vWSmrM13OxmHxC-XxeBmNis=DwVvpJUOogQ@mail.gmail.com
Signed-off-by: Ingo Molnar <mingo@...nel.org>
---
 arch/x86/include/asm/io.h |  4 ++++
 arch/x86/mm/mmap.c        | 16 ++++++++++++++++
 drivers/char/mem.c        |  4 ++++
 3 files changed, 24 insertions(+)

diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
index 93ae8ae..95e9486 100644
--- a/arch/x86/include/asm/io.h
+++ b/arch/x86/include/asm/io.h
@@ -111,6 +111,10 @@ build_mmio_write(__writeq, "q", unsigned long, "r", )
 
 #endif
 
+#define ARCH_HAS_VALID_PHYS_ADDR_RANGE
+extern int valid_phys_addr_range(phys_addr_t addr, size_t size);
+extern int valid_mmap_phys_addr_range(unsigned long pfn, size_t size);
+
 /**
  *	virt_to_phys	-	map virtual addresses to physical
  *	@address: address to remap
diff --git a/arch/x86/mm/mmap.c b/arch/x86/mm/mmap.c
index 62285fe..155ecba 100644
--- a/arch/x86/mm/mmap.c
+++ b/arch/x86/mm/mmap.c
@@ -33,6 +33,8 @@
 #include <linux/compat.h>
 #include <asm/elf.h>
 
+#include "physaddr.h"
+
 struct va_alignment __read_mostly va_align = {
 	.flags = -1,
 };
@@ -220,3 +222,17 @@ bool mmap_address_hint_valid(unsigned long addr, unsigned long len)
 
 	return (addr > DEFAULT_MAP_WINDOW) == (addr + len > DEFAULT_MAP_WINDOW);
 }
+
+/* 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);
+}
+
+/* Can we access it through mmap? Must be a valid physical address: */
+int valid_mmap_phys_addr_range(unsigned long pfn, size_t count)
+{
+	phys_addr_t addr = (phys_addr_t)pfn << PAGE_SHIFT;
+
+	return phys_addr_valid(addr + count - 1);
+}
diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index 970e124..6aefe53 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -343,6 +343,10 @@ static int mmap_mem(struct file *file, struct vm_area_struct *vma)
 	size_t size = vma->vm_end - vma->vm_start;
 	phys_addr_t offset = (phys_addr_t)vma->vm_pgoff << PAGE_SHIFT;
 
+	/* Does it even fit in phys_addr_t? */
+	if (offset >> PAGE_SHIFT != vma->vm_pgoff)
+		return -EINVAL;
+
 	/* It's illegal to wrap around the end of the physical address space. */
 	if (offset + (phys_addr_t)size - 1 < offset)
 		return -EINVAL;