[Openvpn-devel] Fix M_ERRNO behavior on Windows

Message ID 20220222031424.406-1-lstipakov@gmail.com
State Superseded
Headers show
Series
  • [Openvpn-devel] Fix M_ERRNO behavior on Windows
Related show

Commit Message

Lev Stipakov Feb. 22, 2022, 3:14 a.m.
From: Lev Stipakov <lev@openvpn.net>

We use M_ERRNO flag in logging to display error code
and error message. This has been broken on Windows,
where we use error code from GetLastError() and
error description from strerror(). strerror() expects
C runtime error code, which is quite different from
last error code from WinAPI call. As a result, we got
incorrect error description.

The ultimate fix would be introducing another flag
for WinAPI errors, like M_WINERR and use either that or
M_ERRNO depends on context. However, the change would be
quite intrusive and in some cases it is hard to say which
one to use without looking into internals.

Instead we stick to M_ERRNO and in Windows case we
first try to obtain error code from GetLastError() and
if it returns ERROR_SUCCESS (which is 0), we assume that
we have C runtime error and use errno. To get error
description we use strerror_win32() with GetLastError()
and strerror() with errno.

strerror_win32() uses FormatMessage() internally, which
is the right way to get WinAPI error description.

While on it, add a new flag M_SKERR to display socket
error. On Linux it is resolved into M_ERRNO and on Windows
to an conjunction of another new flag M_WSAERR and M_ERRNO.
With M_WSAERR we use WSAGetLastError() to get error code
and strerror_win32() to get error description.
WSAGetLastError() is a correct way to get WinSock error code,
according to documentation.

Signed-off-by: Lev Stipakov <lev@openvpn.net>
---
 src/openvpn/error.c    | 26 ++++++++++++++++++++++----
 src/openvpn/error.h    | 11 ++++++++++-
 src/openvpn/manage.c   |  4 ++--
 src/openvpn/mtu.c      |  2 +-
 src/openvpn/platform.c |  2 +-
 src/openvpn/proxy.c    |  8 ++++----
 src/openvpn/ps.c       |  2 +-
 src/openvpn/socket.c   | 20 ++++++++++----------
 src/openvpn/socket.h   |  6 ++++++
 src/openvpn/socks.c    | 26 +++++++++++++-------------
 10 files changed, 70 insertions(+), 37 deletions(-)

Comments

Selva Nair April 22, 2022, 3:17 p.m. | #1
Hi,

Sorry for the long delay in getting back to this..

On Tue, Feb 22, 2022 at 9:13 AM Lev Stipakov <lstipakov@gmail.com> wrote:

> From: Lev Stipakov <lev@openvpn.net>
>
> We use M_ERRNO flag in logging to display error code
> and error message. This has been broken on Windows,
> where we use error code from GetLastError() and
> error description from strerror(). strerror() expects
> C runtime error code, which is quite different from
> last error code from WinAPI call. As a result, we got
> incorrect error description.
>
> The ultimate fix would be introducing another flag
> for WinAPI errors, like M_WINERR and use either that or
> M_ERRNO depends on context. However, the change would be
> quite intrusive and in some cases it is hard to say which
> one to use without looking into internals.
>
> Instead we stick to M_ERRNO and in Windows case we
> first try to obtain error code from GetLastError() and
> if it returns ERROR_SUCCESS (which is 0), we assume that
> we have C runtime error and use errno. To get error
> description we use strerror_win32() with GetLastError()
> and strerror() with errno.
>
> strerror_win32() uses FormatMessage() internally, which
> is the right way to get WinAPI error description.
>
> While on it, add a new flag M_SKERR to display socket
> error. On Linux it is resolved into M_ERRNO and on Windows
> to an conjunction of another new flag M_WSAERR and M_ERRNO.
> With M_WSAERR we use WSAGetLastError() to get error code
> and strerror_win32() to get error description.
> WSAGetLastError() is a correct way to get WinSock error code,
> according to documentation.
>
> Signed-off-by: Lev Stipakov <lev@openvpn.net>
> ---
>  src/openvpn/error.c    | 26 ++++++++++++++++++++++----
>  src/openvpn/error.h    | 11 ++++++++++-
>  src/openvpn/manage.c   |  4 ++--
>  src/openvpn/mtu.c      |  2 +-
>  src/openvpn/platform.c |  2 +-
>  src/openvpn/proxy.c    |  8 ++++----
>  src/openvpn/ps.c       |  2 +-
>  src/openvpn/socket.c   | 20 ++++++++++----------
>  src/openvpn/socket.h   |  6 ++++++
>  src/openvpn/socks.c    | 26 +++++++++++++-------------
>  10 files changed, 70 insertions(+), 37 deletions(-)
>
> diff --git a/src/openvpn/error.c b/src/openvpn/error.c
> index b0e9a48c..5da588d9 100644
> --- a/src/openvpn/error.c
> +++ b/src/openvpn/error.c
> @@ -242,7 +242,25 @@ x_msg_va(const unsigned int flags, const char
> *format, va_list arglist)
>          return;
>      }
>
> -    e = openvpn_errno();
> +    bool use_strerr = false;
> +#ifdef _WIN32
> +    if (flags & M_WSAERR)
> +    {
> +        e = WSAGetLastError();
> +    }
> +    else
> +    {
> +        e = GetLastError();
> +        if (e == ERROR_SUCCESS)
> +        {
> +            /* error is likely C runtime, fallback to errno/strerr */
> +            use_strerr = true;
> +            e = errno;
> +        }
> +    }
> +#else
> +    e = errno;
> +#endif
>
>
Ideally I would have liked to modify openvpn_errno() and
openvpn_strerror()  to do the right thing instead of adding this logic here
and below. I have not worked out what exactly that would entail --  most
likely those two functions would require an extra argument to keep track of
the kind of error.

Also there are some places openvpn_errno() is used and the result checked
against POSIX error codes:
in forward.c around line 2102:

if (.... && ENETUNREACH == error_code && ...)

where error_code = openvpn_errno() which may return WSAENETUNREACH on
Windows not ENETUNREACH-- even that depends on GetLastError() and
WSAGetLastError() to behave identically.

Also does code like the below in socket.c do the right thing?
(around line 1451 in socket.c)
status = openvpn_errno();
if (status == WSAEWOULDBLOCK) ....
but openvpn_errono() doesn't look for WSA errors (though it probably still
works?).

     /*
>       * Apply muting filter.
> @@ -264,7 +282,7 @@ x_msg_va(const unsigned int flags, const char *format,
> va_list arglist)
>      if ((flags & M_ERRNO) && e)
>      {
>          openvpn_snprintf(m2, ERR_BUF_SIZE, "%s: %s (errno=%d)",
> -                         m1, strerror(e), e);
> +                         m1, use_strerr ? strerror(e) :
> openvpn_strerror(e, &gc), e);
>          SWAP;
>      }
>
> @@ -678,13 +696,13 @@ x_check_status(int status,
>              {
>                  msg(x_cs_info_level, "%s %s [%s]: %s (fd=%d,code=%d)",
> description,
>                      sock ? proto2ascii(sock->info.proto, sock->info.af,
> true) : "",
> -                    extended_msg, strerror(my_errno), sock ? sock->sd :
> -1, my_errno);
> +                    extended_msg, openvpn_strerror(my_errno, &gc), sock ?
> sock->sd : -1, my_errno);
>

Here my_errno = openvpn_errno() -- don't we need the fallback to POSIX
errno in this case? Could this also get called when a socket error is
expected?


>              }
>              else
>              {
>                  msg(x_cs_info_level, "%s %s: %s (fd=%d,code=%d)",
> description,
>                      sock ? proto2ascii(sock->info.proto, sock->info.af,
> true) : "",
> -                    strerror(my_errno), sock ? sock->sd : -1, my_errno);
> +                    openvpn_strerror(my_errno, &gc), sock ? sock->sd :
> -1, my_errno);
>              }
>
>              if (x_cs_err_delay_ms)
> diff --git a/src/openvpn/error.h b/src/openvpn/error.h
> index ad7defe8..9c3f4dfb 100644
> --- a/src/openvpn/error.h
> +++ b/src/openvpn/error.h
> @@ -99,9 +99,18 @@ extern int x_msg_line_num;
>  #define M_NONFATAL        (1<<5)         /* non-fatal error */
>  #define M_WARN            (1<<6)         /* call syslog with LOG_WARNING
> */
>  #define M_DEBUG           (1<<7)
> -
>  #define M_ERRNO           (1<<8)         /* show errno description */
>
> +#ifdef _WIN32
> +#define M_WSAERR          (1<<9)         /* WinSock errors */
> +#endif
> +
> +#ifdef _WIN32
> +#define M_SKERR           (M_ERRNO | M_WSAERR) /* socket errors */
> +#else
> +#define M_SKERR           M_ERRNO
> +#endif
>

The two ifdefs could be combined into one.

The rest looks good to me.


