openvpn-devel

[Openvpn-devel] [XS] Change in openvpn[master]: Add ASSERT to afunix code that dev_node is always the way we expect

[plaisthos (C. Review) <ge...@op...>]

Attention is currently required from: flichtenheld.

Hello flichtenheld,

I'd like you to do a code review.
Please visit

    http://gerrit.openvpn.net/c/openvpn/+/1320?usp=email

to review the following change.


Change subject: Add ASSERT to afunix code that dev_node is always the way we expect
......................................................................

Add ASSERT to afunix code that dev_node is always the way we expect

The calling code only calls tun_afunix_exec_child if is_tun_afunix
which checks that the path is having unix: as prefix.

But since adding an ASSERT here to ensure that it is really the case
does not cost us anything, just add the ASSERT.

Reported-By: co...@jo...
Found-By: Zeropath
Change-Id: Idbb7bf279eb467fc1d56ab75a50b5eb2c8d0a57e
---
M src/openvpn/tun_afunix.c
1 file changed, 2 insertions(+), 0 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/20/1320/1

diff --git a/src/openvpn/tun_afunix.c b/src/openvpn/tun_afunix.c
index 4d48a31..e6f2be1 100644
--- a/src/openvpn/tun_afunix.c
+++ b/src/openvpn/tun_afunix.c
@@ -53,6 +53,8 @@
     const char *msgprefix = "ERROR: failure executing process for tun:";
     struct argv argv = argv_new();
 
+    /* we should always called with a proper unix: dev node string */
+    ASSERT(dev_node && strncmp(dev_node, "unix:", strlen("unix:")) == 0);
     /* since we know that dev-node starts with unix: we can just skip that
      * to get the program name */
     const char *program = dev_node + strlen("unix:");

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1320?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: Idbb7bf279eb467fc1d56ab75a50b5eb2c8d0a57e
Gerrit-Change-Number: 1320
Gerrit-PatchSet: 1
Gerrit-Owner: plaisthos <arn...@rf...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: flichtenheld <fr...@li...>

[Openvpn-devel] [XS] Change in openvpn[master]: Add ASSERT to afunix code that dev_node is always set up the way we e...

[cron2 (C. Review) <ge...@op...>]

Attention is currently required from: flichtenheld.

