Skip to contentSkip to conversation

Return NULL if GetAdaptersInfo fails

PATCH In review [email protected]... 5 participants 5 messages Jan 3, 2018 – Jan 9, 2018

lore
Share & tools
lore

[email protected]...

author
#0 lore raw

PATCH 1/1 Return NULL if GetAdaptersInfo fails

From: Selva Nair <[email protected]...>

- Currently a pointer to potentially uninitialized IP_ADAPTER_INFO struct is returned on error causing ill-defined behaviour.

Signed-off-by Selva Nair <[email protected]...>

---

There have been some reports of unexpected failure in GetAdaptersInfo. When and why that happens is still unclear but this bug could explain why the process goes into a tailspin in such occasions.

 src/openvpn/tun.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)
src/openvpn/tun.c +2 -5
index 25831ce..6e16348 100644
@@ -4178,15 +4178,12 @@ get_adapter_info_list(struct gc_arena *gc)
     else
     {
         pi = (PIP_ADAPTER_INFO) gc_malloc(size, false, gc);
-        if ((status = GetAdaptersInfo(pi, &size)) == NO_ERROR)
-        {
-            return pi;
-        }
-        else
+        if ((status = GetAdaptersInfo(pi, &size)) != NO_ERROR)
         {
             msg(M_INFO, "GetAdaptersInfo #2 failed (status=%u) : %s",
                 (unsigned int)status,
                 strerror_win32(status, gc));
+            pi = NULL;
         }
     }
     return pi;
Signature · 1 line
2.1.4
Acked-by on v1 1/1
#1 lore raw

Hi,

Quoted 28 lines · On 03-01-18 05:02, [email protected]... wrote:

From: Selva Nair <[email protected]...>

- Currently a pointer to potentially uninitialized IP_ADAPTER_INFO struct is returned on error causing ill-defined behaviour.

Signed-off-by Selva Nair <[email protected]...>

---

There have been some reports of unexpected failure in GetAdaptersInfo. When and why that happens is still unclear but this bug could explain why the process goes into a tailspin in such occasions.

 src/openvpn/tun.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)
src/openvpn/tun.c +0 -4
index 25831ce..6e16348 100644
@@ -4178,15 +4178,12 @@ get_adapter_info_list(struct gc_arena *gc)
     else
     {
         pi = (PIP_ADAPTER_INFO) gc_malloc(size, false, gc);
-        if ((status = GetAdaptersInfo(pi, &size)) == NO_ERROR)
-        {
-            return pi;
-        }

src/openvpn/tun.c get_adapter_info_list(struct gc_arena *gc) · line 4189

Quoted 10 lines
src/openvpn/tun.c +2 -1
-        else
+        if ((status = GetAdaptersInfo(pi, &size)) != NO_ERROR)
         {
             msg(M_INFO, "GetAdaptersInfo #2 failed (status=%u) : %s",
                 (unsigned int)status,
                 strerror_win32(status, gc));
+            pi = NULL;
         }
     }
     return pi;

Less lines, *and* less bugs. Very good.

Acked-by Steffan Karger <[email protected]...>

-Steffan

#2 lore raw
In reply to [email protected]...

is it related to trac#973 ?

2018-01-03 9:02 GMT+05:00 <[email protected]...>:

Quoted 50 lines

From: Selva Nair <[email protected]...>

- Currently a pointer to potentially uninitialized IP_ADAPTER_INFO struct is returned on error causing ill-defined behaviour.

Signed-off-by Selva Nair <[email protected]...>

---

There have been some reports of unexpected failure in GetAdaptersInfo. When and why that happens is still unclear but this bug could explain why the process goes into a tailspin in such occasions.

 src/openvpn/tun.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)
src/openvpn/tun.c +2 -6
index 25831ce..6e16348 100644
@@ -4178,15 +4178,12 @@ get_adapter_info_list(struct gc_arena *gc)
     else
     {
         pi = (PIP_ADAPTER_INFO) gc_malloc(size, false, gc);
-        if ((status = GetAdaptersInfo(pi, &size)) == NO_ERROR)
-        {
-            return pi;
-        }
-        else
+        if ((status = GetAdaptersInfo(pi, &size)) != NO_ERROR)
         {
             msg(M_INFO, "GetAdaptersInfo #2 failed (status=%u) : %s",
                 (unsigned int)status,
                 strerror_win32(status, gc));
+            pi = NULL;
         }
     }
     return pi;
--

2.1.4

------------------------------------------------------------
------------------
Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Openvpn-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Acked-by on v1 1/1
#3 lore raw
In reply to [email protected]...

Hi,

Quoted 8 lines

-----Original Message-----
From: [email protected]... [mailto:[email protected]...]
Sent: Wednesday, January 03, 2018 5:02 AM
To: [email protected]
Subject: [Openvpn-devel] [PATCH] Return NULL if GetAdaptersInfo fails

From: Selva Nair <[email protected]...>

Quoted 8 lines

- Currently a pointer to potentially uninitialized IP_ADAPTER_INFO struct is returned on error causing ill-defined behaviour.

Signed-off-by Selva Nair <[email protected]...>

---

There have been some reports of unexpected failure in GetAdaptersInfo. When and why that happens is still unclear but this bug could explain why

the

Quoted 28 lines

process goes into a tailspin in such occasions.

 src/openvpn/tun.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)
src/openvpn/tun.c index 25831ce..6e16348 renamed from src/openvpn/tun.c +0 -0

100644

src/openvpn/tun.c +2 -6
@@ -4178,15 +4178,12 @@ get_adapter_info_list(struct gc_arena *gc)
     else
     {
         pi = (PIP_ADAPTER_INFO) gc_malloc(size, false, gc);
-        if ((status = GetAdaptersInfo(pi, &size)) == NO_ERROR)
-        {
-            return pi;
-        }
-        else
+        if ((status = GetAdaptersInfo(pi, &size)) != NO_ERROR)
         {
             msg(M_INFO, "GetAdaptersInfo #2 failed (status=%u) : %s",
                 (unsigned int)status,
                 strerror_win32(status, gc));
+            pi = NULL;
         }
     }
     return pi;
--
Quoted 4 lines

2.1.4

----------------------------------------------------------------------------
--

Quoted 6 lines

Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Openvpn-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

I have reviewed the program flow after each get_adapter_info_list() function call. All code to use the return value of get_adapter_info_list() indeed checks the return value for non-NULL before using it.

Acked-by Simon Rozman <[email protected]...>

Regards,
Simon

#4 lore raw
In reply to [email protected]...

Thanks.

Your patch has been applied to the master and release/2.4 branch.

commit 5050c5b1ec3f2aa04140ab83f2498b3329381ee4 (master) commit d2b9bd80bab7ce56ada09b0df1c2baec8b029d93 (release/2.4)

Author: Selva Nair
Date:   Tue Jan 2 23:02:03 2018 -0500
     Return NULL if GetAdaptersInfo fails

     Signed-off-by: Selva Nair <[email protected]...>
     Acked-by: Steffan Karger <[email protected]...>
     Acked-by: Simon Rozman <[email protected]...>
     Message-Id: <[email protected]...>
     URL: https://www.mail-archive.com/[email protected]/msg16139.html
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering