Alternate work around for kernel guard pages

battleblow / openjdk-jdk11u

BSD port of OpenJDK 11

GNU General Public License v2.0

9 stars 8 forks source link

Alternate work around for kernel guard pages #84

Closed battleblow closed 5 years ago

battleblow commented 5 years ago

Don't do the address arithmetic workaround in arch specific code.
Set the pthread guard size attribute on FreeBSD to cover the kernel stack guard pages, if disabling them with procctl didn't work.
Read from the guard pages so they get paged in so that guard_memory will work correctly.

This review is currently for comment only. With these changes in place the JVM crashes during the build. I'm looking at why, but more eyes may be helpful.

battleblow commented 5 years ago

Note that I've put in some trace statements locally and am confident that this is setting the guard size correctly as a pthread attribute. I have some other tasks I'm also working on, but will get back to some more in depth debugging later.

bsdkurt commented 5 years ago

It seems the idea I came up with to fault in the pages before the mprotect only works on 13-current. I tested 11.3-RELEASE-p3 and 12.0-RELEASE-p10 and they both fail on the read of the pages here:

    for (char *pos = addr; pos < addr + size; pos += os::Bsd::page_size()) {
      char c = *pos;
    }

So without a mechanism to get mprotect to work (without first calling mmap, which moves the guard pages into the stack further), I think this approach has to be abandoned.

Sorry for leading you down this path. I assumed it would work the same as I found on 13-current.

bsdkurt commented 5 years ago

Looking at 11 and 12, it appears mprotect on top of MAP_STACK space causes the kernel guard pages to move further into the stack. In 13 it ignores mprotect on MAP_STACK pages that have not yet been accessed. I can't see a way to make this work correctly without the procctl or better yet a proper kernel fix where the kernel guard pages are not within the MAP_STACK space + a working mprotect per posix.

battleblow commented 5 years ago

Thanks Kurt. I was definitely seeing the a crash on 11, which is where I was testing. I hadn't gotten to 12 or 13 yet. This was worth exploring, so no problems there, but it seems like a dead end now that we know any release version of FreeBSD likely to support it will also support the procctl change.