diff mbox series

[Openvpn-devel,v2] dco_linux: validate tun interface before fetching stats

Message ID 20250902164521.23145-1-gert@greenie.muc.de
State Accepted
Headers show
Series [Openvpn-devel,v2] dco_linux: validate tun interface before fetching stats | expand

Commit Message

Gert Doering Sept. 2, 2025, 4:45 p.m. UTC 
From: Ralf Lici <ralf@mandelbit.com>

If dco_get_peer_stats() is called with an uninitialized c->c1.tuntap it
results in a segfault. This issue happens when a client who has not
connected to any server:
  - has --management and exits,
  - has --management and a management interface client issues either
    `bytecount` or `status` or
  - if SIGUSR2 is sent to it.

Add a check to ensure the tun interface was set up before attempting to
retrieve peer statistics.

Change-Id: I40c11864745cc1619cb9cbf490b168f90feb5eac
Signed-off-by: Ralf Lici <ralf@mandelbit.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
---

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/+/1166
This mail reflects revision 2 of this Change.

Acked-by according to Gerrit (reflected above):
Gert Doering <gert@greenie.muc.de>

Comments

Gert Doering Sept. 2, 2025, 5 p.m. UTC | #1 
Tested this on ubuntu 20.04 + DCO -  --client + --management-hold, connect
to mgmt, "bytecount 1", "hold release", boom, SIGSEGV.

v1 looked correct, but checked the wrong variables, so "no SIGSEGV, but
no counters at all anymore", not so good either ;-)  - v2 fixes that,
no crash *and* working counters.  With proper timing.  So really getting
there, after a day staring at manage.c...

Your patch has been applied to the master branch.

commit 14ced61eef89797e606c3b7600adc4da83a336f4
Author: Ralf Lici
Date:   Tue Sep 2 18:45:15 2025 +0200

     dco_linux: validate tun interface before fetching stats

     Signed-off-by: Ralf Lici <ralf@mandelbit.com>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Message-Id: <20250902164521.23145-1-gert@greenie.muc.de>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg32768.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c
index a3907fe..e70b74d 100644
--- a/src/openvpn/dco_linux.c
+++ b/src/openvpn/dco_linux.c
@@ -1139,6 +1139,8 @@ 
 static int
 dco_get_peer(dco_context_t *dco, int peer_id, const bool raise_sigusr1_on_err)
 {
+    ASSERT(dco);
+
     /* peer_id == -1 means "dump all peers", but this is allowed in MP mode only.
      * If it happens in P2P mode it means that the DCO peer was deleted and we
      * can simply bail out
@@ -1182,6 +1184,11 @@ 
 int
 dco_get_peer_stats(struct context *c, const bool raise_sigusr1_on_err)
 {
+    if (!c->c1.tuntap || c->c1.tuntap->dco.ifindex == 0)
+    {
+        return -1;
+    }
+
     return dco_get_peer(&c->c1.tuntap->dco, c->c2.tls_multi->dco_peer_id, raise_sigusr1_on_err);
 }