| Message ID | 20220504091305.434-1-lstipakov@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [Openvpn-devel,release/2.5] Fix M_ERRNO behavior on Windows | expand |
Acked-by: Selva Nair <selva.nair@gmail.com> Same as the patch 2429 <https://patchwork.openvpn.net/patch/2429/> for master except for the minor change in x_check_status() to match 2.5. On Wed, May 4, 2022 at 5: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. > --- > > This is a backport of patch acked for master > (https://patchwork.openvpn.net/patch/2429/) with > a conflict resolved in x_check_status() in error.c. > > src/openvpn/error.c | 32 +++++++++++++++++++++++++------- > src/openvpn/error.h | 39 +++++++++++++++++++++++++++++---------- > src/openvpn/forward.c | 9 ++++++++- > src/openvpn/manage.c | 5 +++-- > src/openvpn/platform.c | 2 +- > src/openvpn/tun.h | 4 ++-- > 6 files changed, 68 insertions(+), 23 deletions(-) > > diff --git a/src/openvpn/error.c b/src/openvpn/error.c > index 54796d03..7fbda844 100644 > --- a/src/openvpn/error.c > +++ b/src/openvpn/error.c > @@ -220,6 +220,18 @@ x_msg(const unsigned int flags, const char *format, > ...) > va_end(arglist); > } > > +static const char* > +openvpn_strerror(int err, bool crt_error, struct gc_arena *gc) > +{ > +#ifdef _WIN32 > + if (!crt_error) > + { > + return strerror_win32(err, gc); > + } > +#endif > + return strerror(err); > +} > + > void > x_msg_va(const unsigned int flags, const char *format, va_list arglist) > { > @@ -244,7 +256,8 @@ x_msg_va(const unsigned int flags, const char *format, > va_list arglist) > } > #endif > > - e = openvpn_errno(); > + bool crt_error = false; > + e = openvpn_errno_maybe_crt(&crt_error); > > /* > * Apply muting filter. > @@ -268,7 +281,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, openvpn_strerror(e, crt_error, &gc), e); > SWAP; > } > > @@ -649,7 +662,6 @@ x_check_status(int status, > struct link_socket *sock, > struct tuntap *tt) > { > - const int my_errno = openvpn_errno(); > const char *extended_msg = NULL; > > msg(x_cs_verbose_level, "%s %s returned %d", > @@ -672,26 +684,32 @@ x_check_status(int status, > sock->info.mtu_changed = true; > } > } > -#elif defined(_WIN32) > +#endif /* EXTENDED_SOCKET_ERROR_CAPABILITY */ > + > +#ifdef _WIN32 > /* get possible driver error from TAP-Windows driver */ > if (tuntap_defined(tt)) > { > extended_msg = tap_win_getinfo(tt, &gc); > } > #endif > - if (!ignore_sys_error(my_errno)) > + > + bool crt_error = false; > + int my_errno = openvpn_errno_maybe_crt(&crt_error); > + > + if (!ignore_sys_error(my_errno, crt_error)) > { > if (extended_msg) > { > msg(x_cs_info_level, "%s %s [%s]: %s (code=%d)", > description, > sock ? proto2ascii(sock->info.proto, sock->info.af, > true) : "", > - extended_msg, strerror(my_errno), my_errno); > + extended_msg, openvpn_strerror(my_errno, crt_error, > &gc), my_errno); > } > else > { > msg(x_cs_info_level, "%s %s: %s (code=%d)", description, > sock ? proto2ascii(sock->info.proto, sock->info.af, > true) : "", > - strerror(my_errno), my_errno); > + openvpn_strerror(my_errno, crt_error, &gc), my_errno); > } > > if (x_cs_err_delay_ms) > diff --git a/src/openvpn/error.h b/src/openvpn/error.h > index d2d83c8a..fc878a56 100644 > --- a/src/openvpn/error.h > +++ b/src/openvpn/error.h > @@ -71,13 +71,10 @@ struct gc_arena; > /* String and Error functions */ > > #ifdef _WIN32 > -#define openvpn_errno() GetLastError() > -#define openvpn_strerror(e, gc) strerror_win32(e, gc) > +#define openvpn_errno() GetLastError() > const char *strerror_win32(DWORD errnum, struct gc_arena *gc); > - > #else > -#define openvpn_errno() errno > -#define openvpn_strerror(x, gc) strerror(x) > +#define openvpn_errno() errno > #endif > > /* > @@ -363,20 +360,22 @@ msg_get_virtual_output(void) > * which can be safely ignored. > */ > static inline bool > -ignore_sys_error(const int err) > +ignore_sys_error(const int err, bool crt_error) > { > - /* I/O operation pending */ > #ifdef _WIN32 > - if (err == WSAEWOULDBLOCK || err == WSAEINVAL) > + if (!crt_error && ((err == WSAEWOULDBLOCK || err == WSAEINVAL))) > { > return true; > } > #else > - if (err == EAGAIN) > + crt_error = true; > +#endif > + > + /* I/O operation pending */ > + if (crt_error && (err == EAGAIN)) > { > return true; > } > -#endif > > #if 0 /* if enabled, suppress ENOBUFS errors */ > #ifdef ENOBUFS > @@ -398,6 +397,26 @@ nonfatal(const unsigned int err) > return err & M_FATAL ? (err ^ M_FATAL) | M_NONFATAL : err; > } > > +static inline int > +openvpn_errno_maybe_crt(bool *crt_error) > +{ > + int err = 0; > + *crt_error = false; > +#ifdef _WIN32 > + err = GetLastError(); > + if (err == ERROR_SUCCESS) > + { > + /* error is likely C runtime */ > + *crt_error = true; > + err = errno; > + } > +#else > + *crt_error = true; > + err = errno; > +#endif > + return err; > +} > + > #include "errlevel.h" > > #endif /* ifndef ERROR_H */ > diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c > index 392a5c9f..de80dcff 100644 > --- a/src/openvpn/forward.c > +++ b/src/openvpn/forward.c > @@ -1676,7 +1676,14 @@ process_outgoing_link(struct context *c) > } > > /* for unreachable network and "connecting" state switch to the > next host */ > - if (size < 0 && ENETUNREACH == error_code && c->c2.tls_multi > + > + bool unreachable = error_code == > +#ifdef _WIN32 > + WSAENETUNREACH; > +#else > + ENETUNREACH; > +#endif > + if (size < 0 && unreachable && c->c2.tls_multi > && !tls_initial_packet_received(c->c2.tls_multi) && > c->options.mode == MODE_POINT_TO_POINT) > { > msg(M_INFO, "Network unreachable, restarting"); > diff --git a/src/openvpn/manage.c b/src/openvpn/manage.c > index a4264056..8d2d9983 100644 > --- a/src/openvpn/manage.c > +++ b/src/openvpn/manage.c > @@ -2082,9 +2082,10 @@ man_process_command(struct management *man, const > char *line) > static bool > man_io_error(struct management *man, const char *prefix) > { > - const int err = openvpn_errno(); > + bool crt_error = false; > + int err = openvpn_errno_maybe_crt(&crt_error); > > - if (!ignore_sys_error(err)) > + if (!ignore_sys_error(err, crt_error)) > { > struct gc_arena gc = gc_new(); > msg(D_MANAGEMENT, "MANAGEMENT: TCP %s error: %s", prefix, > diff --git a/src/openvpn/platform.c b/src/openvpn/platform.c > index 2604c27b..4921f035 100644 > --- a/src/openvpn/platform.c > +++ b/src/openvpn/platform.c > @@ -471,7 +471,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/tun.h b/src/openvpn/tun.h > index ea19620d..ae98966b 100644 > --- a/src/openvpn/tun.h > +++ b/src/openvpn/tun.h > @@ -461,7 +461,7 @@ tuntap_stop(int status) > */ > if (status < 0) > { > - return openvpn_errno() == ERROR_FILE_NOT_FOUND; > + return GetLastError() == ERROR_FILE_NOT_FOUND; > } > return false; > } > @@ -474,7 +474,7 @@ tuntap_abort(int status) > */ > if (status < 0) > { > - return openvpn_errno() == ERROR_OPERATION_ABORTED; > + return GetLastError() == ERROR_OPERATION_ABORTED; > } > return false; > } > -- > 2.23.0.windows.1 > > > > _______________________________________________ > Openvpn-devel mailing list > Openvpn-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/openvpn-devel > <div dir="ltr"><div>Acked-by: Selva Nair <<a href="mailto:selva.nair@gmail.com">selva.nair@gmail.com</a>></div><div><br></div><div>Same as the patch <a href="https://patchwork.openvpn.net/patch/2429/">2429</a> for master except for the minor change in x_check_status() to match 2.5.</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, May 4, 2022 at 5:13 AM Lev Stipakov <<a href="mailto:lstipakov@gmail.com">lstipakov@gmail.com</a>> 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 <<a href="mailto:lev@openvpn.net" target="_blank">lev@openvpn.net</a>><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> <br> This is a backport of patch acked for master<br> (<a href="https://patchwork.openvpn.net/patch/2429/" rel="noreferrer" target="_blank">https://patchwork.openvpn.net/patch/2429/</a>) with<br> a conflict resolved in x_check_status() in error.c.<br> <br> src/openvpn/error.c | 32 +++++++++++++++++++++++++-------<br> src/openvpn/error.h | 39 +++++++++++++++++++++++++++++----------<br> src/openvpn/forward.c | 9 ++++++++-<br> src/openvpn/manage.c | 5 +++--<br> src/openvpn/platform.c | 2 +-<br> src/openvpn/tun.h | 4 ++--<br> 6 files changed, 68 insertions(+), 23 deletions(-)<br> <br> diff --git a/src/openvpn/error.c b/src/openvpn/error.c<br> index 54796d03..7fbda844 100644<br> --- a/src/openvpn/error.c<br> +++ b/src/openvpn/error.c<br> @@ -220,6 +220,18 @@ x_msg(const unsigned int flags, const char *format, ...)<br> va_end(arglist);<br> }<br> <br> +static const char*<br> +openvpn_strerror(int err, bool crt_error, struct gc_arena *gc)<br> +{<br> +#ifdef _WIN32<br> + if (!crt_error)<br> + {<br> + return strerror_win32(err, gc);<br> + }<br> +#endif<br> + return strerror(err);<br> +}<br> +<br> void<br> x_msg_va(const unsigned int flags, const char *format, va_list arglist)<br> {<br> @@ -244,7 +256,8 @@ x_msg_va(const unsigned int flags, const char *format, va_list arglist)<br> }<br> #endif<br> <br> - e = openvpn_errno();<br> + bool crt_error = false;<br> + e = openvpn_errno_maybe_crt(&crt_error);<br> <br> /*<br> * Apply muting filter.<br> @@ -268,7 +281,7 @@ x_msg_va(const unsigned int flags, const char *format, va_list arglist)<br> if ((flags & M_ERRNO) && e)<br> {<br> openvpn_snprintf(m2, ERR_BUF_SIZE, "%s: %s (errno=%d)",<br> - m1, strerror(e), e);<br> + m1, openvpn_strerror(e, crt_error, &gc), e);<br> SWAP;<br> }<br> <br> @@ -649,7 +662,6 @@ x_check_status(int status,<br> struct link_socket *sock,<br> struct tuntap *tt)<br> {<br> - const int my_errno = openvpn_errno();<br> const char *extended_msg = NULL;<br> <br> msg(x_cs_verbose_level, "%s %s returned %d",<br> @@ -672,26 +684,32 @@ x_check_status(int status,<br> sock->info.mtu_changed = true;<br> }<br> }<br> -#elif defined(_WIN32)<br> +#endif /* EXTENDED_SOCKET_ERROR_CAPABILITY */<br> +<br> +#ifdef _WIN32<br> /* get possible driver error from TAP-Windows driver */<br> if (tuntap_defined(tt))<br> {<br> extended_msg = tap_win_getinfo(tt, &gc);<br> }<br> #endif<br> - if (!ignore_sys_error(my_errno))<br> +<br> + bool crt_error = false;<br> + int my_errno = openvpn_errno_maybe_crt(&crt_error);<br> +<br> + if (!ignore_sys_error(my_errno, crt_error))<br> {<br> if (extended_msg)<br> {<br> msg(x_cs_info_level, "%s %s [%s]: %s (code=%d)", description,<br> sock ? proto2ascii(sock->info.proto, sock-><a href="http://info.af" rel="noreferrer" target="_blank">info.af</a>, true) : "",<br> - extended_msg, strerror(my_errno), my_errno);<br> + extended_msg, openvpn_strerror(my_errno, crt_error, &gc), my_errno);<br> }<br> else<br> {<br> msg(x_cs_info_level, "%s %s: %s (code=%d)", description,<br> sock ? proto2ascii(sock->info.proto, sock-><a href="http://info.af" rel="noreferrer" target="_blank">info.af</a>, true) : "",<br> - strerror(my_errno), my_errno);<br> + openvpn_strerror(my_errno, crt_error, &gc), 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 d2d83c8a..fc878a56 100644<br> --- a/src/openvpn/error.h<br> +++ b/src/openvpn/error.h<br> @@ -71,13 +71,10 @@ struct gc_arena;<br> /* String and Error functions */<br> <br> #ifdef _WIN32<br> -#define openvpn_errno() GetLastError()<br> -#define openvpn_strerror(e, gc) strerror_win32(e, gc)<br> +#define openvpn_errno() GetLastError()<br> const char *strerror_win32(DWORD errnum, struct gc_arena *gc);<br> -<br> #else<br> -#define openvpn_errno() errno<br> -#define openvpn_strerror(x, gc) strerror(x)<br> +#define openvpn_errno() errno<br> #endif<br> <br> /*<br> @@ -363,20 +360,22 @@ msg_get_virtual_output(void)<br> * which can be safely ignored.<br> */<br> static inline bool<br> -ignore_sys_error(const int err)<br> +ignore_sys_error(const int err, bool crt_error)<br> {<br> - /* I/O operation pending */<br> #ifdef _WIN32<br> - if (err == WSAEWOULDBLOCK || err == WSAEINVAL)<br> + if (!crt_error && ((err == WSAEWOULDBLOCK || err == WSAEINVAL)))<br> {<br> return true;<br> }<br> #else<br> - if (err == EAGAIN)<br> + crt_error = true;<br> +#endif<br> +<br> + /* I/O operation pending */<br> + if (crt_error && (err == EAGAIN))<br> {<br> return true;<br> }<br> -#endif<br> <br> #if 0 /* if enabled, suppress ENOBUFS errors */<br> #ifdef ENOBUFS<br> @@ -398,6 +397,26 @@ nonfatal(const unsigned int err)<br> return err & M_FATAL ? (err ^ M_FATAL) | M_NONFATAL : err;<br> }<br> <br> +static inline int<br> +openvpn_errno_maybe_crt(bool *crt_error)<br> +{<br> + int err = 0;<br> + *crt_error = false;<br> +#ifdef _WIN32<br> + err = GetLastError();<br> + if (err == ERROR_SUCCESS)<br> + {<br> + /* error is likely C runtime */<br> + *crt_error = true;<br> + err = errno;<br> + }<br> +#else<br> + *crt_error = true;<br> + err = errno;<br> +#endif<br> + return err;<br> +}<br> +<br> #include "errlevel.h"<br> <br> #endif /* ifndef ERROR_H */<br> diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c<br> index 392a5c9f..de80dcff 100644<br> --- a/src/openvpn/forward.c<br> +++ b/src/openvpn/forward.c<br> @@ -1676,7 +1676,14 @@ process_outgoing_link(struct context *c)<br> }<br> <br> /* for unreachable network and "connecting" state switch to the next host */<br> - if (size < 0 && ENETUNREACH == error_code && c->c2.tls_multi<br> +<br> + bool unreachable = error_code ==<br> +#ifdef _WIN32<br> + WSAENETUNREACH;<br> +#else<br> + ENETUNREACH;<br> +#endif<br> + if (size < 0 && unreachable && c->c2.tls_multi<br> && !tls_initial_packet_received(c->c2.tls_multi) && c->options.mode == MODE_POINT_TO_POINT)<br> {<br> msg(M_INFO, "Network unreachable, restarting");<br> diff --git a/src/openvpn/manage.c b/src/openvpn/manage.c<br> index a4264056..8d2d9983 100644<br> --- a/src/openvpn/manage.c<br> +++ b/src/openvpn/manage.c<br> @@ -2082,9 +2082,10 @@ man_process_command(struct management *man, const char *line)<br> static bool<br> man_io_error(struct management *man, const char *prefix)<br> {<br> - const int err = openvpn_errno();<br> + bool crt_error = false;<br> + int err = openvpn_errno_maybe_crt(&crt_error);<br> <br> - if (!ignore_sys_error(err))<br> + if (!ignore_sys_error(err, crt_error))<br> {<br> struct gc_arena gc = gc_new();<br> msg(D_MANAGEMENT, "MANAGEMENT: TCP %s error: %s", prefix,<br> diff --git a/src/openvpn/platform.c b/src/openvpn/platform.c<br> index 2604c27b..4921f035 100644<br> --- a/src/openvpn/platform.c<br> +++ b/src/openvpn/platform.c<br> @@ -471,7 +471,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, "Could not access file '%s'", filename);<br> }<br> diff --git a/src/openvpn/tun.h b/src/openvpn/tun.h<br> index ea19620d..ae98966b 100644<br> --- a/src/openvpn/tun.h<br> +++ b/src/openvpn/tun.h<br> @@ -461,7 +461,7 @@ tuntap_stop(int status)<br> */<br> if (status < 0)<br> {<br> - return openvpn_errno() == ERROR_FILE_NOT_FOUND;<br> + return GetLastError() == ERROR_FILE_NOT_FOUND;<br> }<br> return false;<br> }<br> @@ -474,7 +474,7 @@ tuntap_abort(int status)<br> */<br> if (status < 0)<br> {<br> - return openvpn_errno() == ERROR_OPERATION_ABORTED;<br> + return GetLastError() == ERROR_OPERATION_ABORTED;<br> }<br> return false;<br> }<br> -- <br> 2.23.0.windows.1<br> <br> <br> <br> _______________________________________________<br> Openvpn-devel mailing list<br> <a href="mailto:Openvpn-devel@lists.sourceforge.net" target="_blank">Openvpn-devel@lists.sourceforge.net</a><br> <a href="https://lists.sourceforge.net/lists/listinfo/openvpn-devel" rel="noreferrer" target="_blank">https://lists.sourceforge.net/lists/listinfo/openvpn-devel</a><br> </blockquote></div></div>
Your patch has been applied to the release/2.5 branch.
S-O-B added.
commit 4e5b14012550bf934dcf850547b542afa4d6605a
Author: Lev Stipakov
Date: Wed May 4 12:13:05 2022 +0300
Fix M_ERRNO behavior on Windows
Acked-by: Selva Nair <selva.nair@gmail.com>
Message-Id: <20220504091305.434-1-lstipakov@gmail.com>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg24274.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
--
kind regards,
Gert Doering
diff --git a/src/openvpn/error.c b/src/openvpn/error.c index 54796d03..7fbda844 100644 --- a/src/openvpn/error.c +++ b/src/openvpn/error.c @@ -220,6 +220,18 @@ x_msg(const unsigned int flags, const char *format, ...) va_end(arglist); } +static const char* +openvpn_strerror(int err, bool crt_error, struct gc_arena *gc) +{ +#ifdef _WIN32 + if (!crt_error) + { + return strerror_win32(err, gc); + } +#endif + return strerror(err); +} + void x_msg_va(const unsigned int flags, const char *format, va_list arglist) { @@ -244,7 +256,8 @@ x_msg_va(const unsigned int flags, const char *format, va_list arglist) } #endif - e = openvpn_errno(); + bool crt_error = false; + e = openvpn_errno_maybe_crt(&crt_error); /* * Apply muting filter. @@ -268,7 +281,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, openvpn_strerror(e, crt_error, &gc), e); SWAP; } @@ -649,7 +662,6 @@ x_check_status(int status, struct link_socket *sock, struct tuntap *tt) { - const int my_errno = openvpn_errno(); const char *extended_msg = NULL; msg(x_cs_verbose_level, "%s %s returned %d", @@ -672,26 +684,32 @@ x_check_status(int status, sock->info.mtu_changed = true; } } -#elif defined(_WIN32) +#endif /* EXTENDED_SOCKET_ERROR_CAPABILITY */ + +#ifdef _WIN32 /* get possible driver error from TAP-Windows driver */ if (tuntap_defined(tt)) { extended_msg = tap_win_getinfo(tt, &gc); } #endif - if (!ignore_sys_error(my_errno)) + + bool crt_error = false; + int my_errno = openvpn_errno_maybe_crt(&crt_error); + + if (!ignore_sys_error(my_errno, crt_error)) { if (extended_msg) { msg(x_cs_info_level, "%s %s [%s]: %s (code=%d)", description, sock ? proto2ascii(sock->info.proto, sock->info.af, true) : "", - extended_msg, strerror(my_errno), my_errno); + extended_msg, openvpn_strerror(my_errno, crt_error, &gc), my_errno); } else { msg(x_cs_info_level, "%s %s: %s (code=%d)", description, sock ? proto2ascii(sock->info.proto, sock->info.af, true) : "", - strerror(my_errno), my_errno); + openvpn_strerror(my_errno, crt_error, &gc), my_errno); } if (x_cs_err_delay_ms) diff --git a/src/openvpn/error.h b/src/openvpn/error.h index d2d83c8a..fc878a56 100644 --- a/src/openvpn/error.h +++ b/src/openvpn/error.h @@ -71,13 +71,10 @@ struct gc_arena; /* String and Error functions */ #ifdef _WIN32 -#define openvpn_errno() GetLastError() -#define openvpn_strerror(e, gc) strerror_win32(e, gc) +#define openvpn_errno() GetLastError() const char *strerror_win32(DWORD errnum, struct gc_arena *gc); - #else -#define openvpn_errno() errno -#define openvpn_strerror(x, gc) strerror(x) +#define openvpn_errno() errno #endif /* @@ -363,20 +360,22 @@ msg_get_virtual_output(void) * which can be safely ignored. */ static inline bool -ignore_sys_error(const int err) +ignore_sys_error(const int err, bool crt_error) { - /* I/O operation pending */ #ifdef _WIN32 - if (err == WSAEWOULDBLOCK || err == WSAEINVAL) + if (!crt_error && ((err == WSAEWOULDBLOCK || err == WSAEINVAL))) { return true; } #else - if (err == EAGAIN) + crt_error = true; +#endif + + /* I/O operation pending */ + if (crt_error && (err == EAGAIN)) { return true; } -#endif #if 0 /* if enabled, suppress ENOBUFS errors */ #ifdef ENOBUFS @@ -398,6 +397,26 @@ nonfatal(const unsigned int err) return err & M_FATAL ? (err ^ M_FATAL) | M_NONFATAL : err; } +static inline int +openvpn_errno_maybe_crt(bool *crt_error) +{ + int err = 0; + *crt_error = false; +#ifdef _WIN32 + err = GetLastError(); + if (err == ERROR_SUCCESS) + { + /* error is likely C runtime */ + *crt_error = true; + err = errno; + } +#else + *crt_error = true; + err = errno; +#endif + return err; +} + #include "errlevel.h" #endif /* ifndef ERROR_H */ diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c index 392a5c9f..de80dcff 100644 --- a/src/openvpn/forward.c +++ b/src/openvpn/forward.c @@ -1676,7 +1676,14 @@ process_outgoing_link(struct context *c) } /* for unreachable network and "connecting" state switch to the next host */ - if (size < 0 && ENETUNREACH == error_code && c->c2.tls_multi + + bool unreachable = error_code == +#ifdef _WIN32 + WSAENETUNREACH; +#else + ENETUNREACH; +#endif + if (size < 0 && unreachable && c->c2.tls_multi && !tls_initial_packet_received(c->c2.tls_multi) && c->options.mode == MODE_POINT_TO_POINT) { msg(M_INFO, "Network unreachable, restarting"); diff --git a/src/openvpn/manage.c b/src/openvpn/manage.c index a4264056..8d2d9983 100644 --- a/src/openvpn/manage.c +++ b/src/openvpn/manage.c @@ -2082,9 +2082,10 @@ man_process_command(struct management *man, const char *line) static bool man_io_error(struct management *man, const char *prefix) { - const int err = openvpn_errno(); + bool crt_error = false; + int err = openvpn_errno_maybe_crt(&crt_error); - if (!ignore_sys_error(err)) + if (!ignore_sys_error(err, crt_error)) { struct gc_arena gc = gc_new(); msg(D_MANAGEMENT, "MANAGEMENT: TCP %s error: %s", prefix, diff --git a/src/openvpn/platform.c b/src/openvpn/platform.c index 2604c27b..4921f035 100644 --- a/src/openvpn/platform.c +++ b/src/openvpn/platform.c @@ -471,7 +471,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/tun.h b/src/openvpn/tun.h index ea19620d..ae98966b 100644 --- a/src/openvpn/tun.h +++ b/src/openvpn/tun.h @@ -461,7 +461,7 @@ tuntap_stop(int status) */ if (status < 0) { - return openvpn_errno() == ERROR_FILE_NOT_FOUND; + return GetLastError() == ERROR_FILE_NOT_FOUND; } return false; } @@ -474,7 +474,7 @@ tuntap_abort(int status) */ if (status < 0) { - return openvpn_errno() == ERROR_OPERATION_ABORTED; + return GetLastError() == ERROR_OPERATION_ABORTED; } return false; }
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. --- This is a backport of patch acked for master (https://patchwork.openvpn.net/patch/2429/) with a conflict resolved in x_check_status() in error.c. src/openvpn/error.c | 32 +++++++++++++++++++++++++------- src/openvpn/error.h | 39 +++++++++++++++++++++++++++++---------- src/openvpn/forward.c | 9 ++++++++- src/openvpn/manage.c | 5 +++-- src/openvpn/platform.c | 2 +- src/openvpn/tun.h | 4 ++-- 6 files changed, 68 insertions(+), 23 deletions(-)