[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 <sandi...@linux.ibm.com>
Signed-off-by: Aneesh Kumar K.V <aneesh.ku...@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