[RFC] aarch64: Add support for memory protection keys

[Yury Khrustalev]

Hi Florian,

On Wed, Jun 12, 2024 at 05:04:57PM +0200, Florian Weimer wrote:
> * yury khrustalev:
> 
> > +  /* Only the following mapping between POR permission bits (4 bits)
> > +     and PKEY flags is guaranteed:
> > +
> > +       -WXR
> > +       0010       => PKEY_DISABLE_ACCESS (execute only)
> > +       0011       => PKEY_DISABLE_WRITE (read-execute)
> > +       0111       => 0 (no restrictions, read-write-execute)
> > +       otherwise  => undefined behavior
> > +
> > +     Note that pkey_set and pkey_alloc would only set these specific
> > +     values.  */
> > +
> > +  if (perm == S1POE_PERM_X)
> > +    return PKEY_DISABLE_ACCESS;
> > +  if (perm == S1POE_PERM_RX)
> > +    return PKEY_DISABLE_WRITE;
> > +  if (perm == S1POE_PERM_RWX)
> > +    return 0; // no restrictions
> > +  return PKEY_DISABLE_ACCESS; // undefined behavior
> 
> POWER has the same translation issue.  I suggest to get additional
> constants defined by the kernel, whether it's PKEY_DISABLE_READ and
> PKEY_DISABLE_EXEC or the full set of PKEY_ENABLE_* constant may not
> matter.
> 
> PKEY_DISABLE_ACCESS really should have been PKEY_DISABLE_READ |
> PKEY_DISABLE_WRITE, but unfortunately that's not how the x86 hardware
> works.

I agree with this. There is a separate discussion about this in:
https://lore.kernel.org/linux-arm-kernel/ZlnlQ%2FavUAuSum5R@arm.com/

> There are also some concerns that pkey_set/pkey_get are impractical as
> an interface because of the function call overhead, but that's a
> separate matter.
> 
> > +  /* Check that unallocated pkey is not accepted by the
> > +     pkey_mprotect function.  */
> > +  {
> > +    int pkey = -1;
> > +    pkey = pkey_alloc (0, PKEY_DISABLE_WRITE);
> > +    pkey_free (pkey);
> > +    int *page = xmmap (NULL, pagesize, PROT_NONE,
> > +                       MAP_ANONYMOUS | MAP_PRIVATE, -1);
> > +    TEST_COMPARE (pkey_mprotect (page, pagesize, PROT_READ, pkey), -1);
> > +    TEST_COMPARE (errno, EINVAL);
> > +    xmunmap (page, pagesize);
> > +  }
> 
> Is this really something that the kernel API checks?

Yes, at least this is what documented for errno set by pkey_mprotect
when the pkey used has not been allocated.

> When you tested this on x86_64-linux-gnu, did you run it on a machine
> with protection key hardware/kernel support?

Yes, the misc/tst-pkey test PASS-ed rather than UNSUPPORTED and cpuinfo
on this machine includes "pku".

> Thanks,
> Florian

Kind regards,
Yury