Re: [PATCH v5 10/10] powerpc/mm: Detect bad KUAP faults

christophe leroy [off-list ref] writes:

Le 08/03/2019 à 09:53, Christophe Leroy a écrit :

quoted

Le 08/03/2019 à 02:16, Michael Ellerman a écrit :

...

quoted

quoted

diff --git a/arch/powerpc/include/asm/book3s/64/kup-radix.h 

b/arch/powerpc/include/asm/book3s/64/kup-radix.h
index 3d60b04fc3f6..8d2ddc61e92e 100644

--- a/arch/powerpc/include/asm/book3s/64/kup-radix.h
+++ b/arch/powerpc/include/asm/book3s/64/kup-radix.h

@@ -100,6 +100,18 @@ static inline void prevent_user_access(void 

__user *to, const void __user *from,
      set_kuap(AMR_KUAP_BLOCKED);
  }
+static inline bool bad_kuap_fault(struct pt_regs *regs, bool is_write)
+{
+    if (mmu_has_feature(MMU_FTR_RADIX_KUAP) &&
+        ((is_write && (regs->kuap & AMR_KUAP_BLOCK_WRITE)) ||
+         (!is_write && (regs->kuap & AMR_KUAP_BLOCK_READ))))
+    {

Should this { go on the previous line ?

quoted

+        WARN(true, "Bug: %s fault blocked by AMR!", is_write ? 
"Write" : "Read");
+        return true;

Could just be
     return WARN(true, ....)

Or even
     return WARN(mmu_has_feature(MMU_FTR_RADIX_KUAP) &&
         ((is_write && (regs->kuap & AMR_KUAP_BLOCK_WRITE)) ||
          (!is_write && (regs->kuap & AMR_KUAP_BLOCK_READ))), ...);

Could also be simplified as follows since (is_write && ...) and 
(!is_write && ...) are mutually exclusive:

mmu_has_feature(MMU_FTR_RADIX_KUAP) &&
(regs->kuap & (is_write ? AMR_KUAP_BLOCK_WRITE : AMR_KUAP_BLOCK_READ))

Yeah I guess that is better.

I'll do:

static inline bool bad_kuap_fault(struct pt_regs *regs, bool is_write)
{
	return WARN(mmu_has_feature(MMU_FTR_RADIX_KUAP) &&
		    (regs->kuap & (is_write ? AMR_KUAP_BLOCK_WRITE : AMR_KUAP_BLOCK_READ)),
		    "Bug: %s fault blocked by AMR!", is_write ? "Write" : "Read");
}


cheers