[Openvpn-devel,v1] Remove null check after checking for checking for did_open_tun

Message ID 20240925151104.13036-1-gert@greenie.muc.de
State Accepted
Headers show
Series [Openvpn-devel,v1] Remove null check after checking for checking for did_open_tun | expand

Commit Message

Gert Doering Sept. 25, 2024, 3:11 p.m. UTC
From: Arne Schwabe <arne@rfc2549.org>

If we indicate that the tun device has been opened the c1.tuntap struct
is guaranteed to be defined. This extra null check is something that
Coverity flags as we access a do a null check after already accessing fields
of tuntap

Change-Id: I9966636163c7dfa208d26f1cadbf5b81937f3a34
Signed-off-by: Arne Schwabe <arne-openvpn@rfc2549.org>
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/+/772
This mail reflects revision 1 of this Change.

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

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

Comments

Gert Doering Sept. 25, 2024, 4:53 p.m. UTC | #1
I've stared long and hard on the code - nothing really to test here, as
*this* pointer access will never be NULL, if the access before it already
triggers the crash - and I am reasonably sure that *in this function*,
if "c2.did_tun_open" is true, c1.tuntap will always be initialized, so
this is safe (after do_close_tun() this is no longer guaranteed,
but the code path inside do_up() does a new open right away, and the
other code paths will not call do_up()).

Your patch has been applied to the master branch.

commit a8cc97f94f3e7bb52270ca2acf89804093535839
Author: Arne Schwabe
Date:   Wed Sep 25 17:11:04 2024 +0200

     Remove null check after checking for checking for did_open_tun

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


--
kind regards,

Gert Doering

Patch

diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index 876edad..ae911a9 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -2546,10 +2546,7 @@ 
             {
                 event_timeout_init(&c->c2.route_wakeup, c->options.route_delay, now);
                 event_timeout_init(&c->c2.route_wakeup_expire, c->options.route_delay + c->options.route_delay_window, now);
-                if (c->c1.tuntap)
-                {
-                    tun_standby_init(c->c1.tuntap);
-                }
+                tun_standby_init(c->c1.tuntap);
             }
             else
             {