[bug] LTP mmap03 stuck in page fault loop after c46241a370a6 ("powerpc/pkeys: Check vma before returning key fault error to the user")

[bug] LTP mmap03 stuck in page fault loop after c46241a370a6 ("powerpc/pkeys: Check vma before returning key fault error to the user")

[Jan Stancek]

Hi,

LTP mmap03 is getting stuck in page fault loop after commit
  c46241a370a6 ("powerpc/pkeys: Check vma before returning key fault error to the user")

System is ppc64le P9 lpar [1] running v5.8-rc2-34-g3e08a95294a4.

Here's a minimized reproducer:
------------------------- 8< -----------------------------
#include <fcntl.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
#include <sys/mman.h>

int main(int ac, char **av)
{
        int page_sz = getpagesize();
        int fildes;
        char *addr;

        fildes = open("tempfile", O_WRONLY | O_CREAT, 0666);
        write(fildes, &fildes, sizeof(fildes));
        close(fildes);

        fildes = open("tempfile", O_RDONLY);
        unlink("tempfile");

        addr = mmap(0, page_sz, PROT_EXEC, MAP_FILE | MAP_PRIVATE, fildes, 0);

        printf("%d\n", *addr);
        return 0;
}
------------------------- >8 -----------------------------

This would previously end quickly with segmentation fault, after
commit c46241a370a6 test is stuck:

# perf stat timeout 5 ./a.out

 Performance counter stats for 'timeout 5 ./a.out':

          5,001.74 msec task-clock                #    1.000 CPUs utilized
                 9      context-switches          #    0.002 K/sec
                 0      cpu-migrations            #    0.000 K/sec
         3,094,893      page-faults               #    0.619 M/sec
    18,940,869,512      cycles                    #    3.787 GHz                      (33.39%)
     1,377,005,087      stalled-cycles-frontend   #    7.27% frontend cycles idle     (50.19%)
    10,949,936,056      stalled-cycles-backend    #   57.81% backend cycles idle      (16.62%)
    21,133,828,748      instructions              #    1.12  insn per cycle
                                                  #    0.52  stalled cycles per insn  (33.22%)
     4,395,016,137      branches                  #  878.698 M/sec                    (49.81%)
       164,499,002      branch-misses             #    3.74% of all branches          (16.60%)

       5.001237248 seconds time elapsed

       0.321276000 seconds user
       4.680772000 seconds sys


access_pkey_error() in page fault handler now always seem to return false:

  __do_page_fault
    access_pkey_error(is_pkey: 1, is_exec: 0, is_write: 0)
      arch_vma_access_permitted
        pkey_access_permitted
          if (!is_pkey_enabled(pkey))
            return true
      return false

Regards,
Jan

[1]
Architecture:                    ppc64le
Byte Order:                      Little Endian
CPU(s):                          8
On-line CPU(s) list:             0-7
Thread(s) per core:              8
Core(s) per socket:              1
Socket(s):                       1
NUMA node(s):                    2
Model:                           2.2 (pvr 004e 0202)
Model name:                      POWER9 (architected), altivec supported
Hypervisor vendor:               pHyp
Virtualization type:             para
L1d cache:                       32 KiB
L1i cache:                       32 KiB
NUMA node0 CPU(s):
NUMA node1 CPU(s):               0-7
Physical sockets:                2
Physical chips:                  1
Physical cores/chip:             8
Vulnerability Itlb multihit:     Not affected
Vulnerability L1tf:              Mitigation; RFI Flush, L1D private per thread
Vulnerability Mds:               Not affected
Vulnerability Meltdown:          Mitigation; RFI Flush, L1D private per thread
Vulnerability Spec store bypass: Mitigation; Kernel entry/exit barrier (eieio)
Vulnerability Spectre v1:        Mitigation; __user pointer sanitization, ori31 speculation barrier enabled
Vulnerability Spectre v2:        Mitigation; Indirect branch cache disabled, Software link stack flush
Vulnerability Srbds:             Not affected
Vulnerability Tsx async abort:   Not affected

Re: [bug] LTP mmap03 stuck in page fault loop after c46241a370a6 ("powerpc/pkeys: Check vma before returning key fault error to the user")

[Aneesh Kumar K.V]

Hi Jan,

