plaisthos has uploaded this change for review. (
http://gerrit.openvpn.net/c/openvpn/+/1477?usp=email )
Change subject: Fix rare crash when read buffer are not initialised and read is
still signalled
......................................................................
Fix rare crash when read buffer are not initialised and read is still signalled
This assertion happens when we do not expect a read event from the socket and
then in link_socket_read_tcp the function stream_buf_get_next can trigger
an assert on ASSERT(buf_defined(&sb->next));
To avoid this weird corner case, just always initialise the read buffer
whether or not we expect a read to occur.
Reproducing this bug requires very special circumstances. The reproduction is
openvpn --cert server-secp384r1.crt --key server-secp384r1.key \
--dev ml-tan --dev-type tap --tun-mtu 1400 --config ~/fp --topology subnet \
--port 2201 --verb 3 --mode server --tls-server --proto tcp6-server
--ifconfig 10.0.0.1 255.255.255.0
as the server side and
openvpn --client --proto tcp --remote poseidon --port 2201 --dev tap
The client side must be on Linux. Other platform do not reproduce this
bug.
Note that the client will not configure any IP or IPv6 on the interface
and will also not bring up the interface. The server must also send at least
one real data packet to the client (no keepalive ping). Just having the
interface up normally produces enough traffic.
Now forcefully reset the TCP connection. E.g. by executing on the server
sudo ss --kill sport 2201
This will now trigger the assertion. This happens since OpenVPN waits
forever to get a write back from the poll from the tun/tap device but
this never happens since the device is not up.
As long as we do not get back the tun device for writing, we also do
not put the socket back into the EVENT_READ state. And this also means
that code to initialise the read buffer (stream_buf_set_next) is never
run.
But the reset on the TCP socket triggers the TCP socket to be available
for read, even if it is just for a read of 0 bytes to indicate the reset.
So the link_socket_read_tcp will run into the assert.
Change-Id: Ifd3e953104a67c8bf2a225e179865e3dbd0dbfbc
Signed-off-by: Arne Schwabe <[email protected]>
---
M src/openvpn/socket.c
1 file changed, 5 insertions(+), 1 deletion(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/77/1477/1
diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c
index 093f822..0858e64 100644
--- a/src/openvpn/socket.c
+++ b/src/openvpn/socket.c
@@ -2927,7 +2927,11 @@
{
if (s)
{
- if ((rwflags & EVENT_READ) && !stream_buf_read_setup(s))
+ /* Always setup the buffer to be able to get a read. There are corner
cases
+ * where the socket select/poll returns a socket for read when we do
not
+ * expect it, e.g. a read of len 0 when the socket receives a reset */
+ const bool read_setup = stream_buf_read_setup(s);
+ if ((rwflags & EVENT_READ) && !read_setup)
{
ASSERT(!persistent);
rwflags &= ~EVENT_READ;
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1477?usp=email
To unsubscribe, or for help writing mail filters, visit
http://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ifd3e953104a67c8bf2a225e179865e3dbd0dbfbc
Gerrit-Change-Number: 1477
Gerrit-PatchSet: 1
Gerrit-Owner: plaisthos <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
