Messages in this thread | | | Date | Tue, 23 Aug 2022 11:12:24 -0700 | Subject | Re: PKU usage improvements for threads | From | Dave Hansen <> |
| |
On 8/23/22 04:08, Stephen Röttger wrote: > On Mon, Aug 22, 2022 at 11:11 PM Dave Hansen <dave.hansen@intel.com> wrote: >> On 8/22/22 13:40, Kees Cook wrote: >>> 1) It appears to be a bug that a thread without the correct PK can make >>> VMAs covered by a separate PK, out from under other threads. (e.g. mmap >>> a new mapping to wipe out the defined PK for it.) It seems that PK checks >>> should be made when modifying VMAs. >> >> Could you give an example of this? Is this something along the lines of >> a mmap(MAP_FIXED) wiping out an earlier mapping? Or, is it more subtle >> than that? > > Yes, that's one example. And the same applies to other operations on the > VMA. E.g. another case we'd like to prevent would be munmap(addr) where > addr is covered by a pkey to which the calling thread doesn't have access > permissions to.
Yeah, that's something for which our defenses are quite weak. But, it also calls for a very generic mm/ solution and not something specific at all to pkeys.
I assume that you wouldn't want to turn off *all* mmap(), MAP_FIXED or munmap() in the process. You just want to make one or more VMAs more or less immutable. That _sounds_ like a topic that would have broached at some point in the past, although it doesn't ring any bells.
The concept would make a good lightning talk at Plumbers of LSF/MM.
>>> 2) It would be very helpful to have a mechanism for the signal stack to >>> be PK aware, in the sense that the kernel would switch to a predefined >>> PK. i.e. having a new interface to sigaltstack() which includes a PK. >> >> Are you thinking that when switching to the sigaltstack that it would >> also pick up a specific PKRU value? Or, that it would ensure that PKRU >> allows access to the sigaltstack's pkey? > > Either of those would work for us. > >> Logically something like this: >> >> stack_t sas = { >> ss_sp = stack_ptr; >> ss_flags = ... flags; >> ss_size = ...; >> ss_pkey = 12; >> }; >> >> Then the kernel would set up RSP to point to ss_sp, and do (logically): >> >> pkkru &= ~(3<<(12*2)); // clear Write and Access-disable for pkey-12 >> >> before building the signal frame running the signal handler? > > Yeah, that would work for our use case. > We also have a doc discussing this in more detail :) :
That doesn't seem like it would be too much of a stretch. There's a delicate point when building the stack frame that the kernel would need to move over to the new PKRU value to build the frame before it writes the *OLD* value to the frame. But, it's far from impossible.
I also bet we could do this with minimal new ABI. There's already a ->ss_flags field. We could assign a flag to mean that stack_t doesn't end at '->ss_size' and that there's a pkey value *after* ss_size. I do think having a single pkey that is made accessible before signal entry is a more flexible ABI than taking an explicit PKRU value.
I think that would allow just reusing sys_sigaltstack().
| |