cron2 has uploaded a new patch set (#2) to the change originally created by plaisthos. ( http://gerrit.openvpn.net/c/openvpn/+/1320?usp=email )


Change subject: Add ASSERT to afunix code that dev_node is always set up the way we expect
......................................................................

Add ASSERT to afunix code that dev_node is always set up the way we expect

The calling code only calls tun_afunix_exec_child if is_tun_afunix is true,
which checks that the path is having unix: as prefix.

But since adding an ASSERT here to ensure that it is really the case
does not cost us anything, just add the ASSERT.

Reported-By: Joshua Rogers <co...@jo...>
Found-By: Zeropath

Change-Id: Idbb7bf279eb467fc1d56ab75a50b5eb2c8d0a57e
---
M src/openvpn/tun_afunix.c
1 file changed, 2 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/20/1320/2

diff --git a/src/openvpn/tun_afunix.c b/src/openvpn/tun_afunix.c
index 4d48a31..e6f2be1 100644
--- a/src/openvpn/tun_afunix.c
+++ b/src/openvpn/tun_afunix.c
@@ -53,6 +53,8 @@
     const char *msgprefix = "ERROR: failure executing process for tun:";
     struct argv argv = argv_new();
 
+    /* we should always called with a proper unix: dev node string */
+    ASSERT(dev_node && strncmp(dev_node, "unix:", strlen("unix:")) == 0);
     /* since we know that dev-node starts with unix: we can just skip that
      * to get the program name */
     const char *program = dev_node + strlen("unix:");

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1320?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email

Gerrit-MessageType: newpatchset
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Idbb7bf279eb467fc1d56ab75a50b5eb2c8d0a57e
Gerrit-Change-Number: 1320
Gerrit-PatchSet: 2
Gerrit-Owner: plaisthos <arn...@rf...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: flichtenheld <fr...@li...>

[Openvpn-devel] [XS] Change in openvpn[master]: Add ASSERT to afunix code that dev_node is always set up the way we e...

[cron2 (C. Review) <ge...@op...>]

Attention is currently required from: flichtenheld, plaisthos.

cron2 has posted comments on this change by plaisthos. ( http://gerrit.openvpn.net/c/openvpn/+/1320?usp=email )

Change subject: Add ASSERT to afunix code that dev_node is always set up the way we expect
......................................................................


Patch Set 2: Code-Review+2


-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1320?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email

Gerrit-MessageType: comment
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Idbb7bf279eb467fc1d56ab75a50b5eb2c8d0a57e
Gerrit-Change-Number: 1320
Gerrit-PatchSet: 2
Gerrit-Owner: plaisthos <arn...@rf...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Comment-Date: Tue, 28 Oct 2025 11:59:38 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes

[Openvpn-devel] [PATCH v2] Add ASSERT to afunix code that dev_node is always set up the way we expect

[Gert D. <ge...@gr...>]

From: Arne Schwabe <ar...@rf...>

The calling code only calls tun_afunix_exec_child if is_tun_afunix is true,
which checks that the path is having unix: as prefix.

But since adding an ASSERT here to ensure that it is really the case
does not cost us anything, just add the ASSERT.

Reported-By: Joshua Rogers <co...@jo...>
Found-By: Zeropath

Change-Id: Idbb7bf279eb467fc1d56ab75a50b5eb2c8d0a57e
Signed-off-by: Arne Schwabe <arn...@rf...>
Acked-by: Gert Doering <ge...@gr...>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1320
---

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/+/1320
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):
Gert Doering <ge...@gr...>

        
diff --git a/src/openvpn/tun_afunix.c b/src/openvpn/tun_afunix.c
index 4d48a31..e6f2be1 100644
--- a/src/openvpn/tun_afunix.c
+++ b/src/openvpn/tun_afunix.c
@@ -53,6 +53,8 @@
     const char *msgprefix = "ERROR: failure executing process for tun:";
     struct argv argv = argv_new();
 
+    /* we should always called with a proper unix: dev node string */
+    ASSERT(dev_node && strncmp(dev_node, "unix:", strlen("unix:")) == 0);
     /* since we know that dev-node starts with unix: we can just skip that
      * to get the program name */
     const char *program = dev_node + strlen("unix:");

[Openvpn-devel] [PATCH applied] Re: Add ASSERT to afunix code that dev_node is always set up the way we expect

[Gert D. <ge...@gr...>]

Looks reasonable, BB is happy, and the unit tests excercising tun_afunix
(t_server_null) still pass.  Also, I have a t_client test excercising
this, which still works :-)

I have updated the commit message a bit (language, Reported-By:), and
for one decided to try doing this in gerrit - it works, but is not the
best way to do it (it then does a "v2" of the patch, which hides the
test result of the actual patch, and creates extra noise on the list).

Your patch has been applied to the master branch.

commit 5bc0eae87ccf1abd6c400cb27d8e51819feb2036
Author: Arne Schwabe
Date:   Tue Oct 28 12:59:47 2025 +0100

     Add ASSERT to afunix code that dev_node is always set up the way we expect

     Signed-off-by: Arne Schwabe <arn...@rf...>
     Acked-by: Gert Doering <ge...@gr...>
     Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1320
     Message-Id: <202...@gr...>
     URL: https://www.mail-archive.com/ope...@li.../msg33934.html
     Signed-off-by: Gert Doering <ge...@gr...>


--
kind regards,

Gert Doering

[Openvpn-devel] [XS] Change in openvpn[master]: Add ASSERT to afunix code that dev_node is always set up the way we e...

[cron2 (C. Review) <ge...@op...>]

cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/1320?usp=email )

Change subject: Add ASSERT to afunix code that dev_node is always set up the way we expect
......................................................................

Add ASSERT to afunix code that dev_node is always set up the way we expect

