[Openvpn-devel,v3] Improve error reporting from AF_UNIX tun/tap support
Commit Message
From: Arne Schwabe <arne@rfc2549.org>
When having a non-existent lwipovpn binary or similar problems, the
error reporting would often only report read error that were harder to
identify the real problem. Add the openvpn_waitpid_check method
that checks for error conditions and reports a better message in cases
of problems.
Change-Id: I81cbecd19018290d85c6c77fba7769f040d66233
Signed-off-by: Arne Schwabe <arne@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/+/855
This mail reflects revision 3 of this Change.
Acked-by according to Gerrit (reflected above):
Gert Doering <gert@greenie.muc.de>
Comments
Significant improvement, this :-)
Old code, when executing a nonexistant binary:
2025-02-01 19:33:56 Child process PID 7167 for afunix dead? Return code: 32512
2025-02-01 19:33:56 write to TUN/TAP : No error: 0 (fd=-1,code=0)
New code:
2025-02-01 19:35:06 Protocol options: protocol-flags tls-ekm
2025-02-01 19:35:07 ERROR: failure during write to AF_UNIX socket: could not execute external program (exit code 127)
2025-02-01 19:35:07 write to TUN/TAP : No error: 0 (fd=-1,code=0)
2025-02-01 19:35:07 read from TUN/TAP : Connection reset by peer (fd=-1,code=54)
2025-02-01 19:35:08 write to TUN/TAP : No error: 0 (fd=-1,code=0)
Killing lwipovpn while OpenVPN is active yields
2025-02-01 19:36:13 ERROR: failure during write to AF_UNIX socket: external program received signal 15
2025-02-01 19:36:13 write to TUN/TAP : No error: 0 (fd=-1,code=0)
2025-02-01 19:36:13 read from TUN/TAP : Connection reset by peer (fd=-1,code=54)
(the "write to TUN/TAP: No error" is still repeating itself for every
packet, so dunno, maybe we should trigger something-SIG* then? But
that's just "room for further improvement")
Your patch has been applied to the master branch.
commit 800e8abdecd6cab62fd2074594b4e08aa72d0d82
Author: Arne Schwabe
Date: Sat Feb 1 13:20:06 2025 +0100
Improve error reporting from AF_UNIX tun/tap support
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20250201122006.32098-1-gert@greenie.muc.de>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg30782.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
--
kind regards,
Gert Doering
@@ -106,6 +106,47 @@
return (const char *)out.data;
}
+#ifndef WIN32
+bool
+openvpn_waitpid_check(pid_t pid, const char *msg_prefix, int msglevel)
+{
+ if (pid == 0)
+ {
+ return false;
+ }
+ int status;
+ pid_t pidret = waitpid(pid, &status, WNOHANG);
+ if (pidret != pid)
+ {
+ return true;
+ }
+
+ if (WIFEXITED(status))
+ {
+ int exitcode = WEXITSTATUS(status);
+
+ if (exitcode == OPENVPN_EXECVE_FAILURE)
+ {
+ msg(msglevel, "%scould not execute external program (exit code 127)",
+ msg_prefix);
+ }
+ else
+ {
+ msg(msglevel, "%sexternal program exited with error status: %d",
+ msg_prefix, exitcode);
+ }
+
+ }
+ else if (WIFSIGNALED(status))
+ {
+ msg(msglevel, "%sexternal program received signal %d",
+ msg_prefix, WTERMSIG(status));
+ }
+
+ return false;
+}
+#endif /* ifndef WIN32 */
+
bool
openvpn_execve_allowed(const unsigned int flags)
{
@@ -59,6 +59,26 @@
int openvpn_execve_check(const struct argv *a, const struct env_set *es,
const unsigned int flags, const char *error_message);
+
+#ifndef WIN32
+/** Checks if a running process is still running. This is mainly useful
+ * for processes started with \c S_NOWAITPID
+ *
+ * This function is currently not implemented for Windows as the helper
+ * macros used by this function are not available.
+ *
+ * @param pid pid of the process to be checked
+ * @param msg_prefix prefixed of the message that be printed
+ * @param msglevel msglevel of the messages to be printed
+ * @return true if the process is still running, false if
+ * an error condition occurred
+ */
+bool
+openvpn_waitpid_check(pid_t pid, const char *msg_prefix,
+ int msglevel);
+
+#endif
+
/**
* Will run a script and return the exit code of the script if between
* 0 and 255, -1 otherwise
@@ -47,9 +47,12 @@
#include <signal.h>
#include <stdlib.h>
+
+
static void
tun_afunix_exec_child(const char *dev_node, struct tuntap *tt, struct env_set *env)
{
+ const char *msgprefix = "ERROR: failure executing process for tun:";
struct argv argv = argv_new();
/* since we know that dev-node starts with unix: we can just skip that
@@ -58,10 +61,12 @@
argv_printf(&argv, "%s", program);
- argv_msg(M_INFO, &argv);
tt->afunix.childprocess = openvpn_execve_check(&argv, env, S_NOWAITPID,
- "ERROR: failure executing "
- "process for tun");
+ msgprefix);
+ if (!openvpn_waitpid_check(tt->afunix.childprocess, msgprefix, M_WARN))
+ {
+ tt->afunix.childprocess = 0;
+ }
argv_free(&argv);
}
@@ -138,20 +143,27 @@
ssize_t
write_tun_afunix(struct tuntap *tt, uint8_t *buf, int len)
{
- int ret;
- pid_t pidret = waitpid(tt->afunix.childprocess, &ret, WNOHANG);
- if (pidret == tt->afunix.childprocess)
+ const char *msg = "ERROR: failure during write to AF_UNIX socket: ";
+ if (!openvpn_waitpid_check(tt->afunix.childprocess, msg, M_WARN))
{
- msg(M_INFO, "Child process PID %d for afunix dead? Return code: %d",
- tt->afunix.childprocess, ret);
+ tt->afunix.childprocess = 0;
return -ENXIO;
}
+
return write(tt->fd, buf, len);
}
ssize_t
read_tun_afunix(struct tuntap *tt, uint8_t *buf, int len)
{
+ const char *msg = "ERROR: failure during read from AF_UNIX socket: ";
+ if (!openvpn_waitpid_check(tt->afunix.childprocess, msg, M_WARN))
+ {
+ tt->afunix.childprocess = 0;
+ }
+ /* do an actual read on the file descriptor even in the error case since
+ * we otherwise loop on this on this from select and spam the console
+ * with error messages */
return read(tt->fd, buf, len);
}
#else /* ifndef WIN32 */