On 6/26/20 12:29 PM, Jan Stancek wrote:
> Hi,
> 
> LTP mmap03 is getting stuck in page fault loop after commit
>    c46241a370a6 ("powerpc/pkeys: Check vma before returning key fault error to the user")
> 
> System is ppc64le P9 lpar [1] running v5.8-rc2-34-g3e08a95294a4.
> 
> Here's a minimized reproducer:
> ------------------------- 8< -----------------------------
> #include <fcntl.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <unistd.h>
> #include <sys/mman.h>
> 
> int main(int ac, char **av)
> {
>          int page_sz = getpagesize();
>          int fildes;
>          char *addr;
> 
>          fildes = open("tempfile", O_WRONLY | O_CREAT, 0666);
>          write(fildes, &fildes, sizeof(fildes));
>          close(fildes);
> 
>          fildes = open("tempfile", O_RDONLY);
>          unlink("tempfile");
> 
>          addr = mmap(0, page_sz, PROT_EXEC, MAP_FILE | MAP_PRIVATE, fildes, 0);
> 
>          printf("%d\n", *addr);
>          return 0;
> }
> ------------------------- >8 -----------------------------

Thanks for the report. This is execute only key where vma has the 
implied read permission. So The patch do break this case. I will see how 
best we can handle  PROT_EXEC and the multi threaded test that required 
the change in the patch.


-aneesh

Re: [bug] LTP mmap03 stuck in page fault loop after c46241a370a6 ("powerpc/pkeys: Check vma before returning key fault error to the user")

[Aneesh Kumar K.V]

On 6/26/20 1:17 PM, Aneesh Kumar K.V wrote:
> Hi Jan,
> 
> On 6/26/20 12:29 PM, Jan Stancek wrote:
>> Hi,
>>
>> LTP mmap03 is getting stuck in page fault loop after commit
>>    c46241a370a6 ("powerpc/pkeys: Check vma before returning key fault 
>> error to the user")
>>
>> System is ppc64le P9 lpar [1] running v5.8-rc2-34-g3e08a95294a4.
>>
>> Here's a minimized reproducer:
>> ------------------------- 8< -----------------------------
>> #include <fcntl.h>
>> #include <stdio.h>
>> #include <stdlib.h>
>> #include <unistd.h>
>> #include <sys/mman.h>
>>
>> int main(int ac, char **av)
>> {
>>          int page_sz = getpagesize();
>>          int fildes;
>>          char *addr;
>>
>>          fildes = open("tempfile", O_WRONLY | O_CREAT, 0666);
>>          write(fildes, &fildes, sizeof(fildes));
>>          close(fildes);
>>
>>          fildes = open("tempfile", O_RDONLY);
>>          unlink("tempfile");
>>
>>          addr = mmap(0, page_sz, PROT_EXEC, MAP_FILE | MAP_PRIVATE, 
>> fildes, 0);
>>
>>          printf("%d\n", *addr);
>>          return 0;
>> }
>> ------------------------- >8 -----------------------------
> 
> Thanks for the report. This is execute only key where vma has the 
> implied read permission. So The patch do break this case. I will see how 
> best we can handle  PROT_EXEC and the multi threaded test that required 
> the change in the patch.
> 

Can you check with this change? While checking for access permission we 
are checking against UAMOR value which i think is wrong. We just need to 
look at the AMR and IAMR values to check whether access is permitted or 
not. Even if UAMOR deny the userspace management of the key, we should
do the correct access check.

modified   arch/powerpc/mm/book3s64/pkeys.c
@@ -353,9 +353,6 @@ static bool pkey_access_permitted(int pkey, bool 
write, bool execute)
  	int pkey_shift;
  	u64 amr;

-	if (!is_pkey_enabled(pkey))
-		return true;
-
  	pkey_shift = pkeyshift(pkey);
  	if (execute && !(read_iamr() & (IAMR_EX_BIT << pkey_shift)))
  		return true;

Re: [bug] LTP mmap03 stuck in page fault loop after c46241a370a6 ("powerpc/pkeys: Check vma before returning key fault error to the user")

[Jan Stancek]

----- Original Message -----
> Can you check with this change? While checking for access permission we
> are checking against UAMOR value which i think is wrong. We just need to
> look at the AMR and IAMR values to check whether access is permitted or
> not. Even if UAMOR deny the userspace management of the key, we should
> do the correct access check.
> 
> modified   arch/powerpc/mm/book3s64/pkeys.c
> @@ -353,9 +353,6 @@ static bool pkey_access_permitted(int pkey, bool
> write, bool execute)
>   	int pkey_shift;
>   	u64 amr;
> 
> -	if (!is_pkey_enabled(pkey))
> -		return true;
> -
>   	pkey_shift = pkeyshift(pkey);
>   	if (execute && !(read_iamr() & (IAMR_EX_BIT << pkey_shift)))
>   		return true;
> 

This change fixes it for me. mmap03 and reproducer from previous
email no longer get stuck.

Thanks,
Jan