Stack guard pages
Andrew Haley
aph at redhat.com
Tue Feb 23 11:50:15 PST 2010
On 02/23/2010 07:15 PM, Andrew John Hughes wrote:
> On 23 February 2010 16:16, Andrew Haley <aph at redhat.com> wrote:
>> I've been working on a bug in OpenOffice where using Java causes a
>> later crash. https://bugzilla.novell.com/show_bug.cgi?id=578802
>>
>> It's a very strange bug: Java is called, does something, and then the
>> thread is detached from the Java VM. Long after, a segfault occurs.
>>
>> The reason for this crash is the way that stack guard pages work.
>> When DetachCurrentThread() calls
>> JavaThread::remove_stack_guard_pages(), the guard pages are not
>> removed. So, if at some point later in the process the stack grows
>> beyond the point where the guard pages were mapped, the Linux kernel
>> cannot expand the stack any further because the guard pages are in the
>> way, and a segfault occurs.
>>
>> I've attached a reproducer for this bug to the end of this message.
>> It crashes on many of the Linux systems I've tried.
>>
>> The right way to fix this is for remove_stack_guard_pages() to
>> munmap() the guard pages. However, it's essential not to split the
>> stack region, so if the stack has already grown beyond the guard
>> pages, we have to unmap() it. Like so:
>>
>> bool os::create_stack_guard_pages(char* addr, size_t size) {
>> uintptr_t stack_extent, stack_base;
>> // get_stack_bounds() returns the memory extent mapped for the stack
>> // by the kernel.
>> if (get_stack_bounds(&stack_extent, &stack_base)) {
>> if (stack_extent < (uintptr_t)addr)
>> ::munmap((void*)stack_extent, (uintptr_t)addr - stack_extent);
>> }
>>
>> return os::commit_memory(addr, size);
>> }
>>
>> bool os::remove_stack_guard_pages(char* addr, size_t size) {
>> return ::munmap(addr, size) == 0;
>> }
>>
>> This fixes the bug. Unfortunately, there is no os:: interface for
>> create_stack_guard_pages() and remove_stack_guard_pages(), so this
>> solution doesn't fit well with the current organization of the VM. I
>> can't see any way of making this work only by touching os_linux.?pp .
>>
>> I could simply patch OpenJDK locally for the Linux distros, but I
>> think we should have this discussion first.
>
> Segfaults here with the latest OpenJDK7 b84:
>
> $ /mnt/builder/jdk7/j2sdk-image/bin/java -version
> openjdk version "1.7.0-internal"
> OpenJDK Runtime Environment (build 1.7.0-internal-andrew_2010_02_23_18_45-b00)
> OpenJDK 64-Bit Server VM (build 17.0-b09, mixed mode)
>
> $ LD_LIBRARY_PATH=/mnt/builder/jdk7/j2sdk-image/jre/lib/amd64/server ./invoke
> Hello
> Segmentation fault
>
> I think it would it would be clearer what the suggested fix entails if
> you posted a diff or webrev of the suggested change.
Maybe, but I don't ATM have a suggested change. Everything I've tried is
rather intrusive, and potentially affects other targets.
However, here's what I've been testing, for information only.
Andrew.
--- ./src/share/vm/runtime/thread.cpp~ 2008-07-10 21:04:36.000000000 +0100
+++ ./src/share/vm/runtime/thread.cpp 2010-02-23 14:43:20.452135932 +0000
@@ -2075,7 +2075,7 @@
int allocate = os::allocate_stack_guard_pages();
// warning("Guarding at " PTR_FORMAT " for len " SIZE_FORMAT "\n", low_addr, len);
- if (allocate && !os::commit_memory((char *) low_addr, len)) {
+ if (allocate && !os::create_stack_guard_pages((char *) low_addr, len)) {
warning("Attempt to allocate stack guard pages failed.");
return;
}
@@ -2096,7 +2096,7 @@
size_t len = (StackYellowPages + StackRedPages) * os::vm_page_size();
if (os::allocate_stack_guard_pages()) {
- if (os::uncommit_memory((char *) low_addr, len)) {
+ if (os::remove_stack_guard_pages((char *) low_addr, len)) {
_stack_guard_state = stack_guard_unused;
} else {
warning("Attempt to deallocate stack guard pages failed.");
--- ./src/share/vm/runtime/os.hpp~ 2008-07-10 21:04:36.000000000 +0100
+++ ./src/share/vm/runtime/os.hpp 2010-02-23 14:49:27.512732036 +0000
@@ -168,6 +168,9 @@
static bool protect_memory(char* addr, size_t bytes);
static bool guard_memory(char* addr, size_t bytes);
static bool unguard_memory(char* addr, size_t bytes);
+ static bool create_stack_guard_pages(char* addr, size_t bytes);
+ static bool remove_stack_guard_pages(char* addr, size_t bytes);
+
static char* map_memory(int fd, const char* file_name, size_t file_offset,
char *addr, size_t bytes, bool read_only = false,
bool allow_exec = false);
--- ./src/os/linux/vm/os_linux.cpp~ 2010-02-23 11:42:40.005073691 +0000
+++ ./src/os/linux/vm/os_linux.cpp 2010-02-23 15:44:47.112387815 +0000
@@ -57,6 +57,10 @@
# include <sys/shm.h>
# include <link.h>
+#include <sstream>
+#include <iostream>
+#include <fstream>
+
#define MAX_PATH (2 * K)
// for timer info max values which include all bits
@@ -2292,6 +2296,53 @@
!= MAP_FAILED;
}
+static bool
+get_stack_bounds(uintptr_t *bottom, uintptr_t *top)
+{
+ using namespace std;
+
+ ostringstream oss;
+ oss << "/proc/" << syscall(SYS_gettid) << "/maps";
+ ifstream cin(oss.str().c_str());
+ while (!cin.eof())
+ {
+ string str;
+ getline(cin,str);
+ const string stack_str = "[stack]";
+ if (str.length() > stack_str.length()
+ && (str.compare(str.length() - stack_str.length(),
+ stack_str.length(), stack_str)
+ == 0))
+ {
+ istringstream iss(str);
+ iss.flags(ios::hex);
+ iss >> *bottom;
+ char c;
+ iss >> c;
+ iss >> *top;
+ uintptr_t sp = (intptr_t)__builtin_frame_address(0);
+ if (sp >= *bottom && sp <= *top)
+ return true;
+ }
+ }
+
+ return false;
+}
+
+bool os::create_stack_guard_pages(char* addr, size_t size) {
+ uintptr_t stack_extent, stack_base;
+ if (get_stack_bounds(&stack_extent, &stack_base)) {
+ if (stack_extent < (uintptr_t)addr)
+ ::munmap((void*)stack_extent, (uintptr_t)addr - stack_extent);
+ }
+
+ return os::commit_memory(addr, size);
+}
+
+bool os::remove_stack_guard_pages(char* addr, size_t size) {
+ return ::munmap(addr, size) == 0;
+}
+
static address _highest_vm_reserved_address = NULL;
// If 'fixed' is true, anon_mmap() will attempt to reserve anonymous memory
More information about the hotspot-dev
mailing list