8216472: (se) Stack overflow during selection operation leads to crash (win)
Reviewed-by: alanb
Contributed-by: akashche@redhat.com
--- a/src/java.base/windows/classes/sun/nio/ch/WindowsSelectorImpl.java Fri Nov 08 18:35:33 2019 -0800
+++ b/src/java.base/windows/classes/sun/nio/ch/WindowsSelectorImpl.java Sat Nov 09 09:13:04 2019 +0000
@@ -38,6 +38,7 @@
import java.util.List;
import java.util.Map;
import java.util.function.Consumer;
+import jdk.internal.misc.Unsafe;
/**
* A multi-threaded implementation of Selector for Windows.
@@ -47,12 +48,26 @@
*/
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 static final 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
@@ -326,6 +341,9 @@
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 * 3);
private SubSelector() {
this.pollArrayIndex = 0; // main thread
@@ -338,7 +356,7 @@
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 {
@@ -347,11 +365,11 @@
(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, Consumer<SelectionKey> action) {
int numKeysUpdated = 0;
@@ -415,6 +433,10 @@
}
return numKeysUpdated;
}
+
+ private void freeFDSetBuffer() {
+ unsafe.freeMemory(fdsBuffer);
+ }
}
// Represents a helper thread used for select.
@@ -441,8 +463,10 @@
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);
@@ -533,6 +557,7 @@
for (SelectThread t: threads)
t.makeZombie();
startLock.startThreads();
+ subSelector.freeFDSetBuffer();
}
@Override
--- a/src/java.base/windows/native/libnio/ch/WindowsSelectorImpl.c Fri Nov 08 18:35:33 2019 -0800
+++ b/src/java.base/windows/native/libnio/ch/WindowsSelectorImpl.c Sat Nov 09 09:13:04 2019 +0000
@@ -39,6 +39,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"
@@ -56,12 +57,14 @@
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;
@@ -93,103 +96,61 @@
/* 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;
- for (i = 0; i < numfds; i++) {
- /* prepare select structures for the i-th socket */
- errreadfds.fd_count = 0;
- errwritefds.fd_count = 0;
- if (fds[i].events & POLLIN) {
- 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;
- }
- 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)
- == SOCKET_ERROR) {
- /* This socket causes an error. Add it to exceptfds set */
- 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 (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++;
- }
- }
- }
+ JNU_ThrowIOExceptionWithLastError(env, "Select failed");
+ return IOS_THROWN;
}
/* Return selected sockets. */
/* 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;
}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/java/nio/channels/Selector/StackOverflowTest.java Sat Nov 09 09:13:04 2019 +0000
@@ -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 (var sel = Selector.open()) {
+ recursiveSelect(sel);
+ } catch (StackOverflowError e) {
+ // ignore SOE from java calls
+ }
+ }
+
+ static void recursiveSelect(Selector sel) throws Exception {
+ sel.selectNow();
+ recursiveSelect(sel);
+ }
+}