The calling code only calls tun_afunix_exec_child if is_tun_afunix is true,
which checks that the path is having unix: as prefix.

But since adding an ASSERT here to ensure that it is really the case
does not cost us anything, just add the ASSERT.

Reported-By: Joshua Rogers <co...@jo...>
Found-By: Zeropath

Change-Id: Idbb7bf279eb467fc1d56ab75a50b5eb2c8d0a57e
Signed-off-by: Arne Schwabe <arn...@rf...>
Acked-by: Gert Doering <ge...@gr...>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1320
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg33934.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpn/tun_afunix.c
1 file changed, 2 insertions(+), 0 deletions(-)




diff --git a/src/openvpn/tun_afunix.c b/src/openvpn/tun_afunix.c
index 124db6d..42bcd0d 100644
--- a/src/openvpn/tun_afunix.c
+++ b/src/openvpn/tun_afunix.c
@@ -53,6 +53,8 @@
     const char *msgprefix = "ERROR: failure executing process for tun:";
     struct argv argv = argv_new();
 
+    /* we should always called with a proper unix: dev node string */
+    ASSERT(dev_node && strncmp(dev_node, "unix:", strlen("unix:")) == 0);
     /* since we know that dev-node starts with unix: we can just skip that
      * to get the program name */
     const char *program = dev_node + strlen("unix:");

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1320?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email

Gerrit-MessageType: merged
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Idbb7bf279eb467fc1d56ab75a50b5eb2c8d0a57e
Gerrit-Change-Number: 1320
Gerrit-PatchSet: 3
Gerrit-Owner: plaisthos <arn...@rf...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-CC: openvpn-devel <ope...@li...>

[Openvpn-devel] [XS] Change in openvpn[master]: Add ASSERT to afunix code that dev_node is always set up the way we e...

[cron2 (C. Review) <ge...@op...>]

cron2 has uploaded a new patch set (#3) to the change originally created by plaisthos. ( http://gerrit.openvpn.net/c/openvpn/+/1320?usp=email )

The following approvals got outdated and were removed:
Code-Review+2 by cron2


Change subject: Add ASSERT to afunix code that dev_node is always set up the way we expect
......................................................................

Add ASSERT to afunix code that dev_node is always set up the way we expect

The calling code only calls tun_afunix_exec_child if is_tun_afunix is true,
which checks that the path is having unix: as prefix.

But since adding an ASSERT here to ensure that it is really the case
does not cost us anything, just add the ASSERT.

Reported-By: Joshua Rogers <co...@jo...>
Found-By: Zeropath

Change-Id: Idbb7bf279eb467fc1d56ab75a50b5eb2c8d0a57e
Signed-off-by: Arne Schwabe <arn...@rf...>
Acked-by: Gert Doering <ge...@gr...>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1320
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg33934.html
Signed-off-by: Gert Doering <ge...@gr...>
---
M src/openvpn/tun_afunix.c
1 file changed, 2 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/20/1320/3

diff --git a/src/openvpn/tun_afunix.c b/src/openvpn/tun_afunix.c
index 124db6d..42bcd0d 100644
--- a/src/openvpn/tun_afunix.c
+++ b/src/openvpn/tun_afunix.c
@@ -53,6 +53,8 @@
     const char *msgprefix = "ERROR: failure executing process for tun:";
     struct argv argv = argv_new();
 
+    /* we should always called with a proper unix: dev node string */
+    ASSERT(dev_node && strncmp(dev_node, "unix:", strlen("unix:")) == 0);
     /* since we know that dev-node starts with unix: we can just skip that
      * to get the program name */
     const char *program = dev_node + strlen("unix:");

-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1320?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email

Gerrit-MessageType: newpatchset
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Idbb7bf279eb467fc1d56ab75a50b5eb2c8d0a57e
Gerrit-Change-Number: 1320
Gerrit-PatchSet: 3
Gerrit-Owner: plaisthos <arn...@rf...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-CC: openvpn-devel <ope...@li...>