> +
>  #define M_NOMUTE          (1<<11)        /* don't do mute processing */
>  #define M_NOPREFIX        (1<<12)        /* don't show date/time prefix */
>  #define M_USAGE_SMALL     (1<<13)        /* fatal options error, call
> usage_small */
> diff --git a/src/openvpn/manage.c b/src/openvpn/manage.c
> index 496042a6..2fa508ef 100644
> --- a/src/openvpn/manage.c
> +++ b/src/openvpn/manage.c
> @@ -1890,13 +1890,13 @@ man_connect(struct management *man)
>  #if UNIX_SOCK_SUPPORT
>          if (man->settings.flags & MF_UNIX_SOCK)
>          {
> -            msg(D_LINK_ERRORS | M_ERRNO,
> +            msg(D_LINK_ERRORS | M_SKERR,
>                  "MANAGEMENT: connect to unix socket %s failed",
>                  sockaddr_unix_name(&man->settings.local_unix, "NULL"));
>          }
>          else
>  #endif
> -        msg(D_LINK_ERRORS | M_ERRNO,
> +        msg(D_LINK_ERRORS | M_SKERR,
>              "MANAGEMENT: connect to %s failed",
>              print_sockaddr(man->settings.local->ai_addr, &gc));
>          throw_signal_soft(SIGTERM, "management-connect-failed");
> diff --git a/src/openvpn/mtu.c b/src/openvpn/mtu.c
> index aa810f1c..45398953 100644
> --- a/src/openvpn/mtu.c
> +++ b/src/openvpn/mtu.c
> @@ -407,7 +407,7 @@ set_sock_extended_error_passing(int sd)
>      int on = 1;
>      if (setsockopt(sd, SOL_IP, IP_RECVERR, (void *) &on, sizeof(on)))
>      {
> -        msg(M_WARN | M_ERRNO,
> +        msg(M_WARN | M_SKERR,
>              "Note: enable extended error passing on TCP/UDP socket failed
> (IP_RECVERR)");
>      }
>  }
> diff --git a/src/openvpn/platform.c b/src/openvpn/platform.c
> index 450f28ba..1aafd604 100644
> --- a/src/openvpn/platform.c
> +++ b/src/openvpn/platform.c
> @@ -532,7 +532,7 @@ platform_test_file(const char *filename)
>          }
>          else
>          {
> -            if (openvpn_errno() == EACCES)
> +            if (errno == EACCES)
>              {
>                  msg( M_WARN | M_ERRNO, "Could not access file '%s'",
> filename);
>              }
> diff --git a/src/openvpn/proxy.c b/src/openvpn/proxy.c
> index ed720161..409bc4af 100644
> --- a/src/openvpn/proxy.c
> +++ b/src/openvpn/proxy.c
> @@ -110,7 +110,7 @@ recv_line(socket_descriptor_t sd,
>          {
>              if (verbose)
>              {
> -                msg(D_LINK_ERRORS | M_ERRNO, "recv_line: TCP port read
> timeout expired");
> +                msg(D_LINK_ERRORS | M_SKERR, "recv_line: TCP port read
> timeout expired");
>              }
>              goto error;
>          }
> @@ -120,7 +120,7 @@ recv_line(socket_descriptor_t sd,
>          {
>              if (verbose)
>              {
> -                msg(D_LINK_ERRORS | M_ERRNO, "recv_line: TCP port read
> failed on select()");
> +                msg(D_LINK_ERRORS | M_SKERR, "recv_line: TCP port read
> failed on select()");
>              }
>              goto error;
>          }
> @@ -133,7 +133,7 @@ recv_line(socket_descriptor_t sd,
>          {
>              if (verbose)
>              {
> -                msg(D_LINK_ERRORS | M_ERRNO, "recv_line: TCP port read
> failed on recv()");
> +                msg(D_LINK_ERRORS | M_SKERR, "recv_line: TCP port read
> failed on recv()");
>              }
>              goto error;
>          }
> @@ -199,7 +199,7 @@ send_line(socket_descriptor_t sd,
>      const ssize_t size = send(sd, buf, strlen(buf), MSG_NOSIGNAL);
>      if (size != (ssize_t) strlen(buf))
>      {
> -        msg(D_LINK_ERRORS | M_ERRNO, "send_line: TCP port write failed on
> send()");
> +        msg(D_LINK_ERRORS | M_SKERR, "send_line: TCP port write failed on
> send()");
>          return false;
>      }
>      return true;
> diff --git a/src/openvpn/ps.c b/src/openvpn/ps.c
> index 9dca53c8..64cc1ed3 100644
> --- a/src/openvpn/ps.c
> +++ b/src/openvpn/ps.c
> @@ -429,7 +429,7 @@ proxy_entry_new(struct proxy_connection **list,
>      /* connect to port share server */
>      if ((sd_server = socket(PF_INET, SOCK_STREAM, IPPROTO_TCP)) < 0)
>      {
> -        msg(M_WARN|M_ERRNO, "PORT SHARE PROXY: cannot create socket");
> +        msg(M_WARN|M_SKERR, "PORT SHARE PROXY: cannot create socket");
>          return false;
>      }
>      status = openvpn_connect(sd_server,(const struct sockaddr *)
> &server_addr, 5, NULL);
> diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c
> index 0f34a5de..acfcfd65 100644
> --- a/src/openvpn/socket.c
> +++ b/src/openvpn/socket.c
> @@ -1143,7 +1143,7 @@ create_socket(struct link_socket *sock, struct
> addrinfo *addr)
>          msg(M_INFO, "Using bind-dev %s", sock->bind_dev);
>          if (setsockopt(sock->sd, SOL_SOCKET, SO_BINDTODEVICE,
> sock->bind_dev, strlen(sock->bind_dev) + 1) != 0)
>          {
> -            msg(M_WARN|M_ERRNO, "WARN: setsockopt SO_BINDTODEVICE=%s
> failed", sock->bind_dev);
> +            msg(M_WARN|M_SKERR, "WARN: setsockopt SO_BINDTODEVICE=%s
> failed", sock->bind_dev);
>          }
>
>      }
> @@ -1222,7 +1222,7 @@ socket_do_accept(socket_descriptor_t sd,
>
>          if (!socket_defined(new_sd))
>          {
> -            msg(D_LINK_ERRORS | M_ERRNO, "TCP: getpeername() failed");
> +            msg(D_LINK_ERRORS | M_SKERR, "TCP: getpeername() failed");
>          }
>          else
>          {
> @@ -1247,7 +1247,7 @@ socket_do_accept(socket_descriptor_t sd,
>
>      if (!socket_defined(new_sd))
>      {
> -        msg(D_LINK_ERRORS | M_ERRNO, "TCP: accept(%d) failed", (int)sd);
> +        msg(D_LINK_ERRORS | M_SKERR, "TCP: accept(%d) failed", (int)sd);
>      }
>      /* only valid if we have remote_len_af!=0 */
>      else if (remote_len_af && remote_len != remote_len_af)
> @@ -1313,7 +1313,7 @@ socket_listen_accept(socket_descriptor_t sd,
>
>          if (status < 0)
>          {
> -            msg(D_LINK_ERRORS | M_ERRNO, "TCP: select() failed");
> +            msg(D_LINK_ERRORS | M_SKERR, "TCP: select() failed");
>          }
>
>          if (status <= 0)
> @@ -1409,12 +1409,12 @@ socket_bind(socket_descriptor_t sd,
>          msg(M_INFO, "setsockopt(IPV6_V6ONLY=%d)", v6only);
>          if (setsockopt(sd, IPPROTO_IPV6, IPV6_V6ONLY, (void *) &v6only,
> sizeof(v6only)))
>          {
> -            msg(M_NONFATAL|M_ERRNO, "Setting IPV6_V6ONLY=%d failed",
> v6only);
> +            msg(M_NONFATAL | M_SKERR, "Setting IPV6_V6ONLY=%d failed",
> v6only);
>          }
>      }
>      if (bind(sd, cur->ai_addr, cur->ai_addrlen))
>      {
> -        msg(M_FATAL | M_ERRNO, "%s: Socket bind failed on local address
> %s",
> +        msg(M_FATAL | M_SKERR, "%s: Socket bind failed on local address
> %s",
>              prefix,
>              print_sockaddr_ex(local->ai_addr, ":", PS_SHOW_PORT, &gc));
>      }
> @@ -2254,7 +2254,7 @@ link_socket_close(struct link_socket *sock)
>                  msg(D_LOW, "TCP/UDP: Closing socket");
>                  if (openvpn_close_socket(sock->sd))
>                  {
> -                    msg(M_WARN | M_ERRNO, "TCP/UDP: Close Socket failed");
> +                    msg(M_WARN | M_SKERR, "TCP/UDP: Close Socket failed");
>                  }
>              }
>              sock->sd = SOCKET_UNDEFINED;
> @@ -2271,7 +2271,7 @@ link_socket_close(struct link_socket *sock)
>          {
>              if (openvpn_close_socket(sock->ctrl_sd))
>              {
> -                msg(M_WARN | M_ERRNO, "TCP/UDP: Close Socket (ctrl_sd)
> failed");
> +                msg(M_WARN | M_SKERR, "TCP/UDP: Close Socket (ctrl_sd)
> failed");
>              }
>              sock->ctrl_sd = SOCKET_UNDEFINED;
>          }
> @@ -3668,7 +3668,7 @@ sockethandle_finalize(sockethandle_t sh,
>                      /* if no error (i.e. just not finished yet), then
> DON'T execute this code */
>                      io->iostate = IOSTATE_INITIAL;
>                      ASSERT(ResetEvent(io->overlapped.hEvent));
> -                    msg(D_WIN32_IO | M_ERRNO, "WIN32 I/O: Completion
> error");
> +                    msg(D_WIN32_IO | SocketHandleErrorFlag(sh), "WIN32
> I/O: Completion error");
>                  }
>              }
>              break;
> @@ -3681,7 +3681,7 @@ sockethandle_finalize(sockethandle_t sh,
>                  /* error return for a non-queued operation */
>                  SocketHandleSetLastError(sh, io->status);
>                  ret = -1;
> -                msg(D_WIN32_IO | M_ERRNO, "WIN32 I/O: Completion
> non-queued error");
> +                msg(D_WIN32_IO | SocketHandleErrorFlag(sh), "WIN32 I/O:
> Completion non-queued error");
>              }
>              else
>              {
> diff --git a/src/openvpn/socket.h b/src/openvpn/socket.h
> index e9f1524d..a7ec827a 100644
> --- a/src/openvpn/socket.h
> +++ b/src/openvpn/socket.h
> @@ -301,6 +301,12 @@ SocketHandleSetInvalError(sockethandle_t sh)
>      sh.is_handle ? SetLastError(ERROR_INVALID_FUNCTION) :
> WSASetLastError(WSAEINVAL);
>  }
>
> +static inline int
> +SocketHandleErrorFlag(sockethandle_t sh)
> +{
> +    return sh.is_handle ? M_ERRNO : M_SKERR;
> +}
> +
>  #else  /* ifdef _WIN32 */
>
>  #define openvpn_close_socket(s) close(s)
> diff --git a/src/openvpn/socks.c b/src/openvpn/socks.c
> index 768bb613..10107c18 100644
> --- a/src/openvpn/socks.c
> +++ b/src/openvpn/socks.c
> @@ -117,7 +117,7 @@ socks_username_password_auth(struct socks_proxy_info
> *p,
>
>      if (size != strlen(to_send))
>      {
> -        msg(D_LINK_ERRORS | M_ERRNO, "socks_username_password_auth: TCP
> port write failed on send()");
> +        msg(D_LINK_ERRORS | M_SKERR, "socks_username_password_auth: TCP
> port write failed on send()");
>          goto cleanup;
>      }
>
> @@ -145,14 +145,14 @@ socks_username_password_auth(struct socks_proxy_info
> *p,
>          /* timeout? */
>          if (status == 0)
>          {
> -            msg(D_LINK_ERRORS | M_ERRNO, "socks_username_password_auth:
> TCP port read timeout expired");
> +            msg(D_LINK_ERRORS | M_SKERR, "socks_username_password_auth:
> TCP port read timeout expired");
>              goto cleanup;
>          }
>
>          /* error */
>          if (status < 0)
>          {
> -            msg(D_LINK_ERRORS | M_ERRNO, "socks_username_password_auth:
> TCP port read failed on select()");
> +            msg(D_LINK_ERRORS | M_SKERR, "socks_username_password_auth:
> TCP port read failed on select()");
>              goto cleanup;
>          }
>
> @@ -162,7 +162,7 @@ socks_username_password_auth(struct socks_proxy_info
> *p,
>          /* error? */
>          if (size != 1)
>          {
> -            msg(D_LINK_ERRORS | M_ERRNO, "socks_username_password_auth:
> TCP port read failed on recv()");
> +            msg(D_LINK_ERRORS | M_SKERR, "socks_username_password_auth:
> TCP port read failed on recv()");
>              goto cleanup;
>          }
>
> @@ -204,7 +204,7 @@ socks_handshake(struct socks_proxy_info *p,
>      size = send(sd, method_sel, sizeof(method_sel), MSG_NOSIGNAL);
>      if (size != sizeof(method_sel))
>      {
> -        msg(D_LINK_ERRORS | M_ERRNO, "socks_handshake: TCP port write
> failed on send()");
> +        msg(D_LINK_ERRORS | M_SKERR, "socks_handshake: TCP port write
> failed on send()");
>          return false;
>      }
>
> @@ -232,14 +232,14 @@ socks_handshake(struct socks_proxy_info *p,
>          /* timeout? */
>          if (status == 0)
>          {
> -            msg(D_LINK_ERRORS | M_ERRNO, "socks_handshake: TCP port read
> timeout expired");
> +            msg(D_LINK_ERRORS | M_SKERR, "socks_handshake: TCP port read
> timeout expired");
>              return false;
>          }
>
>          /* error */
>          if (status < 0)
>          {
> -            msg(D_LINK_ERRORS | M_ERRNO, "socks_handshake: TCP port read
> failed on select()");
> +            msg(D_LINK_ERRORS | M_SKERR, "socks_handshake: TCP port read
> failed on select()");
>              return false;
>          }
>
> @@ -249,7 +249,7 @@ socks_handshake(struct socks_proxy_info *p,
>          /* error? */
>          if (size != 1)
>          {
> -            msg(D_LINK_ERRORS | M_ERRNO, "socks_handshake: TCP port read
> failed on recv()");
> +            msg(D_LINK_ERRORS | M_SKERR, "socks_handshake: TCP port read
> failed on recv()");
>              return false;
>          }
>
> @@ -342,14 +342,14 @@ recv_socks_reply(socket_descriptor_t sd,
>          /* timeout? */
>          if (status == 0)
>          {
> -            msg(D_LINK_ERRORS | M_ERRNO, "recv_socks_reply: TCP port read
> timeout expired");
> +            msg(D_LINK_ERRORS | M_SKERR, "recv_socks_reply: TCP port read
> timeout expired");
>              return false;
>          }
>
>          /* error */
>          if (status < 0)
>          {
> -            msg(D_LINK_ERRORS | M_ERRNO, "recv_socks_reply: TCP port read
> failed on select()");
> +            msg(D_LINK_ERRORS | M_SKERR, "recv_socks_reply: TCP port read
> failed on select()");
>              return false;
>          }
>
> @@ -359,7 +359,7 @@ recv_socks_reply(socket_descriptor_t sd,
>          /* error? */
>          if (size != 1)
>          {
> -            msg(D_LINK_ERRORS | M_ERRNO, "recv_socks_reply: TCP port read
> failed on recv()");
> +            msg(D_LINK_ERRORS | M_SKERR, "recv_socks_reply: TCP port read
> failed on recv()");
>              return false;
>          }
>
> @@ -484,7 +484,7 @@ establish_socks_proxy_passthru(struct socks_proxy_info
> *p,
>          const ssize_t size = send(sd, buf, 5 + len + 2, MSG_NOSIGNAL);
>          if ((int)size != 5 + (int)len + 2)
>          {
> -            msg(D_LINK_ERRORS | M_ERRNO, "establish_socks_proxy_passthru:
> TCP port write failed on send()");
> +            msg(D_LINK_ERRORS | M_SKERR, "establish_socks_proxy_passthru:
> TCP port write failed on send()");
>              goto error;
>          }
>      }
> @@ -527,7 +527,7 @@ establish_socks_proxy_udpassoc(struct socks_proxy_info
> *p,
>                                    10, MSG_NOSIGNAL);
>          if (size != 10)
>          {
> -            msg(D_LINK_ERRORS | M_ERRNO, "establish_socks_proxy_passthru:
> TCP port write failed on send()");
> +            msg(D_LINK_ERRORS | M_SKERR, "establish_socks_proxy_passthru:
> TCP port write failed on send()");
>              goto error;
>          }
>      }
> --
>

All that said, it may be hard to get all error reporting fixed in one
patch, so we could proceed in steps. First make openvpn_errno() and
openvpn_strerror() do the right thing with GetLastError() vs errno, then
fix WSA error printing, then the rest if any. Just a thought.

Selva
<div dir="ltr"><div>Hi,</div><div><br></div><div>Sorry for the long delay in getting back to this.. </div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Feb 22, 2022 at 9:13 AM Lev Stipakov &lt;<a href="mailto:lstipakov@gmail.com" target="_blank">lstipakov@gmail.com</a>&gt; wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">From: Lev Stipakov &lt;<a href="mailto:lev@openvpn.net" target="_blank">lev@openvpn.net</a>&gt;<br>
<br>
We use M_ERRNO flag in logging to display error code<br>
and error message. This has been broken on Windows,<br>
where we use error code from GetLastError() and<br>
error description from strerror(). strerror() expects<br>
C runtime error code, which is quite different from<br>
last error code from WinAPI call. As a result, we got<br>
incorrect error description.<br>
<br>
The ultimate fix would be introducing another flag<br>
for WinAPI errors, like M_WINERR and use either that or<br>
M_ERRNO depends on context. However, the change would be<br>
quite intrusive and in some cases it is hard to say which<br>
one to use without looking into internals.<br>
<br>
Instead we stick to M_ERRNO and in Windows case we<br>
first try to obtain error code from GetLastError() and<br>
if it returns ERROR_SUCCESS (which is 0), we assume that<br>
we have C runtime error and use errno. To get error<br>
description we use strerror_win32() with GetLastError()<br>
and strerror() with errno.<br>
<br>
strerror_win32() uses FormatMessage() internally, which<br>
is the right way to get WinAPI error description.<br>
<br>
While on it, add a new flag M_SKERR to display socket<br>
error. On Linux it is resolved into M_ERRNO and on Windows<br>
to an conjunction of another new flag M_WSAERR and M_ERRNO.<br>
With M_WSAERR we use WSAGetLastError() to get error code<br>
and strerror_win32() to get error description.<br>
WSAGetLastError() is a correct way to get WinSock error code,<br>
according to documentation.<br>
<br>
Signed-off-by: Lev Stipakov &lt;<a href="mailto:lev@openvpn.net" target="_blank">lev@openvpn.net</a>&gt;<br>
---<br>
 src/openvpn/error.c    | 26 ++++++++++++++++++++++----<br>
 src/openvpn/error.h    | 11 ++++++++++-<br>
 src/openvpn/manage.c   |  4 ++--<br>
 src/openvpn/mtu.c      |  2 +-<br>
 src/openvpn/platform.c |  2 +-<br>
 src/openvpn/proxy.c    |  8 ++++----<br>
 src/openvpn/ps.c       |  2 +-<br>
 src/openvpn/socket.c   | 20 ++++++++++----------<br>
 src/openvpn/socket.h   |  6 ++++++<br>
 src/openvpn/socks.c    | 26 +++++++++++++-------------<br>
 10 files changed, 70 insertions(+), 37 deletions(-)<br>
<br>
diff --git a/src/openvpn/error.c b/src/openvpn/error.c<br>
index b0e9a48c..5da588d9 100644<br>
--- a/src/openvpn/error.c<br>
+++ b/src/openvpn/error.c<br>
@@ -242,7 +242,25 @@ x_msg_va(const unsigned int flags, const char *format, va_list arglist)<br>
         return;<br>
     }<br>
<br>
-    e = openvpn_errno();<br>
+    bool use_strerr = false;<br>
+#ifdef _WIN32<br>
+    if (flags &amp; M_WSAERR)<br>
+    {<br>
+        e = WSAGetLastError();<br>
+    }<br>
+    else<br>
+    {<br>
+        e = GetLastError();<br>
+        if (e == ERROR_SUCCESS)<br>
+        {<br>
+            /* error is likely C runtime, fallback to errno/strerr */<br>
+            use_strerr = true;<br>
+            e = errno;<br>
+        }<br>
+    }<br>
+#else<br>
+    e = errno;<br>
+#endif<br>
<br></blockquote><div><br></div><div>Ideally I would have liked to modify openvpn_errno() and openvpn_strerror()  to do the right thing instead of adding this logic here and below. I have not worked out what exactly that would entail --  most likely those two functions would require an extra argument to keep track of the kind of error.</div><div><br></div><div>Also there are some places openvpn_errno() is used and the result checked against POSIX error codes:</div><div><div>in forward.c around line 2102:</div><div><br></div><div>if (.... &amp;&amp; ENETUNREACH == error_code &amp;&amp; ...)</div><div><br></div><div>where error_code = openvpn_errno() which may return WSAENETUNREACH on Windows not ENETUNREACH-- even that depends on GetLastError() and WSAGetLastError() to behave identically.</div></div><div><br></div><div>Also does code like the below in socket.c do the right thing?</div><div>(around line 1451 in socket.c)</div><div>status = openvpn_errno();  </div><div>if (status == WSAEWOULDBLOCK) ....</div><div>but openvpn_errono() doesn&#39;t look for WSA errors (though it probably still works?).</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
     /*<br>
      * Apply muting filter.<br>
@@ -264,7 +282,7 @@ x_msg_va(const unsigned int flags, const char *format, va_list arglist)<br>
     if ((flags &amp; M_ERRNO) &amp;&amp; e)<br>
     {<br>
         openvpn_snprintf(m2, ERR_BUF_SIZE, &quot;%s: %s (errno=%d)&quot;,<br>
-                         m1, strerror(e), e);<br>
+                         m1, use_strerr ? strerror(e) : openvpn_strerror(e, &amp;gc), e);<br>
         SWAP;<br>
     }<br>
<br>
@@ -678,13 +696,13 @@ x_check_status(int status,<br>
             {<br>
                 msg(x_cs_info_level, &quot;%s %s [%s]: %s (fd=%d,code=%d)&quot;, description,<br>
                     sock ? proto2ascii(sock-&gt;info.proto, sock-&gt;<a href="http://info.af" rel="noreferrer" target="_blank">info.af</a>, true) : &quot;&quot;,<br>
-                    extended_msg, strerror(my_errno), sock ? sock-&gt;sd : -1, my_errno);<br>
+                    extended_msg, openvpn_strerror(my_errno, &amp;gc), sock ? sock-&gt;sd : -1, my_errno);<br></blockquote><div><br></div><div>Here my_errno = openvpn_errno() -- don&#39;t we need the fallback to POSIX errno in this case? Could this also get called when a socket error is expected?</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
             }<br>
             else<br>
             {<br>
                 msg(x_cs_info_level, &quot;%s %s: %s (fd=%d,code=%d)&quot;, description,<br>
                     sock ? proto2ascii(sock-&gt;info.proto, sock-&gt;<a href="http://info.af" rel="noreferrer" target="_blank">info.af</a>, true) : &quot;&quot;,<br>
-                    strerror(my_errno), sock ? sock-&gt;sd : -1, my_errno);<br>
+                    openvpn_strerror(my_errno, &amp;gc), sock ? sock-&gt;sd : -1, my_errno);<br>
             }<br>
<br>
             if (x_cs_err_delay_ms)<br>
diff --git a/src/openvpn/error.h b/src/openvpn/error.h<br>
index ad7defe8..9c3f4dfb 100644<br>
--- a/src/openvpn/error.h<br>
+++ b/src/openvpn/error.h<br>
@@ -99,9 +99,18 @@ extern int x_msg_line_num;<br>
 #define M_NONFATAL        (1&lt;&lt;5)         /* non-fatal error */<br>
 #define M_WARN            (1&lt;&lt;6)         /* call syslog with LOG_WARNING */<br>
 #define M_DEBUG           (1&lt;&lt;7)<br>
-<br>
 #define M_ERRNO           (1&lt;&lt;8)         /* show errno description */<br>
<br>
+#ifdef _WIN32<br>
+#define M_WSAERR          (1&lt;&lt;9)         /* WinSock errors */<br>
+#endif<br>
+<br>
+#ifdef _WIN32<br>
+#define M_SKERR           (M_ERRNO | M_WSAERR) /* socket errors */<br>
+#else<br>
+#define M_SKERR           M_ERRNO<br>
+#endif<br></blockquote><div><br></div><div>The two ifdefs could be combined into one.</div><div><br></div><div>The rest looks good to me.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+<br>
 #define M_NOMUTE          (1&lt;&lt;11)        /* don&#39;t do mute processing */<br>
 #define M_NOPREFIX        (1&lt;&lt;12)        /* don&#39;t show date/time prefix */<br>
 #define M_USAGE_SMALL     (1&lt;&lt;13)        /* fatal options error, call usage_small */<br>
diff --git a/src/openvpn/manage.c b/src/openvpn/manage.c<br>
index 496042a6..2fa508ef 100644<br>
--- a/src/openvpn/manage.c<br>
+++ b/src/openvpn/manage.c<br>
@@ -1890,13 +1890,13 @@ man_connect(struct management *man)<br>
 #if UNIX_SOCK_SUPPORT<br>
         if (man-&gt;settings.flags &amp; MF_UNIX_SOCK)<br>
         {<br>
-            msg(D_LINK_ERRORS | M_ERRNO,<br>
+            msg(D_LINK_ERRORS | M_SKERR,<br>
                 &quot;MANAGEMENT: connect to unix socket %s failed&quot;,<br>
                 sockaddr_unix_name(&amp;man-&gt;settings.local_unix, &quot;NULL&quot;));<br>
         }<br>
         else<br>
 #endif<br>
-        msg(D_LINK_ERRORS | M_ERRNO,<br>
+        msg(D_LINK_ERRORS | M_SKERR,<br>
             &quot;MANAGEMENT: connect to %s failed&quot;,<br>
             print_sockaddr(man-&gt;settings.local-&gt;ai_addr, &amp;gc));<br>
         throw_signal_soft(SIGTERM, &quot;management-connect-failed&quot;);<br>
diff --git a/src/openvpn/mtu.c b/src/openvpn/mtu.c<br>
index aa810f1c..45398953 100644<br>
--- a/src/openvpn/mtu.c<br>
+++ b/src/openvpn/mtu.c<br>
@@ -407,7 +407,7 @@ set_sock_extended_error_passing(int sd)<br>
     int on = 1;<br>
     if (setsockopt(sd, SOL_IP, IP_RECVERR, (void *) &amp;on, sizeof(on)))<br>
     {<br>
-        msg(M_WARN | M_ERRNO,<br>
+        msg(M_WARN | M_SKERR,<br>
             &quot;Note: enable extended error passing on TCP/UDP socket failed (IP_RECVERR)&quot;);<br>
     }<br>
 }<br>
diff --git a/src/openvpn/platform.c b/src/openvpn/platform.c<br>
index 450f28ba..1aafd604 100644<br>
--- a/src/openvpn/platform.c<br>
+++ b/src/openvpn/platform.c<br>
@@ -532,7 +532,7 @@ platform_test_file(const char *filename)<br>
         }<br>
         else<br>
         {<br>
-            if (openvpn_errno() == EACCES)<br>
+            if (errno == EACCES)<br>
             {<br>
                 msg( M_WARN | M_ERRNO, &quot;Could not access file &#39;%s&#39;&quot;, filename);<br>
             }<br>
diff --git a/src/openvpn/proxy.c b/src/openvpn/proxy.c<br>
index ed720161..409bc4af 100644<br>
--- a/src/openvpn/proxy.c<br>
+++ b/src/openvpn/proxy.c<br>
@@ -110,7 +110,7 @@ recv_line(socket_descriptor_t sd,<br>
         {<br>
             if (verbose)<br>
             {<br>
-                msg(D_LINK_ERRORS | M_ERRNO, &quot;recv_line: TCP port read timeout expired&quot;);<br>
+                msg(D_LINK_ERRORS | M_SKERR, &quot;recv_line: TCP port read timeout expired&quot;);<br>
             }<br>
             goto error;<br>
         }<br>
@@ -120,7 +120,7 @@ recv_line(socket_descriptor_t sd,<br>
         {<br>
             if (verbose)<br>
             {<br>
-                msg(D_LINK_ERRORS | M_ERRNO, &quot;recv_line: TCP port read failed on select()&quot;);<br>
+                msg(D_LINK_ERRORS | M_SKERR, &quot;recv_line: TCP port read failed on select()&quot;);<br>
             }<br>
             goto error;<br>
         }<br>
@@ -133,7 +133,7 @@ recv_line(socket_descriptor_t sd,<br>
         {<br>
             if (verbose)<br>
             {<br>
-                msg(D_LINK_ERRORS | M_ERRNO, &quot;recv_line: TCP port read failed on recv()&quot;);<br>
+                msg(D_LINK_ERRORS | M_SKERR, &quot;recv_line: TCP port read failed on recv()&quot;);<br>
             }<br>
             goto error;<br>
         }<br>
@@ -199,7 +199,7 @@ send_line(socket_descriptor_t sd,<br>
     const ssize_t size = send(sd, buf, strlen(buf), MSG_NOSIGNAL);<br>
     if (size != (ssize_t) strlen(buf))<br>
     {<br>
-        msg(D_LINK_ERRORS | M_ERRNO, &quot;send_line: TCP port write failed on send()&quot;);<br>
+        msg(D_LINK_ERRORS | M_SKERR, &quot;send_line: TCP port write failed on send()&quot;);<br>
         return false;<br>
     }<br>
     return true;<br>
diff --git a/src/openvpn/ps.c b/src/openvpn/ps.c<br>
index 9dca53c8..64cc1ed3 100644<br>
--- a/src/openvpn/ps.c<br>
+++ b/src/openvpn/ps.c<br>
@@ -429,7 +429,7 @@ proxy_entry_new(struct proxy_connection **list,<br>
     /* connect to port share server */<br>
     if ((sd_server = socket(PF_INET, SOCK_STREAM, IPPROTO_TCP)) &lt; 0)<br>
     {<br>
-        msg(M_WARN|M_ERRNO, &quot;PORT SHARE PROXY: cannot create socket&quot;);<br>
+        msg(M_WARN|M_SKERR, &quot;PORT SHARE PROXY: cannot create socket&quot;);<br>
         return false;<br>
     }<br>
     status = openvpn_connect(sd_server,(const struct sockaddr *)  &amp;server_addr, 5, NULL);<br>
diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c<br>
index 0f34a5de..acfcfd65 100644<br>
--- a/src/openvpn/socket.c<br>
+++ b/src/openvpn/socket.c<br>
@@ -1143,7 +1143,7 @@ create_socket(struct link_socket *sock, struct addrinfo *addr)<br>
         msg(M_INFO, &quot;Using bind-dev %s&quot;, sock-&gt;bind_dev);<br>
         if (setsockopt(sock-&gt;sd, SOL_SOCKET, SO_BINDTODEVICE, sock-&gt;bind_dev, strlen(sock-&gt;bind_dev) + 1) != 0)<br>
         {<br>
-            msg(M_WARN|M_ERRNO, &quot;WARN: setsockopt SO_BINDTODEVICE=%s failed&quot;, sock-&gt;bind_dev);<br>
+            msg(M_WARN|M_SKERR, &quot;WARN: setsockopt SO_BINDTODEVICE=%s failed&quot;, sock-&gt;bind_dev);<br>
         }<br>
<br>
     }<br>
@@ -1222,7 +1222,7 @@ socket_do_accept(socket_descriptor_t sd,<br>
<br>
         if (!socket_defined(new_sd))<br>
         {<br>
-            msg(D_LINK_ERRORS | M_ERRNO, &quot;TCP: getpeername() failed&quot;);<br>
+            msg(D_LINK_ERRORS | M_SKERR, &quot;TCP: getpeername() failed&quot;);<br>
         }<br>
         else<br>
         {<br>
@@ -1247,7 +1247,7 @@ socket_do_accept(socket_descriptor_t sd,<br>
<br>
     if (!socket_defined(new_sd))<br>
     {<br>
-        msg(D_LINK_ERRORS | M_ERRNO, &quot;TCP: accept(%d) failed&quot;, (int)sd);<br>
+        msg(D_LINK_ERRORS | M_SKERR, &quot;TCP: accept(%d) failed&quot;, (int)sd);<br>
     }<br>
     /* only valid if we have remote_len_af!=0 */<br>
     else if (remote_len_af &amp;&amp; remote_len != remote_len_af)<br>
@@ -1313,7 +1313,7 @@ socket_listen_accept(socket_descriptor_t sd,<br>
<br>
         if (status &lt; 0)<br>
         {<br>
-            msg(D_LINK_ERRORS | M_ERRNO, &quot;TCP: select() failed&quot;);<br>
+            msg(D_LINK_ERRORS | M_SKERR, &quot;TCP: select() failed&quot;);<br>
         }<br>
<br>
         if (status &lt;= 0)<br>
@@ -1409,12 +1409,12 @@ socket_bind(socket_descriptor_t sd,<br>
         msg(M_INFO, &quot;setsockopt(IPV6_V6ONLY=%d)&quot;, v6only);<br>
         if (setsockopt(sd, IPPROTO_IPV6, IPV6_V6ONLY, (void *) &amp;v6only, sizeof(v6only)))<br>
         {<br>
-            msg(M_NONFATAL|M_ERRNO, &quot;Setting IPV6_V6ONLY=%d failed&quot;, v6only);<br>
+            msg(M_NONFATAL | M_SKERR, &quot;Setting IPV6_V6ONLY=%d failed&quot;, v6only);<br>
         }<br>
     }<br>
     if (bind(sd, cur-&gt;ai_addr, cur-&gt;ai_addrlen))<br>
     {<br>
-        msg(M_FATAL | M_ERRNO, &quot;%s: Socket bind failed on local address %s&quot;,<br>
+        msg(M_FATAL | M_SKERR, &quot;%s: Socket bind failed on local address %s&quot;,<br>
             prefix,<br>
             print_sockaddr_ex(local-&gt;ai_addr, &quot;:&quot;, PS_SHOW_PORT, &amp;gc));<br>
     }<br>
@@ -2254,7 +2254,7 @@ link_socket_close(struct link_socket *sock)<br>
                 msg(D_LOW, &quot;TCP/UDP: Closing socket&quot;);<br>
                 if (openvpn_close_socket(sock-&gt;sd))<br>
                 {<br>
-                    msg(M_WARN | M_ERRNO, &quot;TCP/UDP: Close Socket failed&quot;);<br>
+                    msg(M_WARN | M_SKERR, &quot;TCP/UDP: Close Socket failed&quot;);<br>
                 }<br>
             }<br>
             sock-&gt;sd = SOCKET_UNDEFINED;<br>
@@ -2271,7 +2271,7 @@ link_socket_close(struct link_socket *sock)<br>
         {<br>
             if (openvpn_close_socket(sock-&gt;ctrl_sd))<br>
             {<br>
-                msg(M_WARN | M_ERRNO, &quot;TCP/UDP: Close Socket (ctrl_sd) failed&quot;);<br>
+                msg(M_WARN | M_SKERR, &quot;TCP/UDP: Close Socket (ctrl_sd) failed&quot;);<br>
             }<br>
             sock-&gt;ctrl_sd = SOCKET_UNDEFINED;<br>
         }<br>
@@ -3668,7 +3668,7 @@ sockethandle_finalize(sockethandle_t sh,<br>
                     /* if no error (i.e. just not finished yet), then DON&#39;T execute this code */<br>
                     io-&gt;iostate = IOSTATE_INITIAL;<br>
                     ASSERT(ResetEvent(io-&gt;overlapped.hEvent));<br>
-                    msg(D_WIN32_IO | M_ERRNO, &quot;WIN32 I/O: Completion error&quot;);<br>
+                    msg(D_WIN32_IO | SocketHandleErrorFlag(sh), &quot;WIN32 I/O: Completion error&quot;);<br>
                 }<br>
             }<br>
             break;<br>
@@ -3681,7 +3681,7 @@ sockethandle_finalize(sockethandle_t sh,<br>
                 /* error return for a non-queued operation */<br>
                 SocketHandleSetLastError(sh, io-&gt;status);<br>
                 ret = -1;<br>
-                msg(D_WIN32_IO | M_ERRNO, &quot;WIN32 I/O: Completion non-queued error&quot;);<br>
+                msg(D_WIN32_IO | SocketHandleErrorFlag(sh), &quot;WIN32 I/O: Completion non-queued error&quot;);<br>
             }<br>
             else<br>
             {<br>
diff --git a/src/openvpn/socket.h b/src/openvpn/socket.h<br>
index e9f1524d..a7ec827a 100644<br>
--- a/src/openvpn/socket.h<br>
+++ b/src/openvpn/socket.h<br>
@@ -301,6 +301,12 @@ SocketHandleSetInvalError(sockethandle_t sh)<br>
     sh.is_handle ? SetLastError(ERROR_INVALID_FUNCTION) : WSASetLastError(WSAEINVAL);<br>
 }<br>
<br>
+static inline int<br>
+SocketHandleErrorFlag(sockethandle_t sh)<br>
+{<br>
+    return sh.is_handle ? M_ERRNO : M_SKERR;<br>
+}<br>
+<br>
 #else  /* ifdef _WIN32 */<br>
<br>
 #define openvpn_close_socket(s) close(s)<br>
diff --git a/src/openvpn/socks.c b/src/openvpn/socks.c<br>
index 768bb613..10107c18 100644<br>
--- a/src/openvpn/socks.c<br>
+++ b/src/openvpn/socks.c<br>
@@ -117,7 +117,7 @@ socks_username_password_auth(struct socks_proxy_info *p,<br>
<br>
     if (size != strlen(to_send))<br>
     {<br>
-        msg(D_LINK_ERRORS | M_ERRNO, &quot;socks_username_password_auth: TCP port write failed on send()&quot;);<br>
+        msg(D_LINK_ERRORS | M_SKERR, &quot;socks_username_password_auth: TCP port write failed on send()&quot;);<br>
         goto cleanup;<br>
     }<br>
<br>
@@ -145,14 +145,14 @@ socks_username_password_auth(struct socks_proxy_info *p,<br>
         /* timeout? */<br>
         if (status == 0)<br>
         {<br>
-            msg(D_LINK_ERRORS | M_ERRNO, &quot;socks_username_password_auth: TCP port read timeout expired&quot;);<br>
+            msg(D_LINK_ERRORS | M_SKERR, &quot;socks_username_password_auth: TCP port read timeout expired&quot;);<br>
             goto cleanup;<br>
         }<br>
<br>
         /* error */<br>
         if (status &lt; 0)<br>
         {<br>
-            msg(D_LINK_ERRORS | M_ERRNO, &quot;socks_username_password_auth: TCP port read failed on select()&quot;);<br>
+            msg(D_LINK_ERRORS | M_SKERR, &quot;socks_username_password_auth: TCP port read failed on select()&quot;);<br>
             goto cleanup;<br>
         }<br>
<br>
@@ -162,7 +162,7 @@ socks_username_password_auth(struct socks_proxy_info *p,<br>
         /* error? */<br>
         if (size != 1)<br>
         {<br>
-            msg(D_LINK_ERRORS | M_ERRNO, &quot;socks_username_password_auth: TCP port read failed on recv()&quot;);<br>
+            msg(D_LINK_ERRORS | M_SKERR, &quot;socks_username_password_auth: TCP port read failed on recv()&quot;);<br>
             goto cleanup;<br>
         }<br>
<br>
@@ -204,7 +204,7 @@ socks_handshake(struct socks_proxy_info *p,<br>
     size = send(sd, method_sel, sizeof(method_sel), MSG_NOSIGNAL);<br>
     if (size != sizeof(method_sel))<br>
     {<br>
-        msg(D_LINK_ERRORS | M_ERRNO, &quot;socks_handshake: TCP port write failed on send()&quot;);<br>
+        msg(D_LINK_ERRORS | M_SKERR, &quot;socks_handshake: TCP port write failed on send()&quot;);<br>
         return false;<br>
     }<br>
<br>
@@ -232,14 +232,14 @@ socks_handshake(struct socks_proxy_info *p,<br>
         /* timeout? */<br>
         if (status == 0)<br>
         {<br>
-            msg(D_LINK_ERRORS | M_ERRNO, &quot;socks_handshake: TCP port read timeout expired&quot;);<br>
+            msg(D_LINK_ERRORS | M_SKERR, &quot;socks_handshake: TCP port read timeout expired&quot;);<br>
             return false;<br>
         }<br>
<br>
         /* error */<br>
         if (status &lt; 0)<br>
         {<br>
-            msg(D_LINK_ERRORS | M_ERRNO, &quot;socks_handshake: TCP port read failed on select()&quot;);<br>
+            msg(D_LINK_ERRORS | M_SKERR, &quot;socks_handshake: TCP port read failed on select()&quot;);<br>
             return false;<br>
         }<br>
<br>
@@ -249,7 +249,7 @@ socks_handshake(struct socks_proxy_info *p,<br>
         /* error? */<br>
         if (size != 1)<br>
         {<br>
-            msg(D_LINK_ERRORS | M_ERRNO, &quot;socks_handshake: TCP port read failed on recv()&quot;);<br>
+            msg(D_LINK_ERRORS | M_SKERR, &quot;socks_handshake: TCP port read failed on recv()&quot;);<br>
             return false;<br>
         }<br>
<br>
@@ -342,14 +342,14 @@ recv_socks_reply(socket_descriptor_t sd,<br>
         /* timeout? */<br>
         if (status == 0)<br>
         {<br>
-            msg(D_LINK_ERRORS | M_ERRNO, &quot;recv_socks_reply: TCP port read timeout expired&quot;);<br>
+            msg(D_LINK_ERRORS | M_SKERR, &quot;recv_socks_reply: TCP port read timeout expired&quot;);<br>
             return false;<br>
         }<br>
<br>
         /* error */<br>
         if (status &lt; 0)<br>
         {<br>
-            msg(D_LINK_ERRORS | M_ERRNO, &quot;recv_socks_reply: TCP port read failed on select()&quot;);<br>
+            msg(D_LINK_ERRORS | M_SKERR, &quot;recv_socks_reply: TCP port read failed on select()&quot;);<br>
             return false;<br>
         }<br>
<br>
@@ -359,7 +359,7 @@ recv_socks_reply(socket_descriptor_t sd,<br>
         /* error? */<br>
         if (size != 1)<br>
         {<br>
-            msg(D_LINK_ERRORS | M_ERRNO, &quot;recv_socks_reply: TCP port read failed on recv()&quot;);<br>
+            msg(D_LINK_ERRORS | M_SKERR, &quot;recv_socks_reply: TCP port read failed on recv()&quot;);<br>
             return false;<br>
         }<br>
<br>
@@ -484,7 +484,7 @@ establish_socks_proxy_passthru(struct socks_proxy_info *p,<br>
         const ssize_t size = send(sd, buf, 5 + len + 2, MSG_NOSIGNAL);<br>
         if ((int)size != 5 + (int)len + 2)<br>
         {<br>
-            msg(D_LINK_ERRORS | M_ERRNO, &quot;establish_socks_proxy_passthru: TCP port write failed on send()&quot;);<br>
+            msg(D_LINK_ERRORS | M_SKERR, &quot;establish_socks_proxy_passthru: TCP port write failed on send()&quot;);<br>
             goto error;<br>
         }<br>
     }<br>
@@ -527,7 +527,7 @@ establish_socks_proxy_udpassoc(struct socks_proxy_info *p,<br>
                                   10, MSG_NOSIGNAL);<br>
         if (size != 10)<br>
         {<br>
-            msg(D_LINK_ERRORS | M_ERRNO, &quot;establish_socks_proxy_passthru: TCP port write failed on send()&quot;);<br>
+            msg(D_LINK_ERRORS | M_SKERR, &quot;establish_socks_proxy_passthru: TCP port write failed on send()&quot;);<br>
             goto error;<br>
         }<br>
     }<br>
-- <br>
</blockquote><div><br></div><div>All that said, it may be hard to get all error reporting fixed in one patch, so we could proceed in steps. First make openvpn_errno() and openvpn_strerror() do the right thing with GetLastError() vs errno, then fix WSA error printing, then the rest if any. Just a thought.</div><div><br></div><div>Selva</div></div></div>
Lev Stipakov May 3, 2022, 1:01 p.m. | #2
Hi,

Also there are some places openvpn_errno() is used and the result checked
> against POSIX error codes:
> in forward.c around line 2102:
>
> if (.... && ENETUNREACH == error_code && ...)
>
> where error_code = openvpn_errno() which may return WSAENETUNREACH on
> Windows not ENETUNREACH-- even that depends on GetLastError() and
> WSAGetLastError() to behave identically.
>

Thanks, fixed.


> Also does code like the below in socket.c do the right thing?
> (around line 1451 in socket.c)
> status = openvpn_errno();
> if (status == WSAEWOULDBLOCK) ....
> but openvpn_errono() doesn't look for WSA errors (though it probably still
> works?).
>

Yeah we compare GetLastError result here with WSA*, but in practice that
works. Here we just call openvpn_errno(), we could add WSA* support but
this will make code less readable for the benefit of being more "pure", not
sure it is worth it.


> -                    extended_msg, strerror(my_errno), sock ? sock->sd :
>> -1, my_errno);
>> +                    extended_msg, openvpn_strerror(my_errno, &gc), sock
>> ? sock->sd : -1, my_errno);
>>
>
> Here my_errno = openvpn_errno() -- don't we need the fallback to POSIX
> errno in this case? Could this also get called when a socket error is
> expected?
>

You are right. I slightly refactored that part to reuse crt error fallback
code which we need in 3 different places.

The rest looks good to me.
>

As you proposed, I removed WSA error printing from this patch, it could
come in a follow-up.

V2 is coming.
<div dir="ltr"><div dir="ltr">Hi,</div><div dir="ltr"><br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div>Also there are some places openvpn_errno() is used and the result checked against POSIX error codes:</div><div><div>in forward.c around line 2102:</div><div><br></div><div>if (.... &amp;&amp; ENETUNREACH == error_code &amp;&amp; ...)</div><div><br></div><div>where error_code = openvpn_errno() which may return WSAENETUNREACH on Windows not ENETUNREACH-- even that depends on GetLastError() and WSAGetLastError() to behave identically.</div></div></div></div></blockquote><div><br></div><div>Thanks, fixed.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div>Also does code like the below in socket.c do the right thing?</div><div>(around line 1451 in socket.c)</div><div>status = openvpn_errno();  </div><div>if (status == WSAEWOULDBLOCK) ....</div><div>but openvpn_errono() doesn&#39;t look for WSA errors (though it probably still works?).</div></div></div></blockquote><div><br></div><div>Yeah we compare GetLastError result here with WSA*, but in practice that works. Here we just call openvpn_errno(), we could add WSA* support but this will make code less readable for the benefit of being more &quot;pure&quot;, not sure it is worth it.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
-                    extended_msg, strerror(my_errno), sock ? sock-&gt;sd : -1, my_errno);<br>
+                    extended_msg, openvpn_strerror(my_errno, &amp;gc), sock ? sock-&gt;sd : -1, my_errno);<br></blockquote><div><br></div><div>Here my_errno = openvpn_errno() -- don&#39;t we need the fallback to POSIX errno in this case? Could this also get called when a socket error is expected?</div></div></div></blockquote><div><br></div><div>You are right. I slightly refactored that part to reuse crt error fallback code which we need in 3 different places.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><div>The rest looks good to me.</div></div></div></blockquote><div><br></div><div>As you proposed, I removed WSA error printing from this patch, it could come in a follow-up.</div><div> <br></div><div>V2 is coming.</div></div></div>

Patch

diff --git a/src/openvpn/error.c b/src/openvpn/error.c
index b0e9a48c..5da588d9 100644
--- a/src/openvpn/error.c
+++ b/src/openvpn/error.c
@@ -242,7 +242,25 @@  x_msg_va(const unsigned int flags, const char *format, va_list arglist)
         return;
     }
 
-    e = openvpn_errno();
+    bool use_strerr = false;
+#ifdef _WIN32
+    if (flags & M_WSAERR)
+    {
+        e = WSAGetLastError();
+    }
+    else
+    {
+        e = GetLastError();
+        if (e == ERROR_SUCCESS)
+        {
+            /* error is likely C runtime, fallback to errno/strerr */
+            use_strerr = true;
+            e = errno;
+        }
+    }
+#else
+    e = errno;
+#endif
 
     /*
      * Apply muting filter.
@@ -264,7 +282,7 @@  x_msg_va(const unsigned int flags, const char *format, va_list arglist)
     if ((flags & M_ERRNO) && e)
     {
         openvpn_snprintf(m2, ERR_BUF_SIZE, "%s: %s (errno=%d)",
-                         m1, strerror(e), e);
+                         m1, use_strerr ? strerror(e) : openvpn_strerror(e, &gc), e);
         SWAP;
     }
 
@@ -678,13 +696,13 @@  x_check_status(int status,
             {
                 msg(x_cs_info_level, "%s %s [%s]: %s (fd=%d,code=%d)", description,
                     sock ? proto2ascii(sock->info.proto, sock->info.af, true) : "",
-                    extended_msg, strerror(my_errno), sock ? sock->sd : -1, my_errno);
+                    extended_msg, openvpn_strerror(my_errno, &gc), sock ? sock->sd : -1, my_errno);
             }
             else
             {
                 msg(x_cs_info_level, "%s %s: %s (fd=%d,code=%d)", description,
                     sock ? proto2ascii(sock->info.proto, sock->info.af, true) : "",
-                    strerror(my_errno), sock ? sock->sd : -1, my_errno);
+                    openvpn_strerror(my_errno, &gc), sock ? sock->sd : -1, my_errno);
             }
 
             if (x_cs_err_delay_ms)
diff --git a/src/openvpn/error.h b/src/openvpn/error.h
index ad7defe8..9c3f4dfb 100644
--- a/src/openvpn/error.h
+++ b/src/openvpn/error.h
@@ -99,9 +99,18 @@  extern int x_msg_line_num;
 #define M_NONFATAL        (1<<5)         /* non-fatal error */
 #define M_WARN            (1<<6)         /* call syslog with LOG_WARNING */
 #define M_DEBUG           (1<<7)
-
 #define M_ERRNO           (1<<8)         /* show errno description */
 
+#ifdef _WIN32
+#define M_WSAERR          (1<<9)         /* WinSock errors */
+#endif
+
+#ifdef _WIN32
+#define M_SKERR           (M_ERRNO | M_WSAERR) /* socket errors */
+#else
+#define M_SKERR           M_ERRNO
+#endif
+
 #define M_NOMUTE          (1<<11)        /* don't do mute processing */
 #define M_NOPREFIX        (1<<12)        /* don't show date/time prefix */
 #define M_USAGE_SMALL     (1<<13)        /* fatal options error, call usage_small */
diff --git a/src/openvpn/manage.c b/src/openvpn/manage.c
index 496042a6..2fa508ef 100644
--- a/src/openvpn/manage.c
+++ b/src/openvpn/manage.c
@@ -1890,13 +1890,13 @@  man_connect(struct management *man)
 #if UNIX_SOCK_SUPPORT
         if (man->settings.flags & MF_UNIX_SOCK)
         {
-            msg(D_LINK_ERRORS | M_ERRNO,
+            msg(D_LINK_ERRORS | M_SKERR,
                 "MANAGEMENT: connect to unix socket %s failed",
                 sockaddr_unix_name(&man->settings.local_unix, "NULL"));
         }
         else
 #endif
-        msg(D_LINK_ERRORS | M_ERRNO,
+        msg(D_LINK_ERRORS | M_SKERR,
             "MANAGEMENT: connect to %s failed",
             print_sockaddr(man->settings.local->ai_addr, &gc));
         throw_signal_soft(SIGTERM, "management-connect-failed");
diff --git a/src/openvpn/mtu.c b/src/openvpn/mtu.c
index aa810f1c..45398953 100644
--- a/src/openvpn/mtu.c
+++ b/src/openvpn/mtu.c
@@ -407,7 +407,7 @@  set_sock_extended_error_passing(int sd)
     int on = 1;
     if (setsockopt(sd, SOL_IP, IP_RECVERR, (void *) &on, sizeof(on)))
     {
-        msg(M_WARN | M_ERRNO,
+        msg(M_WARN | M_SKERR,
             "Note: enable extended error passing on TCP/UDP socket failed (IP_RECVERR)");
     }
 }
diff --git a/src/openvpn/platform.c b/src/openvpn/platform.c
index 450f28ba..1aafd604 100644
--- a/src/openvpn/platform.c
+++ b/src/openvpn/platform.c
@@ -532,7 +532,7 @@  platform_test_file(const char *filename)
         }
         else
         {
-            if (openvpn_errno() == EACCES)
+            if (errno == EACCES)
             {
                 msg( M_WARN | M_ERRNO, "Could not access file '%s'", filename);
             }
diff --git a/src/openvpn/proxy.c b/src/openvpn/proxy.c
index ed720161..409bc4af 100644
--- a/src/openvpn/proxy.c
+++ b/src/openvpn/proxy.c
@@ -110,7 +110,7 @@  recv_line(socket_descriptor_t sd,
         {
             if (verbose)
             {
-                msg(D_LINK_ERRORS | M_ERRNO, "recv_line: TCP port read timeout expired");
+                msg(D_LINK_ERRORS | M_SKERR, "recv_line: TCP port read timeout expired");
             }
             goto error;
         }
@@ -120,7 +120,7 @@  recv_line(socket_descriptor_t sd,
         {
             if (verbose)
             {
-                msg(D_LINK_ERRORS | M_ERRNO, "recv_line: TCP port read failed on select()");
+                msg(D_LINK_ERRORS | M_SKERR, "recv_line: TCP port read failed on select()");
             }
             goto error;
         }
@@ -133,7 +133,7 @@  recv_line(socket_descriptor_t sd,
         {
             if (verbose)
             {
-                msg(D_LINK_ERRORS | M_ERRNO, "recv_line: TCP port read failed on recv()");
+                msg(D_LINK_ERRORS | M_SKERR, "recv_line: TCP port read failed on recv()");
             }
             goto error;
         }
@@ -199,7 +199,7 @@  send_line(socket_descriptor_t sd,
     const ssize_t size = send(sd, buf, strlen(buf), MSG_NOSIGNAL);
     if (size != (ssize_t) strlen(buf))
     {
-        msg(D_LINK_ERRORS | M_ERRNO, "send_line: TCP port write failed on send()");
+        msg(D_LINK_ERRORS | M_SKERR, "send_line: TCP port write failed on send()");
         return false;
     }
     return true;
diff --git a/src/openvpn/ps.c b/src/openvpn/ps.c
index 9dca53c8..64cc1ed3 100644
--- a/src/openvpn/ps.c
+++ b/src/openvpn/ps.c
@@ -429,7 +429,7 @@  proxy_entry_new(struct proxy_connection **list,
     /* connect to port share server */
     if ((sd_server = socket(PF_INET, SOCK_STREAM, IPPROTO_TCP)) < 0)
     {
-        msg(M_WARN|M_ERRNO, "PORT SHARE PROXY: cannot create socket");
+        msg(M_WARN|M_SKERR, "PORT SHARE PROXY: cannot create socket");
         return false;
     }
     status = openvpn_connect(sd_server,(const struct sockaddr *)  &server_addr, 5, NULL);
diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c
index 0f34a5de..acfcfd65 100644
--- a/src/openvpn/socket.c
+++ b/src/openvpn/socket.c
@@ -1143,7 +1143,7 @@  create_socket(struct link_socket *sock, struct addrinfo *addr)
         msg(M_INFO, "Using bind-dev %s", sock->bind_dev);
         if (setsockopt(sock->sd, SOL_SOCKET, SO_BINDTODEVICE, sock->bind_dev, strlen(sock->bind_dev) + 1) != 0)
         {
-            msg(M_WARN|M_ERRNO, "WARN: setsockopt SO_BINDTODEVICE=%s failed", sock->bind_dev);
+            msg(M_WARN|M_SKERR, "WARN: setsockopt SO_BINDTODEVICE=%s failed", sock->bind_dev);
         }
 
     }
@@ -1222,7 +1222,7 @@  socket_do_accept(socket_descriptor_t sd,
 
         if (!socket_defined(new_sd))
         {
-            msg(D_LINK_ERRORS | M_ERRNO, "TCP: getpeername() failed");
+            msg(D_LINK_ERRORS | M_SKERR, "TCP: getpeername() failed");
         }
         else
         {
@@ -1247,7 +1247,7 @@  socket_do_accept(socket_descriptor_t sd,
 
     if (!socket_defined(new_sd))
     {
-        msg(D_LINK_ERRORS | M_ERRNO, "TCP: accept(%d) failed", (int)sd);
+        msg(D_LINK_ERRORS | M_SKERR, "TCP: accept(%d) failed", (int)sd);
     }
     /* only valid if we have remote_len_af!=0 */
     else if (remote_len_af && remote_len != remote_len_af)
@@ -1313,7 +1313,7 @@  socket_listen_accept(socket_descriptor_t sd,
 
         if (status < 0)
         {
-            msg(D_LINK_ERRORS | M_ERRNO, "TCP: select() failed");
+            msg(D_LINK_ERRORS | M_SKERR, "TCP: select() failed");
         }
 
         if (status <= 0)
@@ -1409,12 +1409,12 @@  socket_bind(socket_descriptor_t sd,
         msg(M_INFO, "setsockopt(IPV6_V6ONLY=%d)", v6only);
         if (setsockopt(sd, IPPROTO_IPV6, IPV6_V6ONLY, (void *) &v6only, sizeof(v6only)))
         {
-            msg(M_NONFATAL|M_ERRNO, "Setting IPV6_V6ONLY=%d failed", v6only);
+            msg(M_NONFATAL | M_SKERR, "Setting IPV6_V6ONLY=%d failed", v6only);
         }
     }
     if (bind(sd, cur->ai_addr, cur->ai_addrlen))
     {
-        msg(M_FATAL | M_ERRNO, "%s: Socket bind failed on local address %s",
+        msg(M_FATAL | M_SKERR, "%s: Socket bind failed on local address %s",
             prefix,
             print_sockaddr_ex(local->ai_addr, ":", PS_SHOW_PORT, &gc));
     }
@@ -2254,7 +2254,7 @@  link_socket_close(struct link_socket *sock)
                 msg(D_LOW, "TCP/UDP: Closing socket");
                 if (openvpn_close_socket(sock->sd))
                 {
-                    msg(M_WARN | M_ERRNO, "TCP/UDP: Close Socket failed");
+                    msg(M_WARN | M_SKERR, "TCP/UDP: Close Socket failed");
                 }
             }
             sock->sd = SOCKET_UNDEFINED;
@@ -2271,7 +2271,7 @@  link_socket_close(struct link_socket *sock)
         {
             if (openvpn_close_socket(sock->ctrl_sd))
             {
-                msg(M_WARN | M_ERRNO, "TCP/UDP: Close Socket (ctrl_sd) failed");
+                msg(M_WARN | M_SKERR, "TCP/UDP: Close Socket (ctrl_sd) failed");
             }
             sock->ctrl_sd = SOCKET_UNDEFINED;
         }
@@ -3668,7 +3668,7 @@  sockethandle_finalize(sockethandle_t sh,
                     /* if no error (i.e. just not finished yet), then DON'T execute this code */
                     io->iostate = IOSTATE_INITIAL;
                     ASSERT(ResetEvent(io->overlapped.hEvent));
-                    msg(D_WIN32_IO | M_ERRNO, "WIN32 I/O: Completion error");
+                    msg(D_WIN32_IO | SocketHandleErrorFlag(sh), "WIN32 I/O: Completion error");
                 }
             }
             break;
@@ -3681,7 +3681,7 @@  sockethandle_finalize(sockethandle_t sh,
                 /* error return for a non-queued operation */
                 SocketHandleSetLastError(sh, io->status);
                 ret = -1;
-                msg(D_WIN32_IO | M_ERRNO, "WIN32 I/O: Completion non-queued error");
+                msg(D_WIN32_IO | SocketHandleErrorFlag(sh), "WIN32 I/O: Completion non-queued error");
             }
             else
             {
diff --git a/src/openvpn/socket.h b/src/openvpn/socket.h
index e9f1524d..a7ec827a 100644
--- a/src/openvpn/socket.h
+++ b/src/openvpn/socket.h
@@ -301,6 +301,12 @@  SocketHandleSetInvalError(sockethandle_t sh)
     sh.is_handle ? SetLastError(ERROR_INVALID_FUNCTION) : WSASetLastError(WSAEINVAL);
 }
 
+static inline int
+SocketHandleErrorFlag(sockethandle_t sh)
+{
+    return sh.is_handle ? M_ERRNO : M_SKERR;
+}
+
 #else  /* ifdef _WIN32 */
 
 #define openvpn_close_socket(s) close(s)
diff --git a/src/openvpn/socks.c b/src/openvpn/socks.c
index 768bb613..10107c18 100644
--- a/src/openvpn/socks.c
+++ b/src/openvpn/socks.c
@@ -117,7 +117,7 @@  socks_username_password_auth(struct socks_proxy_info *p,
 
     if (size != strlen(to_send))
     {
-        msg(D_LINK_ERRORS | M_ERRNO, "socks_username_password_auth: TCP port write failed on send()");
+        msg(D_LINK_ERRORS | M_SKERR, "socks_username_password_auth: TCP port write failed on send()");
         goto cleanup;
     }
 
@@ -145,14 +145,14 @@  socks_username_password_auth(struct socks_proxy_info *p,
         /* timeout? */
         if (status == 0)
         {
-            msg(D_LINK_ERRORS | M_ERRNO, "socks_username_password_auth: TCP port read timeout expired");
+            msg(D_LINK_ERRORS | M_SKERR, "socks_username_password_auth: TCP port read timeout expired");
             goto cleanup;
         }
 
         /* error */
         if (status < 0)
         {
-            msg(D_LINK_ERRORS | M_ERRNO, "socks_username_password_auth: TCP port read failed on select()");
+            msg(D_LINK_ERRORS | M_SKERR, "socks_username_password_auth: TCP port read failed on select()");
             goto cleanup;
         }
 
@@ -162,7 +162,7 @@  socks_username_password_auth(struct socks_proxy_info *p,
         /* error? */
         if (size != 1)
         {
-            msg(D_LINK_ERRORS | M_ERRNO, "socks_username_password_auth: TCP port read failed on recv()");
+            msg(D_LINK_ERRORS | M_SKERR, "socks_username_password_auth: TCP port read failed on recv()");
             goto cleanup;
         }
 
@@ -204,7 +204,7 @@  socks_handshake(struct socks_proxy_info *p,
     size = send(sd, method_sel, sizeof(method_sel), MSG_NOSIGNAL);
     if (size != sizeof(method_sel))
     {
-        msg(D_LINK_ERRORS | M_ERRNO, "socks_handshake: TCP port write failed on send()");
+        msg(D_LINK_ERRORS | M_SKERR, "socks_handshake: TCP port write failed on send()");
         return false;
     }
 
@@ -232,14 +232,14 @@  socks_handshake(struct socks_proxy_info *p,
         /* timeout? */
         if (status == 0)
         {
-            msg(D_LINK_ERRORS | M_ERRNO, "socks_handshake: TCP port read timeout expired");
+            msg(D_LINK_ERRORS | M_SKERR, "socks_handshake: TCP port read timeout expired");
             return false;
         }
 
         /* error */
         if (status < 0)
         {
-            msg(D_LINK_ERRORS | M_ERRNO, "socks_handshake: TCP port read failed on select()");
+            msg(D_LINK_ERRORS | M_SKERR, "socks_handshake: TCP port read failed on select()");
             return false;
         }
 
@@ -249,7 +249,7 @@  socks_handshake(struct socks_proxy_info *p,
         /* error? */
         if (size != 1)
         {
-            msg(D_LINK_ERRORS | M_ERRNO, "socks_handshake: TCP port read failed on recv()");
+            msg(D_LINK_ERRORS | M_SKERR, "socks_handshake: TCP port read failed on recv()");
             return false;
         }
 
@@ -342,14 +342,14 @@  recv_socks_reply(socket_descriptor_t sd,
         /* timeout? */
         if (status == 0)
         {
-            msg(D_LINK_ERRORS | M_ERRNO, "recv_socks_reply: TCP port read timeout expired");
+            msg(D_LINK_ERRORS | M_SKERR, "recv_socks_reply: TCP port read timeout expired");
             return false;
         }
 
         /* error */
         if (status < 0)
         {
-            msg(D_LINK_ERRORS | M_ERRNO, "recv_socks_reply: TCP port read failed on select()");
+            msg(D_LINK_ERRORS | M_SKERR, "recv_socks_reply: TCP port read failed on select()");
             return false;
         }
 
@@ -359,7 +359,7 @@  recv_socks_reply(socket_descriptor_t sd,
         /* error? */
         if (size != 1)
         {
-            msg(D_LINK_ERRORS | M_ERRNO, "recv_socks_reply: TCP port read failed on recv()");
+            msg(D_LINK_ERRORS | M_SKERR, "recv_socks_reply: TCP port read failed on recv()");
             return false;
         }
 
@@ -484,7 +484,7 @@  establish_socks_proxy_passthru(struct socks_proxy_info *p,
         const ssize_t size = send(sd, buf, 5 + len + 2, MSG_NOSIGNAL);
         if ((int)size != 5 + (int)len + 2)
         {
-            msg(D_LINK_ERRORS | M_ERRNO, "establish_socks_proxy_passthru: TCP port write failed on send()");
+            msg(D_LINK_ERRORS | M_SKERR, "establish_socks_proxy_passthru: TCP port write failed on send()");
             goto error;
         }
     }
@@ -527,7 +527,7 @@  establish_socks_proxy_udpassoc(struct socks_proxy_info *p,
                                   10, MSG_NOSIGNAL);
         if (size != 10)
         {
-            msg(D_LINK_ERRORS | M_ERRNO, "establish_socks_proxy_passthru: TCP port write failed on send()");
+            msg(D_LINK_ERRORS | M_SKERR, "establish_socks_proxy_passthru: TCP port write failed on send()");
             goto error;
         }
     }