Re: [PATCH 2/3] powerpc/book3e: Fix set_memory_x() and set_memory_nx()

[Nicholas Piggin]

Excerpts from Christophe Leroy's message of October 26, 2021 3:39 pm:
> set_memory_x() calls pte_mkexec() which sets _PAGE_EXEC.
> set_memory_nx() calls pte_exprotec() which clears _PAGE_EXEC.
> 
> Book3e has 2 bits, UX and SX, which defines the exec rights
> resp. for user (PR=1) and for kernel (PR=0).
> 
> _PAGE_EXEC is defined as UX only.
> 
> An executable kernel page is set with either _PAGE_KERNEL_RWX
> or _PAGE_KERNEL_ROX, which both have SX set and UX cleared.
> 
> So set_memory_nx() call for an executable kernel page does
> nothing because UX is already cleared.
> 
> And set_memory_x() on a non-executable kernel page makes it
> executable for the user and keeps it non-executable for kernel.
> 
> Also, pte_exec() always returns 'false' on kernel pages, because
> it checks _PAGE_EXEC which doesn't include SX, so for instance
> the W+X check doesn't work.
> 
> To fix this:
> - change tlb_low_64e.S to use _PAGE_BAP_UX instead of _PAGE_USER
> - sets both UX and SX in _PAGE_EXEC so that pte_user() returns
> true whenever one of the two bits is set

I don't understand this change. Which pte_user() returns true after
this change? Or do you mean pte_exec()?

Does this filter through in some cases at least for kernel executable
PTEs will get both bits set? Seems cleaner to distinguish user and
kernel exec for that but maybe it's a lot of churn?

Thanks,
Nick

> and pte_exprotect()
> clears both bits.
> - Define a book3e specific version of pte_mkexec() which sets
> either SX or UX based on UR.
> 
> Fixes: 1f9ad21c3b38 ("powerpc/mm: Implement set_memory() routines")
> Signed-off-by: Christophe Leroy <christophe.leroy@xxxxxxxxxx>
> ---
> v3: Removed pte_mkexec() from nohash/64/pgtable.h
> v2: New
> ---