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

From
Nicholas Piggin <npiggin@gmail.com>
Date
2021-10-27 05:27:30
Also in
lkml
Reply to
Christophe Leroy, 2021-10-27
Excerpts from Christophe Leroy's message of October 27, 2021 2:55 pm:

Le 27/10/2021 à 06:44, Nicholas Piggin a écrit :
quoted
Excerpts from Christophe Leroy's message of October 26, 2021 3:39 pm:
quoted
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()?
Oops, yes, I mean pte_exec()

Unless I have to re-spin, can Michael eventually fix that typo while 
applying ?
quoted
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?
Didn't understand what you mean.

I did it like that to be able to continue using _PAGE_EXEC for checking 
executability regardless of whether this is user or kernel, and then 
continue using the generic nohash pte_exec() helper.

Other solution would be to get rid of _PAGE_EXEC completely for book3e 
and implement both pte_exec() and pte_mkexec() with _PAGE_BAP_UX and 
_PAGE_BAP_SX, but I'm not sure it is worth the churn as you say. It 
would also mean different helpers for book3s/32 when it is using 32 bits 
PTE (CONFIG_PTE_64BIT=n)
That's basically what I mean. And _PAGE_KERNEL_ROX etc would then not 
set the UX bit. But at least for now it seems to be an improvement.

Thanks,
Nick

Thread (12 messages) 12 messages, 4 authors, last reply 2021-11-02
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help