On 05/02/2018 08:12 AM, Florian Weimer wrote: > On 05/02/2018 04:30 PM, Dave Hansen wrote: >> On 05/02/2018 06:26 AM, Florian Weimer wrote: >>> pkeys support for IBM POWER intends to inherited the access rights of >>> the current thread in signal handlers. The advantage is that this >>> preserves access to memory regions associated with non-default keys, >>> enabling additional usage scenarios for memory protection keys which >>> currently do not work on x86 due to the unconditional reset to the >>> (configurable) default key in signal handlers. >> >> What's the usage scenario that does not work? > > Here's what I want to do: > > Nick Clifton wrote a binutils patch which puts the .got.plt section on > separate pages. We allocate a protection key for it, assign it to all > such sections in the process image, and change the access rights of the > main thread to disallow writes via that key during process startup. In > _dl_fixup, we enable write access to the GOT, update the GOT entry, and > then disable it again. > > This way, we have a pretty safe form of lazy binding, without having to > resort to BIND_NOW. > > With the current kernel behavior on x86, we cannot do that because > signal handlers revert to the default (deny) access rights, so the GOT > turns inaccessible. cc'ing Andy Lutomirksi... He was the one that specifically asked for the deny-by-default behavior that I think is biting you. The behavior we had before implementing Andy's suggestion was, I think, the one you would want: we had allow-all as the default PKRU value. The other option here that I think we discussed in the past was to have an *explicit* signal PKRU value. That way, we can be restrictive by default but allow overrides for special cases like you have. >>> Consequently, this commit updates the x86 implementation to preserve >>> the PKRU register value of the interrupted context in signal handlers. >>> If a key is allocated successfully with the PKEY_ALLOC_SIGNALINHERIT >>> flag, the application can assume this signal inheritance behavior. >> >> I think this is a pretty gross misuse of the API. Adding an argument to >> pkey_alloc() is something that folks would assume would impact the key >> being *allocated*, not pkeys behavior across the process as a whole. > > From the application point of view, only the allocated key is > affected—it has specific semantics that were undefined before and varied > between x86 and POWER. I'd really like to see it in a separate API. >>> This change does not affect the init_pkru optimization because if the >>> thread's PKRU register is zero due to the init_pkru setting, it will >>> remain zero in the signal handler through inheritance from the >>> interrupted context. >> >> I think you are right, but it's rather convoluted. It does: >> >> 1. Running with PKRU in the init state >> 2. Kernel saves off init-state-PKRU XSAVE signal buffer >> 3. Enter signal, kernel XRSTOR (may) set the init state again >> 4. fpu__clear() does __write_pkru(), takes it out of the init state >> 5. Signal handler runs, exits >> 6. fpu__restore_sig() XRSTOR's the state from #2, taking PKRU back to >> the init state > > Isn't that just the cost of not hard-coding the XSAVE area layout? No. You unnecessarily take PKRU out of the init state. If you don't want to change its value, just don't do that when you run XRSTOR. >> But, about the patch in general: >> >> I'm not a big fan of doing this in such a PKRU-specific way. It would >> be nice to have this available for all XSAVE states. It would also keep >> you from so unnecessarily frobbing with WRPKRU in fpu__clear(). You >> could just clear the PKRU bit in the Requested Feature BitMap (RFBM) >> passed to XRSTOR. That would be much straightforward and able to be >> more easily extended to more states. > > I don't see where I could plug this into the current kernel sources. > Would you please provide some pointers? In handle_signal()->fpu__clear()->...__copy_kernel_to_fpregs(), we pass -1 as the RFBM. Don't do that. Pass (~0 & ~XFEATURE_MASK_PKRU). -- To unsubscribe from this list: send the line "unsubscribe linux-x86_64" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
