On Wed, Dec 05, 2018 at 08:21:02AM -0800, Andy Lutomirski wrote: > > On Dec 2, 2018, at 8:02 PM, Ram Pai <linuxram@xxxxxxxxxx> wrote: > > > >> On Thu, Nov 29, 2018 at 12:37:15PM +0100, Florian Weimer wrote: > >> * Dave Hansen: > >> > >>>> On 11/27/18 3:57 AM, Florian Weimer wrote: > >>>> I would have expected something that translates PKEY_DISABLE_WRITE | > >>>> PKEY_DISABLE_READ into PKEY_DISABLE_ACCESS, and also accepts > >>>> PKEY_DISABLE_ACCESS | PKEY_DISABLE_READ, for consistency with POWER. > >>>> > >>>> (My understanding is that PKEY_DISABLE_ACCESS does not disable all > >>>> access, but produces execute-only memory.) > >>> > >>> Correct, it disables all data access, but not execution. > >> > >> So I would expect something like this (completely untested, I did not > >> even compile this): > > > > > > Ok. I re-read through the entire email thread to understand the problem and > > the proposed solution. Let me summarize it below. Lets see if we are on the same > > plate. > > > > So the problem is as follows: > > > > Currently the kernel supports 'disable-write' and 'disable-access'. > > > > On x86, cpu supports 'disable-write' and 'disable-access'. This > > matches with what the kernel supports. All good. > > > > However on power, cpu supports 'disable-read' too. Since userspace can > > program the cpu directly, userspace has the ability to set > > 'disable-read' too. This can lead to inconsistency between the kernel > > and the userspace. > > > > We want the kernel to match userspace on all architectures. > > > > Proposed Solution: > > > > Enhance the kernel to understand 'disable-read', and facilitate architectures > > that understand 'disable-read' to allow it. > > > > Also explicitly define the semantics of disable-access as > > 'disable-read and disable-write' > > > > Did I get this right? Assuming I did, the implementation has to do > > the following -- > > > > On power, sys_pkey_alloc() should succeed if the init_val > > is PKEY_DISABLE_READ, PKEY_DISABLE_WRITE, PKEY_DISABLE_ACCESS > > or any combination of the three. > > > > On x86, sys_pkey_alloc() should succeed if the init_val is > > PKEY_DISABLE_WRITE or PKEY_DISABLE_ACCESS or PKEY_DISABLE_READ > > or any combination of the three, except PKEY_DISABLE_READ > > specified all by itself. > > > > On all other arches, none of the flags are supported. > > I don’t really love having a situation where you can use different > flag combinations to refer to the same mode. true. But it is a side-effect of x86 cpu implicitly defining 'disable-access' as a combination of 'disable-read' and 'disable_write'. In other words, if you disable-access on a pte on x86, you are automatically disabling read and disabling write on that page. The software/kernel just happens to explicitly capture that implicit behavior. > > Also, we should document the effect these flags have on execute permission. Actually none of the above flags, interact with execute permission. They operate independently; both on x86 and on POWER. But yes, this statement needs to be documented somewhere. RP