Skip to contentSkip to conversation

On signal reception, return EAI_SYSTEM from openvpn_getaddrinfo().

PATCH In review Gert Doering 3 participants 7 messages May 31, 2015 – Jun 1, 2015

lore
Share & tools
lore

Gert Doering

author
#0 lore raw

PATCH 1/1 On signal reception, return EAI_SYSTEM from openvpn_getaddrinfo().

A signal (except SIGUSR1) received while waiting for getaddrinfo() is considered fatal, so openvpn_getaddrinfo() is destroying the returned information with freeaddrinfo(), but still signalled "success" (0) to the caller - so if the caller accessed *res before checking *signal_received, it would access just-free()ed memory, which on some platforms still worked and on others caused a crash.

Also, ensure that *ai is also NULLed in the caller now.

Signed-off-by Gert Doering <[email protected]...>
---
 src/openvpn/socket.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
src/openvpn/socket.c +4 -1
index cf2dec3..034ccc1 100644
@@ -219,10 +219,13 @@ msg(M_INFO, "if(*signal_received)");
                     }
                   else
                     {
+		      /* turn success into failure (interrupted syscall) */
                       if (0 == status) {
                           ASSERT(res);
                           freeaddrinfo(*res);
-                          res = NULL;
+                          *res = NULL;
+                          status = EAI_SYSTEM;
+                          errno = EINTR;
                       }
                       goto done;
                     }
Signature · 1 line
2.4.0
author
#1 lore raw

Hi,

Quoted 8 lines · Gert Doering

A signal (except SIGUSR1) received while waiting for getaddrinfo() is considered fatal, so openvpn_getaddrinfo() is destroying the returned information with freeaddrinfo(), but still signalled "success" (0) to the caller - so if the caller accessed *res before checking *signal_received, it would access just-free()ed memory, which on some platforms still worked and on others caused a crash.

Also, ensure that *ai is also NULLed in the caller now.

This is actually "Trac #276" :-) - need to add this to the commit note.

gert

Signature · 4 lines
USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...
#2 lore raw
In reply to Gert Doering

Am 31.05.15 um 22:41 schrieb Gert Doering:

Quoted 23 lines

A signal (except SIGUSR1) received while waiting for getaddrinfo() is considered fatal, so openvpn_getaddrinfo() is destroying the returned information with freeaddrinfo(), but still signalled "success" (0) to the caller - so if the caller accessed *res before checking *signal_received, it would access just-free()ed memory, which on some platforms still worked and on others caused a crash.

Also, ensure that *ai is also NULLed in the caller now.

Signed-off-by Gert Doering <[email protected]...>
---
 src/openvpn/socket.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
src/openvpn/socket.c +1 -0
index cf2dec3..034ccc1 100644
@@ -219,10 +219,13 @@ msg(M_INFO, "if(*signal_received)");
                     }
                   else
                     {
+		      /* turn success into failure (interrupted syscall) */

src/openvpn/socket.c msg(M_INFO, "if(*signal_received)"); · line 231

Quoted 10 lines
src/openvpn/socket.c +3 -1
                       if (0 == status) {
                           ASSERT(res);
                           freeaddrinfo(*res);
-                          res = NULL;
+                          *res = NULL;
+                          status = EAI_SYSTEM;
+                          errno = EINTR;
                       }
                       goto done;
                     }

ACK. The patch is doing the right thing.

Arne

author
#3 lore raw
In reply to Gert Doering

Patch has been applied to the master and release/2.3 branch.

commit 5f6c01ea6172ed1d8ed04e31f9f6c3f8e4696109 (master) commit 38c9f980d4f7cb7061f9db3ed8645ab3404e533d (release/2.3)

Author: Gert Doering
Date:   Sun May 31 22:41:58 2015 +0200
     On signal reception, return EAI_SYSTEM from openvpn_getaddrinfo().

     Signed-off-by: Gert Doering <[email protected]...>
     Acked-by: Arne Schwabe <[email protected]...>
     Message-Id: <[email protected]...>
     URL: http://article.gmane.org/gmane.network.openvpn.devel/9764

--
kind regards,

Gert Doering

author
#4 lore raw

PATCH 1/1 Use EAI_AGAIN instead of EAI_SYSTEM for openvpn_getaddrinfo().

In reply to Gert Doering

Windows has no EAI_SYSTEM (introduced by 5f6c01ea6172ed), but everyone has EAI_AGAIN - which also fits ("a temporary failure in name resolution").

Trac #276

Signed-off-by Gert Doering <[email protected]...>
---
 src/openvpn/socket.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
src/openvpn/socket.c +1 -1
index e751154..4f1139d 100644
@@ -417,7 +417,7 @@ openvpn_getaddrinfo (unsigned int flags,
                           ASSERT(res);
                           freeaddrinfo(*res);
                           *res = NULL;
-                          status = EAI_SYSTEM;
+                          status = EAI_AGAIN;	/* = temporary failure */
                           errno = EINTR;
                       }
                       goto done;
Signature · 1 line
2.3.6
#5 lore raw

Re: [Openvpn-devel] [PATCH] Use EAI_AGAIN instead of EAI_SYSTEM for openvpn_getaddrinfo().

Quoted 24 lines · Gert Doering

Windows has no EAI_SYSTEM (introduced by 5f6c01ea6172ed), but everyone has EAI_AGAIN - which also fits ("a temporary failure in name resolution").

Trac #276

Signed-off-by Gert Doering <[email protected]...>
---
 src/openvpn/socket.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
src/openvpn/socket.c +1 -2
index e751154..4f1139d 100644
@@ -417,7 +417,7 @@ openvpn_getaddrinfo (unsigned int flags,
                           ASSERT(res);
                           freeaddrinfo(*res);
                           *res = NULL;
-                          status = EAI_SYSTEM;
+                          status = EAI_AGAIN;  /* = temporary failure */
                           errno = EINTR;
                       }
                       goto done;
--
Quoted 1 line

2.3.6

ACK

-Steffan

author
#6 lore raw

[Openvpn-devel] [PATCH applied] Re: Use EAI_AGAIN instead of EAI_SYSTEM for openvpn_getaddrinfo().

In reply to Gert Doering

Patch has been applied to the master and release/2.3 branch.

commit 8ceb9619a26f8c507bafbc6d59aed3f65a30455d (master) commit 874ffde11255beb6873024b24c4231934460f485 (release/2.3)

Author: Gert Doering
Date:   Mon Jun 1 19:15:14 2015 +0200
     Use EAI_AGAIN instead of EAI_SYSTEM for openvpn_getaddrinfo().

     Signed-off-by: Gert Doering <[email protected]...>
     Acked-by: Steffan Karger <[email protected]...>
     Message-Id: <[email protected]...>
     URL: http://article.gmane.org/gmane.network.openvpn.devel/9768

--
kind regards,

Gert Doering