Re: [PATCH] powerpc/mm: Fix false detection of read faults

2023-04-05 Thread Michael Ellerman
On Fri, 10 Mar 2023 16:08:34 +1100, Russell Currey wrote:
> To support detection of read faults with Radix execute-only memory, the
> vma_is_accessible() check in access_error() (which checks for PROT_NONE)
> was replaced with a check to see if VM_READ was missing, and if so,
> returns true to assert the fault was caused by a bad read.
> 
> This is incorrect, as it ignores that both VM_WRITE and VM_EXEC imply
> read on powerpc, as defined in protection_map[].  This causes mappings
> containing VM_WRITE or VM_EXEC without VM_READ to misreport the cause of
> page faults, since the MMU is still allowing reads.
> 
> [...]

Applied to powerpc/fixes.

[1/1] powerpc/mm: Fix false detection of read faults
  https://git.kernel.org/powerpc/c/f2c7e3562b4c4f1699acc1538ebf3e75f5cced35

cheers


[PATCH] powerpc/mm: Fix false detection of read faults

2023-03-09 Thread Russell Currey
To support detection of read faults with Radix execute-only memory, the
vma_is_accessible() check in access_error() (which checks for PROT_NONE)
was replaced with a check to see if VM_READ was missing, and if so,
returns true to assert the fault was caused by a bad read.

This is incorrect, as it ignores that both VM_WRITE and VM_EXEC imply
read on powerpc, as defined in protection_map[].  This causes mappings
containing VM_WRITE or VM_EXEC without VM_READ to misreport the cause of
page faults, since the MMU is still allowing reads.

Correct this by restoring the original vma_is_accessible() check for
PROT_NONE mappings, and adding a separate check for Radix PROT_EXEC-only
mappings.

Fixes: 395cac7752b9 ("powerpc/mm: Support execute-only memory on the Radix MMU")
Reported-by: Michal Suchánek 
Tested-by: Benjamin Gray 
Signed-off-by: Russell Currey 
---
 arch/powerpc/mm/fault.c | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
index c7ae86b04b8a..d0710ecc1fc7 100644
--- a/arch/powerpc/mm/fault.c
+++ b/arch/powerpc/mm/fault.c
@@ -271,11 +271,16 @@ static bool access_error(bool is_write, bool is_exec, 
struct vm_area_struct *vma
}
 
/*
-* Check for a read fault.  This could be caused by a read on an
-* inaccessible page (i.e. PROT_NONE), or a Radix MMU execute-only page.
+* VM_READ, VM_WRITE and VM_EXEC all imply read permissions, as
+* defined in protection_map[].  Read faults can only be caused by
+* a PROT_NONE mapping, or with a PROT_EXEC-only mapping on Radix.
 */
-   if (unlikely(!(vma->vm_flags & VM_READ)))
+   if (unlikely(!vma_is_accessible(vma)))
return true;
+
+   if (unlikely(radix_enabled() && ((vma->vm_flags & VM_ACCESS_FLAGS) == 
VM_EXEC)))
+   return true;
+
/*
 * We should ideally do the vma pkey access check here. But in the
 * fault path, handle_mm_fault() also does the same check. To avoid
-- 
2.39.2