]> git.ipfire.org Git - thirdparty/openvpn.git/commit
bandaid fix for TCP multipoint server crash with Linux-DCO
authorGert Doering <gert@greenie.muc.de>
Tue, 27 Dec 2022 20:26:14 +0000 (21:26 +0100)
committerGert Doering <gert@greenie.muc.de>
Tue, 27 Dec 2022 22:01:21 +0000 (23:01 +0100)
commitc7416160fb2e5a66d5801e4b789751a7480e6384
tree22f22c0d2569182ec7b83c819a8b1722ad202134
parente7f2169772f90f9bf158a17f5656a6a985e74e31
bandaid fix for TCP multipoint server crash with Linux-DCO

TCP multipoint servers with Linux-DCO can crash under yet-unknown
circumstances where a TCP socket gets handed to the kernel (= userland
shall not acceess it again) but the socket still lands in the event
polling mechanism, and is passed to link_socket_read() with
sock->fd being "-1" (SOCKET_UNDEFINED).

This is a bug, but it happens very unfrequently so not fixed yet.

When this happens, the server gets stuck in an endless loop of
"trying recvfrom(-1, ..), getting an error, looging that error,
continue" until the server's disk is full.

The situation is being made a bit more complex by the dco-win
approach of treating "all kernel sockets as UDP", so the Linux
implementation tries to access the -1 socket as UDP, confusing
the picture more.

As a bandaid to avoid the crash, this patch changes

 - socket.h: only do the "if dco_installed, treat as UDP" for WIN32
   (link_socket_read())

 - socket.c: add ASSERT(sock->fd >= 0); checks to all UDP socket paths
   (we should never even hit those as this is a TCP specific problem,
   but in the "sock->fd = -1" case, doing a clean server abort is
   preferred to "the disk is full with non-helpful logfiles, and then
   the server crashes anyway")

 - socket.c: in the TCP read function, link_socket_read_tcp(),
   check for sock->fd < 0 and trigger "sock->stream_reset = true"
   (+ write to the log what happened).

This change will kill this particular TCP client instance (SIGTERM),
but leave the rest of the server running fine - and given that
in our tests this issue seems to be triggered by inbound TCP RST
in just the wrong moment, it seems to be "a properly-sized bandaid".

v2: rebase on top of "move dco_installed back to link_socket"
v3: move sock->fd check inside !residual_fully_formed clause (so
    we can still handle already-read packets)

Github: OpenVPN/openvpn#190

Signed-off-by: Gert Doering <gert@greenie.muc.de>
Acked-by: Arne Schwabe <arne@rfc2549.org>
Message-Id: <20221227202614.2114971-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25844.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
src/openvpn/socket.c
src/openvpn/socket.h