linux-kernel - [PATCH 30/37] x86, pkeys: make mprotect_key() mask off additional vm

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] [thread-next>] [day] [month] [year] [list]

Message-Id: <20151117033553.B5C1324D@viggo.jf.intel.com>
Date:	Mon, 16 Nov 2015 19:35:53 -0800
From:	Dave Hansen <dave@...1.net>
To:	linux-kernel@...r.kernel.org
Cc:	x86@...nel.org, Dave Hansen <dave@...1.net>,
	dave.hansen@...ux.intel.com
Subject: [PATCH 30/37] x86, pkeys: make mprotect_key() mask off additional vm_flags


From: Dave Hansen <dave.hansen@...ux.intel.com>

Today, mprotect() takes 4 bits of data: PROT_READ/WRITE/EXEC/NONE.
Three of those bits: READ/WRITE/EXEC get translated directly in to
vma->vm_flags by calc_vm_prot_bits().  If a bit is unset in
mprotect()'s 'prot' argument then it must be cleared in vma->vm_flags
during the mprotect() call.

We do the by first calculating the VMA flags we want set, then
clearing the ones we do not want to inherit from the original VMA:

	vm_flags = calc_vm_prot_bits(prot, key);
	...
	newflags = vm_flags;
	newflags |= (vma->vm_flags & ~(VM_READ | VM_WRITE | VM_EXEC));

However, we *also* want to mask off the original VMA's vm_flags in
which we store the protection key.

To do that, this patch adds a new macro:

	ARCH_VM_FLAGS_AFFECTED_BY_MPROTECT

which allows the architecture to specify additional bits that it would
like cleared.  We use that to ensure that the VM_PKEY_BIT* bits get
cleared.

This got missed in my testing because I was always going from a pkey=0
VMA to a nonzero one.  The current code works when we only set bits
but never clear them.  I've fixed this up in my testing.

Signed-off-by: Dave Hansen <dave.hansen@...ux.intel.com>
---

 b/arch/x86/include/asm/pkeys.h |    2 ++
 b/include/linux/pkeys.h        |    1 +
 b/mm/mprotect.c                |    9 ++++++++-
 3 files changed, 11 insertions(+), 1 deletion(-)

diff -puN arch/x86/include/asm/pkeys.h~pkeys-mask-off-correct-vm_flags arch/x86/include/asm/pkeys.h
--- a/arch/x86/include/asm/pkeys.h~pkeys-mask-off-correct-vm_flags	2015-11-16 12:36:29.797643668 -0800
+++ b/arch/x86/include/asm/pkeys.h	2015-11-16 19:21:56.330145213 -0800
@@ -5,6 +5,8 @@
 				CONFIG_NR_PROTECTION_KEYS : 1)
 #define arch_validate_pkey(pkey) (((pkey) >= 0) && ((pkey) < arch_max_pkey()))
 
+#define ARCH_VM_PKEY_FLAGS (VM_PKEY_BIT0 | VM_PKEY_BIT1 | VM_PKEY_BIT2 | VM_PKEY_BIT3)
+
 #endif /*_ASM_X86_PKEYS_H */
 
 
diff -puN include/linux/pkeys.h~pkeys-mask-off-correct-vm_flags include/linux/pkeys.h
--- a/include/linux/pkeys.h~pkeys-mask-off-correct-vm_flags	2015-11-16 12:36:29.799643758 -0800
+++ b/include/linux/pkeys.h	2015-11-16 19:21:56.331145258 -0800
@@ -7,6 +7,7 @@
 #include <asm/pkeys.h>
 #include <asm/mmu_context.h>
 #else /* ! CONFIG_ARCH_HAS_PKEYS */
+#define ARCH_VM_PKEY_FLAGS 0
 
 /*
  * This is called from mprotect_pkey().
diff -puN mm/mprotect.c~pkeys-mask-off-correct-vm_flags mm/mprotect.c
--- a/mm/mprotect.c~pkeys-mask-off-correct-vm_flags	2015-11-16 12:36:29.800643804 -0800
+++ b/mm/mprotect.c	2015-11-16 19:21:56.332145303 -0800
@@ -406,6 +406,13 @@ static int do_mprotect_pkey(unsigned lon
 
 	for (nstart = start ; ; ) {
 		unsigned long newflags;
+		/*
+		 * Each mprotect() call explicitly passes r/w/x permissions.
+		 * If a permission is not passed to mprotect(), it must be
+		 * cleared from the VMA.
+		 */
+		unsigned long mask_off_old_flags = VM_READ | VM_WRITE | VM_EXEC;
+		mask_off_old_flags |= ARCH_VM_PKEY_FLAGS;
 
 		/* Here we know that vma->vm_start <= nstart < vma->vm_end. */
 
@@ -417,7 +424,7 @@ static int do_mprotect_pkey(unsigned lon
 			newflags = calc_vm_prot_bits(prot, vma_pkey(vma));
 		else
 			newflags = calc_vm_prot_bits(prot, pkey);
-		newflags |= (vma->vm_flags & ~(VM_READ | VM_WRITE | VM_EXEC));
+		newflags |= (vma->vm_flags & ~mask_off_old_flags);
 
 		/* newflags >> 4 shift VM_MAY% in place of VM_% */
 		if ((newflags & ~(newflags >> 4)) & (VM_READ | VM_WRITE | VM_EXEC)) {
_
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/