The 07/08/2024 18:53, Catalin Marinas wrote: > Hi Szabolcs, > > On Mon, Jun 17, 2024 at 03:51:35PM +0100, Szabolcs Nagy wrote: > > The 06/17/2024 15:40, Florian Weimer wrote: > > > >> A user can still set it by interacting with the register directly, but I guess > > > >> we want something for the glibc interface.. > > > >> > > > >> Dave, any thoughts here? > > > > > > > > adding Florian too, since i found an old thread of his that tried > > > > to add separate PKEY_DISABLE_READ and PKEY_DISABLE_EXECUTE, but > > > > it did not seem to end up upstream. (this makes more sense to me > > > > as libc api than the weird disable access semantics) > > > > > > I still think it makes sense to have a full complenent of PKEY_* flags > > > complementing the PROT_* flags, in a somewhat abstract fashion for > > > pkey_alloc only. The internal protection mask register encoding will > > > differ from architecture to architecture, but the abstract glibc > > > functions pkey_set and pkey_get could use them (if we are a bit > > > careful). > > > > to me it makes sense to have abstract > > > > PKEY_DISABLE_READ > > PKEY_DISABLE_WRITE > > PKEY_DISABLE_EXECUTE > > PKEY_DISABLE_ACCESS > > > > where access is handled like > > > > if (flags&PKEY_DISABLE_ACCESS) > > flags |= PKEY_DISABLE_READ|PKEY_DISABLE_WRITE; > > disable_read = flags&PKEY_DISABLE_READ; > > disable_write = flags&PKEY_DISABLE_WRITE; > > disable_exec = flags&PKEY_DISABLE_EXECUTE; > > > > if there are unsupported combinations like > > disable_read&&!disable_write then those are rejected > > by pkey_alloc and pkey_set. > > > > this allows portable use of pkey apis. > > (the flags could be target specific, but don't have to be) > > On powerpc, PKEY_DISABLE_ACCESS also disables execution. AFAICT, the > kernel doesn't define a PKEY_DISABLE_READ, only PKEY_DISABLE_ACCESS so > for powerpc there's no way to to set an execute-only permission via this > interface. I wouldn't like to diverge from powerpc. the exec permission should be documented in the man. and i think it should be consistent across targets to allow portable use. now ppc and x86 are inconsistent, i think it's not ideal, but ok to say that targets without disable-exec support do whatever x86 does with PKEY_DISABLE_ACCESS otherwise it means whatever ppc does. > > However, does it matter much? That's only for the initial setup, the > user can then change the permissions directly via the sysreg. So maybe > we don't need all those combinations upfront. A PKEY_DISABLE_EXECUTE > together with the full PKEY_DISABLE_ACCESS would probably suffice. this is ok. a bit awkward in userspace when the register is directly set to e.g write-only and pkey_get has to return something, but we can handle settings outside of valid PKEY_* macros as unspec, users who want that would use their own register set/get code. i would have designed the permission to use either existing PROT_* flags or say that it is architectural and written to the register directly and let the libc wrapper deal with portable api, i guess it's too late now. (the signal handling behaviour should have a control and it is possible to fix e.g. via pkey_alloc flags, but that may not be the best solution and this can be done later.) > > Give that on x86 the PKEY_ACCESS_MASK will have to stay as > PKEY_DISABLE_ACCESS|PKEY_DISABLE_WRITE, we'll probably do the same as > powerpc and define an arm64 specific PKEY_DISABLE_EXECUTE with the > corresponding PKEY_ACCESS_MASK including it. We can generalise the masks > with some ARCH_HAS_PKEY_DISABLE_EXECUTE but it's probably more hassle > than just defining the arm64 PKEY_DISABLE_EXECUTE. > > I assume you'd like PKEY_DISABLE_EXECUTE to be part of this series, > otherwise changing PKEY_ACCESS_MASK later will cause potential ABI > issues. yes i think we should figure this out in the initial support.
