[PATCH] powerpc/book3s64/pkeys: Fix pkey_access_permitted w.r.t execute disable pkey

[Aneesh Kumar K.V]

Even if the IAMR value deny the execute access, current kernel return true
w.r.t pkey_access_permitted() for execute permission check if the AMR
read pkey bit is cleared.

This results in repeated page fault loop with a test like below.

 #define _GNU_SOURCE
 #include <errno.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <signal.h>
 #include <inttypes.h>

 #include <assert.h>
 #include <malloc.h>
 #include <unistd.h>
 #include <pthread.h>
 #include <sys/mman.h>

 #ifdef SYS_pkey_mprotect
 #undef SYS_pkey_mprotect
 #endif

 #ifdef SYS_pkey_alloc
 #undef SYS_pkey_alloc
 #endif

 #ifdef SYS_pkey_free
 #undef SYS_pkey_free
 #endif

 #undef PKEY_DISABLE_EXECUTE
 #define PKEY_DISABLE_EXECUTE	0x4

 #define SYS_pkey_mprotect	386
 #define SYS_pkey_alloc		384
 #define SYS_pkey_free		385

 #define PPC_INST_NOP		0x60000000
 #define PPC_INST_BLR		0x4e800020
 #define PROT_RWX		(PROT_READ | PROT_WRITE | PROT_EXEC)

static int sys_pkey_mprotect(void *addr, size_t len, int prot, int pkey)
{
	return syscall(SYS_pkey_mprotect, addr, len, prot, pkey);
}

static int sys_pkey_alloc(unsigned long flags, unsigned long access_rights)
{
	return syscall(SYS_pkey_alloc, flags, access_rights);
}

static int sys_pkey_free(int pkey)
{
	return syscall(SYS_pkey_free, pkey);
}

static void do_execute(void *region)
{
	/* jump to region */
	asm volatile(
		"mtctr	%0;"
		"bctrl"
		: : "r"(region) : "ctr", "lr");
}

static void do_protect(void *region)
{
	size_t pgsize;
	int i, pkey;

	pgsize = getpagesize();

	pkey = sys_pkey_alloc(0, PKEY_DISABLE_EXECUTE);
	assert (pkey > 0);

	/* perform mprotect */
	assert(!sys_pkey_mprotect(region, pgsize, PROT_RWX, pkey));
	do_execute(region);

	/* free pkey */
	assert(!sys_pkey_free(pkey));

}

int main(int argc, char **argv)
{
	size_t pgsize, numinsns;
	unsigned int *region;
	int i;

	/* allocate memory region to protect */
	pgsize = getpagesize();
	region = memalign(pgsize, pgsize);
	assert(region != NULL);
	assert(!mprotect(region, pgsize, PROT_RWX));

	/* fill page with NOPs with a BLR at the end */
	numinsns = pgsize / sizeof(region[0]);
	for (i = 0; i < numinsns - 1; i++)
		region[i] = PPC_INST_NOP;
	region[i] = PPC_INST_BLR;

	do_protect(region);

	return EXIT_SUCCESS;
}

Fixes: f2407ef3ba22 ("powerpc: helper to validate key-access permissions of a pte")

Reported-by: Sandipan Das <sandipan at linux.ibm.com>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar at linux.ibm.com>
---
 arch/powerpc/mm/book3s64/pkeys.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
index ca5fcb4bff32..d174106bab67 100644
--- a/arch/powerpc/mm/book3s64/pkeys.c
+++ b/arch/powerpc/mm/book3s64/pkeys.c
@@ -354,12 +354,14 @@ static bool pkey_access_permitted(int pkey, bool write, bool execute)
 	u64 amr;
 
 	pkey_shift = pkeyshift(pkey);
-	if (execute && !(read_iamr() & (IAMR_EX_BIT << pkey_shift)))
-		return true;
+	if (execute)
+		return !(read_iamr() & (IAMR_EX_BIT << pkey_shift));
+
+	amr = read_amr();
+	if (write)
+		return !(amr & (AMR_WR_BIT << pkey_shift));
 
-	amr = read_amr(); /* Delay reading amr until absolutely needed */
-	return ((!write && !(amr & (AMR_RD_BIT << pkey_shift))) ||
-		(write &&  !(amr & (AMR_WR_BIT << pkey_shift))));
+	return !(amr & (AMR_RD_BIT << pkey_shift));
 }
 
 bool arch_pte_access_permitted(u64 pte, bool write, bool execute)
-- 
2.26.2