On Wed, May 2, 2018 at 10:17 AM Florian Weimer <fweimer@xxxxxxxxxx> wrote: > On 05/02/2018 07:09 PM, Andy Lutomirski wrote: > >> 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. > > Dave is right: the current behavior was my request, and I still think it’s correct. The whole point is that, even if something nasty happens like a SIGALRM handler hitting in the middle of _dl_fixup, the SIGALRM handler is preventing from accidentally writing to the protected memory. When SIGALRM returns, PKRU should get restored > > > > Another way of looking at this is that the kernel would like to approximate what the ISA behavior*should* have been: the whole sequence “modify PKRU; access memory; restore PKRU” should be as atomic as possible. > > > > Florian, what is the actual problematic sequence of events? > See above. The signal handler will crash if it calls any non-local > function through the GOT because with the default access rights, it's > not readable in the signal handler. > Any use of memory protection keys for basic infrastructure will run into > this problem, so I think the current kernel behavior is not very useful. > It's also x86-specific. > From a security perspective, the atomic behavior is not very useful > because you generally want to modify PKRU *before* computing the details > of the memory access, so that you don't have a general “poke anywhere > with this access right” primitive in the text segment. (I called this > the “suffix problem” in another context.) Ugh, right. It's been long enough that I forgot about the underlying issue. A big part of the problem here is that pkey_alloc() should set the initial value of the key across all threads, but it *can't*. There is literally no way to do it in a multithreaded program that uses RDPKRU and WRPKRU. But I think the right fix, at least for your use case, is to have a per-mm init_pkru variable that starts as "deny all". We'd add a new pkey_alloc() flag like PKEY_ALLOC_UPDATE_INITIAL_STATE that causes the specified mode to update init_pkru. New threads and delivered signals would get the init_pkru state instead of the hardcoded default.
