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

[Michael Ellerman]

christophe leroy <christophe.leroy at c-s.fr> writes:
> Le 08/03/2019 à 09:53, Christophe Leroy a écrit :
>> Le 08/03/2019 à 02:16, Michael Ellerman a écrit :
...
>>> 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 ?
>> 
>>> +        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