On 11/07/2017 08:59 PM, Adhemerval Zanella wrote:
>> The compile-time check is there to prevent the accidental use of the
>> macro with 128-bit integer types.
> 
> Ok, would you mind adding this comment on macro implementation?

It's included in the separate patch I posted.

> Reading powerpc patchset it seems to me that it will be worth add
> some generalization on arch-pkey.h and pkey_{set,get}
> 
>    - Based on current patchset, powerpc seems to define different
>      value for PKEY_DISABLE_ACCESS, so I think it would be worth
>      to parametrize by moving it to arch-defined header.
> 
>    - What about adding this on arch-pkey.h

I'm still against this premature generalization.  We can revisit this 
topic once POWER support lands in the kernel.  s390 has hardware support 
as well, and if there are three architectures with this features, a 
pattern hopefully emerges.

> My understanding from documentation it key 0 is default one and it
> is assigned to any memory regions (through mmap) which has not been
> explicitly assigned through pkey_mprotect. My questioning whether we
> should filter out key 0 is to:
> 
>    1. Issue an error when application tries to use a key which has
>       not been previously allocated by pkey_alloc (since key 0 is
>       pre-allocated by the kernel).
> 
>    2. Avoid changing default masking bits for pages which has not
>       been explicitly assigned through a pkey_mprotect.
> 
> However this does not prevent uses from issuing a RDPKRU/WRPKRU
> directly.

There is no way to probe allocated keys, so I don't think we can 
implement more extensive checking without further kernel interfaces.

I'm attaching an updated patch with a first stab at documentation.  The 
documentation is at odds with the manual pages, but this is because what 
I see in tst-pkey does not match the behavior described in the manual page.

I changed the implementation of pkey_mprotect to use plain mprotect for 
-1 keys, so that application usage is simplified.

Thanks,
Florian