diff mbox series

[Openvpn-devel,v2] Repair interaction between DCO and persist-tun after reconnection

Message ID 20260114112403.7046-1-gert@greenie.muc.de
State New
Headers
Series [Openvpn-devel,v2] Repair interaction between DCO and persist-tun after reconnection |

Commit Message

Gert Doering Jan. 14, 2026, 11:23 a.m. UTC 
  When --persist-tun is active, openvpn userland on Linux and FreeBSD fails
to re-enable "poll for DCO events" after a reconnect (e.g. triggered by
a ping timeout).  The reconnect will still work fine, but the *next*
DCO event notification from the kernel will not be received by OpenVPN
userland, and so the system will get into an inconsistent state (Userland
assumes "all is well", kernel DCO has disconnected the peer, connection
is broken until the next tls-renegotion and/or manual restart, *and* the
next DCO key setup might fail due to "peer id gone").

This only affects client side, --server tun is always "persistent", and
there is no "full restart" (and the code path in question is also
only used for client and p2p server).

The root cause is an incorrect check for "is this interface up?" when
calling dco_event_set() in forard.c::io_wait() - "c2.did_open_tun" is
only true if the tun interface was actually configured on this reconnect,
which it isn't if --persist-tun is active.  Replace with a check for
"do we have a tuntap structure, and if yes, do we have active DCO?"
which reflects the original intent much better.

The original code also had a check for "out_socket & EVENT_READ" there,
which did to some extend avoid calling dco_event_set() for every single
UDP packet sent and received by userland - but this only worked on initial
connection, and is always true on reconnect, so this condition was removed
for simplicity.  We should come back here...

v2:
  - some language fixes on the commit message
  - do not check ->dco.open in forward.c, as this is not available if
    not on FreeBSD, or if compiled with --disable-dco.
    FreeBSD DCO does the "if (!dco || !dco->open)" check in dco_event_set()
    anyway, so it's not needed, and Linux DCO has "dco->nl_sock", which is
    also reliably set/unset, and checked by dco_event_set() already.

Github: OpenVPN/openvpn#947

Change-Id: Idbd0a47ba4d297a833a350611a23f19fd9a797b5
Signed-off-by: Gert Doering <gert@greenie.muc.de>
Acked-by: Antonio Quartulli <antonio@mandelbit.com>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1473
---

This change was reviewed on Gerrit and approved by at least one
developer. I request to merge it to master.

Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1473
This mail reflects revision 2 of this Change.

Signed-off-by line for the author was added as per our policy.

Acked-by according to Gerrit (reflected above):
Antonio Quartulli <antonio@mandelbit.com>

Comments

Gert Doering Jan. 14, 2026, 11:51 a.m. UTC | #1 
This was quite a hunt... for an "it looks trivial" 1-liner ;-)

Your patch has been applied to the master and release/2.6 branch (bugfix)
(slight massaging needed because someone has renamed "socket" to
"out_socket", so the check going away looks different - but did the
same thing, and had the same bug.  Unsurprising, as this was day#1 DCO
code from before 2.6.0...)

Note that this commit does not close the GH issue yet - the original
"why does it crash on me?" bug has been fixed, but the resulting code
is still not appealing to my sensitivities.

Note2: this does not apply to Windows, because win-dco signals things
differently ("we get timeout notification from driver via error on 
pending read request").

commit 52c3b435b11e6daf7f3f9524ff801ba285c1d985 (master)
commit fae4a9e3f51554bdecc9df45344135006da1f0d9 (release/2.6)
Author: Gert Doering
Date:   Wed Jan 14 12:23:49 2026 +0100

     Repair interaction between DCO and persist-tun after reconnection

     Signed-off-by: Gert Doering <gert@greenie.muc.de>
     Acked-by: Antonio Quartulli <antonio@mandelbit.com>
     Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1473
     Message-Id: <20260114112403.7046-1-gert@greenie.muc.de>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg35239.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
index d208c21..39ac3b3 100644
--- a/src/openvpn/forward.c
+++ b/src/openvpn/forward.c
@@ -2197,7 +2197,7 @@ 
     multi_io_process_flags(c, c->c2.event_set, flags, &out_socket, &out_tuntap);
 
 #if defined(TARGET_LINUX) || defined(TARGET_FREEBSD)
-    if (out_socket & EVENT_READ && c->c2.did_open_tun)
+    if (c->c1.tuntap)
     {
         dco_event_set(&c->c1.tuntap->dco, c->c2.event_set, (void *)dco_shift);
     }