From 054236d322f564ef4cb6f05c4fa9a632ced7d23f Mon Sep 17 00:00:00 2001 From: akasko Date: Thu, 27 Feb 2020 05:37:26 +0000 Subject: [PATCH] 8216472: (se) Stack overflow during selection operation leads to crash (win) Reviewed-by: andrew --- .../sun/nio/ch/WindowsSelectorImpl.java | 33 +++++- .../native/sun/nio/ch/WindowsSelectorImpl.c | 101 +++++++++--------- .../channels/Selector/StackOverflowTest.java | 49 +++++++++ 3 files changed, 131 insertions(+), 52 deletions(-) create mode 100644 test/java/nio/channels/Selector/StackOverflowTest.java diff --git a/src/windows/classes/sun/nio/ch/WindowsSelectorImpl.java b/src/windows/classes/sun/nio/ch/WindowsSelectorImpl.java index eec35ea95..53f290225 100644 --- a/src/windows/classes/sun/nio/ch/WindowsSelectorImpl.java +++ b/src/windows/classes/sun/nio/ch/WindowsSelectorImpl.java @@ -40,6 +40,7 @@ import java.util.List; import java.util.ArrayList; import java.util.HashMap; import java.util.Iterator; +import sun.misc.Unsafe; /** * A multi-threaded implementation of Selector for Windows. @@ -49,12 +50,26 @@ import java.util.Iterator; */ final class WindowsSelectorImpl extends SelectorImpl { + private static final Unsafe unsafe = Unsafe.getUnsafe(); + private static int addressSize = unsafe.addressSize(); + + private static int dependsArch(int value32, int value64) { + return (addressSize == 4) ? value32 : value64; + } + // Initial capacity of the poll array private final int INIT_CAP = 8; // Maximum number of sockets for select(). // Should be INIT_CAP times a power of 2 private final static int MAX_SELECTABLE_FDS = 1024; + // Size of FD_SET struct to allocate a buffer for it in SubSelector, + // aligned to 8 bytes on 64-bit: + // struct { unsigned int fd_count; SOCKET fd_array[MAX_SELECTABLE_FDS]; }. + private static final long SIZEOF_FD_SET = dependsArch( + 4 + MAX_SELECTABLE_FDS * 4, // SOCKET = unsigned int + 4 + MAX_SELECTABLE_FDS * 8 + 4); // SOCKET = unsigned __int64 + // The list of SelectableChannels serviced by this Selector. Every mod // MAX_SELECTABLE_FDS entry is bogus, to align this array with the poll // array, where the corresponding entry is occupied by the wakeupSocket @@ -283,6 +298,9 @@ final class WindowsSelectorImpl extends SelectorImpl { private final int[] readFds = new int [MAX_SELECTABLE_FDS + 1]; private final int[] writeFds = new int [MAX_SELECTABLE_FDS + 1]; private final int[] exceptFds = new int [MAX_SELECTABLE_FDS + 1]; + // Buffer for readfds, writefds and exceptfds structs that are passed + // to native select(). + private final long fdsBuffer = unsafe.allocateMemory(SIZEOF_FD_SET * 6); private SubSelector() { this.pollArrayIndex = 0; // main thread @@ -295,7 +313,7 @@ final class WindowsSelectorImpl extends SelectorImpl { private int poll() throws IOException{ // poll for the main thread return poll0(pollWrapper.pollArrayAddress, Math.min(totalChannels, MAX_SELECTABLE_FDS), - readFds, writeFds, exceptFds, timeout); + readFds, writeFds, exceptFds, timeout, fdsBuffer); } private int poll(int index) throws IOException { @@ -304,11 +322,11 @@ final class WindowsSelectorImpl extends SelectorImpl { (pollArrayIndex * PollArrayWrapper.SIZE_POLLFD), Math.min(MAX_SELECTABLE_FDS, totalChannels - (index + 1) * MAX_SELECTABLE_FDS), - readFds, writeFds, exceptFds, timeout); + readFds, writeFds, exceptFds, timeout, fdsBuffer); } private native int poll0(long pollAddress, int numfds, - int[] readFds, int[] writeFds, int[] exceptFds, long timeout); + int[] readFds, int[] writeFds, int[] exceptFds, long timeout, long fdsBuffer); private int processSelectedKeys(long updateCount) { int numKeysUpdated = 0; @@ -400,6 +418,10 @@ final class WindowsSelectorImpl extends SelectorImpl { } return numKeysUpdated; } + + private void freeFDSetBuffer() { + unsafe.freeMemory(fdsBuffer); + } } // Represents a helper thread used for select. @@ -425,8 +447,10 @@ final class WindowsSelectorImpl extends SelectorImpl { while (true) { // poll loop // wait for the start of poll. If this thread has become // redundant, then exit. - if (startLock.waitForStart(this)) + if (startLock.waitForStart(this)) { + subSelector.freeFDSetBuffer(); return; + } // call poll() try { subSelector.poll(index); @@ -525,6 +549,7 @@ final class WindowsSelectorImpl extends SelectorImpl { for (SelectThread t: threads) t.makeZombie(); startLock.startThreads(); + subSelector.freeFDSetBuffer(); } } } diff --git a/src/windows/native/sun/nio/ch/WindowsSelectorImpl.c b/src/windows/native/sun/nio/ch/WindowsSelectorImpl.c index c43496a59..633584906 100644 --- a/src/windows/native/sun/nio/ch/WindowsSelectorImpl.c +++ b/src/windows/native/sun/nio/ch/WindowsSelectorImpl.c @@ -38,6 +38,7 @@ #include "jvm.h" #include "jni.h" #include "jni_util.h" +#include "nio.h" #include "sun_nio_ch_WindowsSelectorImpl.h" #include "sun_nio_ch_PollArrayWrapper.h" @@ -55,12 +56,14 @@ JNIEXPORT jint JNICALL Java_sun_nio_ch_WindowsSelectorImpl_00024SubSelector_poll0(JNIEnv *env, jobject this, jlong pollAddress, jint numfds, jintArray returnReadFds, jintArray returnWriteFds, - jintArray returnExceptFds, jlong timeout) + jintArray returnExceptFds, jlong timeout, jlong fdsBuffer) { DWORD result = 0; pollfd *fds = (pollfd *) pollAddress; int i; - FD_SET readfds, writefds, exceptfds; + FD_SET *readfds = (FD_SET *) jlong_to_ptr(fdsBuffer); + FD_SET *writefds = (FD_SET *) jlong_to_ptr(fdsBuffer + sizeof(FD_SET)); + FD_SET *exceptfds = (FD_SET *) jlong_to_ptr(fdsBuffer + sizeof(FD_SET) * 2); struct timeval timevalue, *tv; static struct timeval zerotime = {0, 0}; int read_count = 0, write_count = 0, except_count = 0; @@ -82,66 +85,68 @@ Java_sun_nio_ch_WindowsSelectorImpl_00024SubSelector_poll0(JNIEnv *env, jobject /* Set FD_SET structures required for select */ for (i = 0; i < numfds; i++) { if (fds[i].events & POLLIN) { - readfds.fd_array[read_count] = fds[i].fd; + readfds->fd_array[read_count] = fds[i].fd; read_count++; } if (fds[i].events & (POLLOUT | POLLCONN)) { - writefds.fd_array[write_count] = fds[i].fd; + writefds->fd_array[write_count] = fds[i].fd; write_count++; } - exceptfds.fd_array[except_count] = fds[i].fd; + exceptfds->fd_array[except_count] = fds[i].fd; except_count++; } - readfds.fd_count = read_count; - writefds.fd_count = write_count; - exceptfds.fd_count = except_count; + readfds->fd_count = read_count; + writefds->fd_count = write_count; + exceptfds->fd_count = except_count; /* Call select */ - if ((result = select(0 , &readfds, &writefds, &exceptfds, tv)) + if ((result = select(0 , readfds, writefds, exceptfds, tv)) == SOCKET_ERROR) { /* Bad error - this should not happen frequently */ /* Iterate over sockets and call select() on each separately */ - FD_SET errreadfds, errwritefds, errexceptfds; - readfds.fd_count = 0; - writefds.fd_count = 0; - exceptfds.fd_count = 0; + FD_SET *errreadfds = (FD_SET *) jlong_to_ptr(fdsBuffer + sizeof(FD_SET) * 3); + FD_SET *errwritefds = (FD_SET *) jlong_to_ptr(fdsBuffer + sizeof(FD_SET) * 4); + FD_SET *errexceptfds = (FD_SET *) jlong_to_ptr(fdsBuffer + sizeof(FD_SET) * 5); + readfds->fd_count = 0; + writefds->fd_count = 0; + exceptfds->fd_count = 0; for (i = 0; i < numfds; i++) { /* prepare select structures for the i-th socket */ - errreadfds.fd_count = 0; - errwritefds.fd_count = 0; + errreadfds->fd_count = 0; + errwritefds->fd_count = 0; if (fds[i].events & POLLIN) { - errreadfds.fd_array[0] = fds[i].fd; - errreadfds.fd_count = 1; + errreadfds->fd_array[0] = fds[i].fd; + errreadfds->fd_count = 1; } if (fds[i].events & (POLLOUT | POLLCONN)) { - errwritefds.fd_array[0] = fds[i].fd; - errwritefds.fd_count = 1; + errwritefds->fd_array[0] = fds[i].fd; + errwritefds->fd_count = 1; } - errexceptfds.fd_array[0] = fds[i].fd; - errexceptfds.fd_count = 1; + errexceptfds->fd_array[0] = fds[i].fd; + errexceptfds->fd_count = 1; /* call select on the i-th socket */ - if (select(0, &errreadfds, &errwritefds, &errexceptfds, &zerotime) + if (select(0, errreadfds, errwritefds, errexceptfds, &zerotime) == SOCKET_ERROR) { /* This socket causes an error. Add it to exceptfds set */ - exceptfds.fd_array[exceptfds.fd_count] = fds[i].fd; - exceptfds.fd_count++; + exceptfds->fd_array[exceptfds->fd_count] = fds[i].fd; + exceptfds->fd_count++; } else { /* This socket does not cause an error. Process result */ - if (errreadfds.fd_count == 1) { - readfds.fd_array[readfds.fd_count] = fds[i].fd; - readfds.fd_count++; + if (errreadfds->fd_count == 1) { + readfds->fd_array[readfds->fd_count] = fds[i].fd; + readfds->fd_count++; } - if (errwritefds.fd_count == 1) { - writefds.fd_array[writefds.fd_count] = fds[i].fd; - writefds.fd_count++; + if (errwritefds->fd_count == 1) { + writefds->fd_array[writefds->fd_count] = fds[i].fd; + writefds->fd_count++; } - if (errexceptfds.fd_count == 1) { - exceptfds.fd_array[exceptfds.fd_count] = fds[i].fd; - exceptfds.fd_count++; + if (errexceptfds->fd_count == 1) { + exceptfds->fd_array[exceptfds->fd_count] = fds[i].fd; + exceptfds->fd_count++; } } } @@ -151,34 +156,34 @@ Java_sun_nio_ch_WindowsSelectorImpl_00024SubSelector_poll0(JNIEnv *env, jobject /* Each Java array consists of sockets count followed by sockets list */ #ifdef _WIN64 - resultbuf[0] = readfds.fd_count; - for (i = 0; i < (int)readfds.fd_count; i++) { - resultbuf[i + 1] = (int)readfds.fd_array[i]; + resultbuf[0] = readfds->fd_count; + for (i = 0; i < (int)readfds->fd_count; i++) { + resultbuf[i + 1] = (int)readfds->fd_array[i]; } (*env)->SetIntArrayRegion(env, returnReadFds, 0, - readfds.fd_count + 1, resultbuf); + readfds->fd_count + 1, resultbuf); - resultbuf[0] = writefds.fd_count; - for (i = 0; i < (int)writefds.fd_count; i++) { - resultbuf[i + 1] = (int)writefds.fd_array[i]; + resultbuf[0] = writefds->fd_count; + for (i = 0; i < (int)writefds->fd_count; i++) { + resultbuf[i + 1] = (int)writefds->fd_array[i]; } (*env)->SetIntArrayRegion(env, returnWriteFds, 0, - writefds.fd_count + 1, resultbuf); + writefds->fd_count + 1, resultbuf); - resultbuf[0] = exceptfds.fd_count; - for (i = 0; i < (int)exceptfds.fd_count; i++) { - resultbuf[i + 1] = (int)exceptfds.fd_array[i]; + resultbuf[0] = exceptfds->fd_count; + for (i = 0; i < (int)exceptfds->fd_count; i++) { + resultbuf[i + 1] = (int)exceptfds->fd_array[i]; } (*env)->SetIntArrayRegion(env, returnExceptFds, 0, - exceptfds.fd_count + 1, resultbuf); + exceptfds->fd_count + 1, resultbuf); #else (*env)->SetIntArrayRegion(env, returnReadFds, 0, - readfds.fd_count + 1, (jint *)&readfds); + readfds->fd_count + 1, (jint *)readfds); (*env)->SetIntArrayRegion(env, returnWriteFds, 0, - writefds.fd_count + 1, (jint *)&writefds); + writefds->fd_count + 1, (jint *)writefds); (*env)->SetIntArrayRegion(env, returnExceptFds, 0, - exceptfds.fd_count + 1, (jint *)&exceptfds); + exceptfds->fd_count + 1, (jint *)exceptfds); #endif return 0; } diff --git a/test/java/nio/channels/Selector/StackOverflowTest.java b/test/java/nio/channels/Selector/StackOverflowTest.java new file mode 100644 index 000000000..9ca0a67b5 --- /dev/null +++ b/test/java/nio/channels/Selector/StackOverflowTest.java @@ -0,0 +1,49 @@ +/* + * Copyright (c) 2019, Red Hat, Inc. and/or its affiliates. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +/** + * @test + * @bug 8216472 + * @summary native call in WindowsSelectorImpl.SubSelector.poll can use + * more stack space than available in a shadow zone, this can cause + * a crash if selector is called from a deep recursive java call + * @requires (os.family == "windows") + */ + +import java.nio.channels.Selector; + +public class StackOverflowTest { + + public static void main(String[] args) throws Exception { + try (Selector sel = Selector.open()) { + recursiveSelect(sel); + } catch (StackOverflowError e) { + // ignore SOE from java calls + } + } + + static void recursiveSelect(Selector sel) throws Exception { + sel.selectNow(); + recursiveSelect(sel); + } +} -- GitLab