• Aneesh Kumar K.V's avatar
    powerpc/book3s64/pkeys: Fix pkey_access_permitted() for execute disable pkey · 62c1cf71
    Aneesh Kumar K.V authored
    commit 192b6a78 upstream.
    
    Even if the IAMR value denies execute access, the current code returns
    true from pkey_access_permitted() for an 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;
      }
    
    The fix is to only check the IAMR for an execute check, the AMR value
    is not relevant.
    
    Fixes: f2407ef3 ("powerpc: helper to validate key-access permissions of a pte")
    Cc: stable@vger.kernel.org # v4.16+
    Reported-by: default avatarSandipan Das <sandipan@linux.ibm.com>
    Signed-off-by: default avatarAneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
    [mpe: Add detail to change log, tweak wording & formatting]
    Signed-off-by: default avatarMichael Ellerman <mpe@ellerman.id.au>
    Link: https://lore.kernel.org/r/20200712132047.1038594-1-aneesh.kumar@linux.ibm.comSigned-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
    62c1cf71
pkeys.c 10.5 KB