Fixed stack corruption on some phones...

[Fra-Ktus]

the dynamically allocated array on the stack can crash some phones, moved that to a block allocated in main memory.

[Mygod]

I tried your code and it's still doing stack corruptions... :/

[kerpra]

I tried your code and it's still doing stack corruptions... :/

Me too...

[Mario-Klebsch]

I also had a stack corrution and I think, I found the cause. The array struct ifaddrs *l_links[l_numLinks]; is too small. Its size is determined by counting the RTM_NEWLINK messages, but is is indexed by l_info->ifi_index - 1. When e.g. network namespaces are used, ifi_index values are not continuous. I added some print statements and got this output:

l_numLinks=2
l_info->ifi_index=1
l_info->ifi_index=4

So, l_numLinks should be determined by finding the max value of l_info->ifi_index.

[Mario-Klebsch]

The code also does not behave correctly, when the buffer for receiving Netlink messages is too small.

Here is my fix for both problems:

--- getifaddrs.c        (revision 15518)
+++ getifaddrs.c        (working copy)
@@ -96,8 +96,11 @@
         l_msg.msg_control = NULL;
         l_msg.msg_controllen = 0;
         l_msg.msg_flags = 0;
-        int l_result = recvmsg(p_socket, &l_msg, 0);
-
+        int l_result = recvmsg(p_socket, &l_msg, MSG_PEEK | MSG_TRUNC);
+        if (l_result > p_len)// buffer was too small
+            return -1;
+
+        l_result = recvmsg(p_socket, &l_msg, 0);
         if(l_result < 0)
         {
             if(errno == EINTR)
@@ -106,11 +109,6 @@
             }
             return -2;
         }
-
-        if(l_msg.msg_flags & MSG_TRUNC)
-        { // buffer was too small
-            return -1;
-        }
         return l_result;
     }
 }
@@ -521,9 +519,9 @@
     }
 }

-static unsigned countLinks(int p_socket, NetlinkList *p_netlinkList)
+static unsigned get_max_ifi_index(int p_socket, NetlinkList *p_netlinkList)
 {
-    unsigned l_links = 0;
+    unsigned l_max_ifi_index = 0;
     pid_t l_pid = getpid();
     for(; p_netlinkList; p_netlinkList = p_netlinkList->m_next)
     {
@@ -543,12 +541,14 @@

             if(l_hdr->nlmsg_type == RTM_NEWLINK)
             {
-                ++l_links;
+                struct ifinfomsg *l_info = (struct ifinfomsg *)NLMSG_DATA(l_hdr);
+                if (l_max_ifi_index < l_info->ifi_index)
+                   l_max_ifi_index = l_info->ifi_index;
             }
         }
     }

-    return l_links;
+    return l_max_ifi_index;
 }

 int getifaddrs(struct ifaddrs **ifap)
@@ -580,9 +580,9 @@
         return -1;
     }

-    unsigned l_numLinks = countLinks(l_socket, l_linkResults) + countLinks(l_socket, l_addrResults);
-    struct ifaddrs *l_links[l_numLinks];
-    memset(l_links, 0, l_numLinks * sizeof(struct ifaddrs *));
+    unsigned l_max_ifi_index = get_max_ifi_index(l_socket, l_linkResults) + get_max_ifi_index(l_socket, l_addrResults);
+    struct ifaddrs *l_links[l_max_ifi_index];
+    memset(l_links, 0, l_max_ifi_index * sizeof(struct ifaddrs *));

     interpret(l_socket, l_linkResults, l_links, ifap);
     interpret(l_socket, l_addrResults, l_links, ifap);

[Mygod]

In the end I copied the implementation from aosp: https://github.com/Mygod/DHCPv6-Client-Android/tree/master/mobile/src/main/cpp

[Mario-Klebsch]

The aosp version is only 240 lines, while this implementation is 600 lines.

But the short version uses an RX buffer with fixed size of 8192. I have not enough experience with Netlink communication to be sure, that it always is lage enough and no Netlink message gets truncated.

[surpass007]

I tried your code and it's still doing stack corruptions... :/

Me too...

me too..

[ShootingKing-AM]

The code also does not behave correctly, when the buffer for receiving Netlink messages is too small.

Here is my fix for both problems:

--- getifaddrs.c        (revision 15518)
+++ getifaddrs.c        (working copy)
@@ -96,8 +96,11 @@
         l_msg.msg_control = NULL;
         l_msg.msg_controllen = 0;
         l_msg.msg_flags = 0;
-        int l_result = recvmsg(p_socket, &l_msg, 0);
-
+        int l_result = recvmsg(p_socket, &l_msg, MSG_PEEK | MSG_TRUNC);
+        if (l_result > p_len)// buffer was too small
+            return -1;
+
+        l_result = recvmsg(p_socket, &l_msg, 0);
         if(l_result < 0)
         {
             if(errno == EINTR)
@@ -106,11 +109,6 @@
             }
             return -2;
         }
-
-        if(l_msg.msg_flags & MSG_TRUNC)
-        { // buffer was too small
-            return -1;
-        }
         return l_result;
     }
 }
@@ -521,9 +519,9 @@
     }
 }

-static unsigned countLinks(int p_socket, NetlinkList *p_netlinkList)
+static unsigned get_max_ifi_index(int p_socket, NetlinkList *p_netlinkList)
 {
-    unsigned l_links = 0;
+    unsigned l_max_ifi_index = 0;
     pid_t l_pid = getpid();
     for(; p_netlinkList; p_netlinkList = p_netlinkList->m_next)
     {
@@ -543,12 +541,14 @@

             if(l_hdr->nlmsg_type == RTM_NEWLINK)
             {
-                ++l_links;
+                struct ifinfomsg *l_info = (struct ifinfomsg *)NLMSG_DATA(l_hdr);
+                if (l_max_ifi_index < l_info->ifi_index)
+                   l_max_ifi_index = l_info->ifi_index;
             }
         }
     }

-    return l_links;
+    return l_max_ifi_index;
 }

 int getifaddrs(struct ifaddrs **ifap)
@@ -580,9 +580,9 @@
         return -1;
     }

-    unsigned l_numLinks = countLinks(l_socket, l_linkResults) + countLinks(l_socket, l_addrResults);
-    struct ifaddrs *l_links[l_numLinks];
-    memset(l_links, 0, l_numLinks * sizeof(struct ifaddrs *));
+    unsigned l_max_ifi_index = get_max_ifi_index(l_socket, l_linkResults) + get_max_ifi_index(l_socket, l_addrResults);
+    struct ifaddrs *l_links[l_max_ifi_index];
+    memset(l_links, 0, l_max_ifi_index * sizeof(struct ifaddrs *));

     interpret(l_socket, l_linkResults, l_links, ifap);
     interpret(l_socket, l_addrResults, l_links, ifap);

I cannot understand this diff, please make a PULL REQUEST with the fixed code if possible :( @Mario-Klebsch

[jim518057]

These days I use this code for try to add functions to support my Android HUAWEI P9. Because my HUAWEI P9 is API level 23, which doesn't have this function in NDK.

When I write a small program for test, It's always cause stack corruption in my Android phone.

After debug and trace, I found at:

android-ifaddrs/ifaddrs.c

Line 473 in 8f9a87c

char l_mask[16] = {0};

char l_mask[16] = {0};

I change this line to: char l_mask[20] = {0};

It looks like work well now.