Avoid dangling socket slot access in the WinAPI driver.

When the last reference got released while a blocking socket operation was still in progress, the old slot was still accessed from the completion callback, leading to null pointer accesses or other issues.
This commit is contained in:
Sönke Ludwig 2017-08-21 21:02:13 +02:00
parent 44b43e8415
commit 74352a9ac5

View file

@ -72,6 +72,7 @@ final class WinAPIEventDriverSockets : EventDriverSockets {
connectCallback = on_connect; connectCallback = on_connect;
state = ConnectionState.connecting; state = ConnectionState.connecting;
} }
addRef(sock);
return sock; return sock;
} else { } else {
clearSocketSlot(sock); clearSocketSlot(sock);
@ -111,7 +112,7 @@ final class WinAPIEventDriverSockets : EventDriverSockets {
setupOverlapped(streamSocket.read.overlapped); setupOverlapped(streamSocket.read.overlapped);
} }
() @trusted { WSAAsyncSelect(socket, m_hwnd, WM_USER_SOCKET, FD_READ|FD_WRITE|FD_CONNECT|FD_CLOSE); } (); () @trusted { WSAAsyncSelect(socket, m_hwnd, WM_USER_SOCKET, FD_CONNECT|FD_CLOSE); } ();
return fd; return fd;
} }
@ -223,6 +224,7 @@ final class WinAPIEventDriverSockets : EventDriverSockets {
} }
} }
slot.read.callback = on_read_finish; slot.read.callback = on_read_finish;
addRef(socket);
m_core.addWaiter(); m_core.addWaiter();
} }
@ -239,7 +241,8 @@ final class WinAPIEventDriverSockets : EventDriverSockets {
slot.common.core.removeWaiter(); slot.common.core.removeWaiter();
auto cb = slot.streamSocket.read.callback; auto cb = slot.streamSocket.read.callback;
slot.streamSocket.read.callback = null; slot.streamSocket.read.callback = null;
cb(cast(StreamSocketFD)slot.common.fd, status, nsent); if (slot.common.driver.releaseRef(cast(StreamSocketFD)slot.common.fd))
cb(cast(StreamSocketFD)slot.common.fd, status, nsent);
} }
slot.streamSocket.read.bytesTransferred += cbTransferred; slot.streamSocket.read.bytesTransferred += cbTransferred;
@ -295,6 +298,7 @@ final class WinAPIEventDriverSockets : EventDriverSockets {
} }
} }
m_sockets[socket].streamSocket.write.callback = on_write_finish; m_sockets[socket].streamSocket.write.callback = on_write_finish;
addRef(socket);
m_core.addWaiter(); m_core.addWaiter();
} }
@ -310,7 +314,8 @@ final class WinAPIEventDriverSockets : EventDriverSockets {
slot.common.core.removeWaiter(); slot.common.core.removeWaiter();
auto cb = slot.streamSocket.write.callback; auto cb = slot.streamSocket.write.callback;
slot.streamSocket.write.callback = null; slot.streamSocket.write.callback = null;
cb(cast(StreamSocketFD)slot.common.fd, status, nsent); if (slot.common.driver.releaseRef(cast(StreamSocketFD)slot.common.fd))
cb(cast(StreamSocketFD)slot.common.fd, status, nsent);
} }
slot.streamSocket.write.bytesTransferred += cbTransferred; slot.streamSocket.write.bytesTransferred += cbTransferred;
@ -463,6 +468,7 @@ final class WinAPIEventDriverSockets : EventDriverSockets {
() @trusted { CancelIoEx(cast(HANDLE)cast(SOCKET)socket, cast(LPOVERLAPPED)&slot.read.overlapped); } (); () @trusted { CancelIoEx(cast(HANDLE)cast(SOCKET)socket, cast(LPOVERLAPPED)&slot.read.overlapped); } ();
slot.read.callback = on_read_finish; slot.read.callback = on_read_finish;
addRef(socket);
m_core.addWaiter(); m_core.addWaiter();
} }
@ -487,7 +493,8 @@ final class WinAPIEventDriverSockets : EventDriverSockets {
auto cb = slot.datagramSocket.read.callback; auto cb = slot.datagramSocket.read.callback;
slot.datagramSocket.read.callback = null; slot.datagramSocket.read.callback = null;
scope addr = new RefAddress(cast(sockaddr*)&slot.datagramSocket.sourceAddr, slot.datagramSocket.sourceAddrLen); scope addr = new RefAddress(cast(sockaddr*)&slot.datagramSocket.sourceAddr, slot.datagramSocket.sourceAddrLen);
cb(cast(DatagramSocketFD)slot.common.fd, status, nsent, status == IOStatus.ok ? addr : null); if (slot.common.driver.releaseRef(cast(DatagramSocketFD)slot.common.fd))
cb(cast(DatagramSocketFD)slot.common.fd, status, nsent, status == IOStatus.ok ? addr : null);
} }
slot.datagramSocket.read.bytesTransferred += cbTransferred; slot.datagramSocket.read.bytesTransferred += cbTransferred;
@ -551,6 +558,7 @@ final class WinAPIEventDriverSockets : EventDriverSockets {
() @trusted { CancelIoEx(cast(HANDLE)cast(SOCKET)socket, cast(LPOVERLAPPED)&slot.write.overlapped); } (); () @trusted { CancelIoEx(cast(HANDLE)cast(SOCKET)socket, cast(LPOVERLAPPED)&slot.write.overlapped); } ();
slot.write.callback = on_write_finish; slot.write.callback = on_write_finish;
addRef(socket);
m_core.addWaiter(); m_core.addWaiter();
} }
@ -577,11 +585,13 @@ final class WinAPIEventDriverSockets : EventDriverSockets {
slot.datagramSocket.write.callback = null; slot.datagramSocket.write.callback = null;
slot.datagramSocket.targetAddr = null; slot.datagramSocket.targetAddr = null;
if (addr) { if (slot.common.driver.releaseRef(cast(DatagramSocketFD)slot.common.fd)) {
scope raddr = new RefAddress(addr.name, addr.nameLen); if (addr) {
cb(cast(DatagramSocketFD)slot.common.fd, status, nsent, raddr); scope raddr = new RefAddress(addr.name, addr.nameLen);
} else { cb(cast(DatagramSocketFD)slot.common.fd, status, nsent, raddr);
cb(cast(DatagramSocketFD)slot.common.fd, status, nsent, null); } else {
cb(cast(DatagramSocketFD)slot.common.fd, status, nsent, null);
}
} }
} }
@ -683,7 +693,7 @@ final class WinAPIEventDriverSockets : EventDriverSockets {
{ {
m_sockets[fd.value].common.refCount = 1; m_sockets[fd.value].common.refCount = 1;
m_sockets[fd.value].common.fd = fd; m_sockets[fd.value].common.fd = fd;
m_sockets[fd.value].common.core = m_core; m_sockets[fd.value].common.driver = this;
} }
package void clearSocketSlot(FD fd) package void clearSocketSlot(FD fd)
@ -718,7 +728,8 @@ final class WinAPIEventDriverSockets : EventDriverSockets {
cb(cast(StreamSocketFD)sock, ConnectStatus.refused); cb(cast(StreamSocketFD)sock, ConnectStatus.refused);
} else { } else {
slot.streamSocket.state = ConnectionState.connected; slot.streamSocket.state = ConnectionState.connected;
cb(cast(StreamSocketFD)sock, ConnectStatus.connected); if (slot.common.driver.releaseRef(cast(StreamSocketFD)sock))
cb(cast(StreamSocketFD)sock, ConnectStatus.connected);
} }
break; break;
case FD_READ: case FD_READ:
@ -781,7 +792,8 @@ void setupWindowClass() nothrow
static struct SocketSlot { static struct SocketSlot {
SocketFD fd; // redundant, but needed by the current IO Completion Routines based approach SocketFD fd; // redundant, but needed by the current IO Completion Routines based approach
WinAPIEventDriverCore core; // redundant, but needed by the current IO Completion Routines based approach WinAPIEventDriverSockets driver; // redundant, but needed by the current IO Completion Routines based approach
@property inout(WinAPIEventDriverCore) core() @safe nothrow inout { return driver.m_core; }
int refCount; int refCount;
DataInitializer userDataDestructor; DataInitializer userDataDestructor;
ubyte[16*size_t.sizeof] userData; ubyte[16*size_t.sizeof] userData;