Attachment 6720 Details for Bug 14806 – proposed fix

[patch] proposed fix

Fix-stack-overflow-in-getaddrinfo-if-host-has-many-a.patch (text/plain), 6.60 KB, created by Michal Kubeček on 2012-11-05 13:47:53 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Michal Kubeček

Created: 2012-11-05 13:47:53 UTC

Size: 6.60 KB

patch

obsolete

>From 4321a2bcbe89242006862eb39463722759b92d7d Mon Sep 17 00:00:00 2001
>From: Michal Kubecek <mkubecek@suse.cz>
>Date: Mon, 5 Nov 2012 10:56:34 +0100
>Subject: [PATCH] Fix stack overflow in getaddrinfo() if host has many
> addresses
>
>Allocating a linked list using alloca() in make_request() can
>lead to stack overflow if the host has many addresses.
>
>The patch is a bit more complicated than necessary in order to
>avoid
>
>  (1) calling malloc() in the most common case when there are
>      just a few addresses
>  (2) allocating many small blocks if there are many addresses
>  (3) allocating too much memory on stack
>
>Thus the patch allocates a small block for 16 addresses on
>stack; if this is not enough (usually it is), linked list of
>blocks for 64 addresses is allocated as needed.
>---
> sysdeps/unix/sysv/linux/check_pf.c |  159 ++++++++++++++++++++++++++++--------
> 1 file changed, 124 insertions(+), 35 deletions(-)
>
>diff --git a/sysdeps/unix/sysv/linux/check_pf.c b/sysdeps/unix/sysv/linux/check_pf.c
>index eebb3a4..e7add41 100644
>--- a/sysdeps/unix/sysv/linux/check_pf.c
>+++ b/sysdeps/unix/sysv/linux/check_pf.c
>@@ -44,6 +44,9 @@
> # define IFA_F_OPTIMISTIC 0
> #endif
> 
>+#define IN6AI_FIXED_SIZE 16
>+#define IN6AI_CHUNK_SIZE 64
>+
> 
> struct cached_data
> {
>@@ -61,6 +64,19 @@ static struct cached_data noai6ai_cached =
>     .in6ailen = 0
>   };
> 
>+struct in6ai_chunk
>+{
>+  struct in6addrinfo info[IN6AI_CHUNK_SIZE];
>+  struct in6ai_chunk *next;
>+};
>+
>+struct in6ai_list
>+{
>+  struct in6addrinfo info[IN6AI_FIXED_SIZE];
>+  unsigned count;
>+  struct in6ai_chunk *next;
>+};
>+
> libc_freeres_ptr (static struct cached_data *cache);
> __libc_lock_define_initialized (static, lock);
> 
>@@ -102,6 +118,90 @@ cache_valid_p (void)
> }
> 
> 
>+static void
>+in6ailist_init(struct in6ai_list *list)
>+{
>+  list->count = 0;
>+  list->next = NULL;
>+}
>+
>+
>+static struct in6addrinfo *
>+in6ailist_add(struct in6ai_list *list)
>+{
>+  if (list->count < IN6AI_FIXED_SIZE)
>+    return &list->info[list->count++];
>+
>+  unsigned idx = list->count - IN6AI_FIXED_SIZE;
>+  struct in6ai_chunk *chunk = list->next;
>+  while (idx > IN6AI_CHUNK_SIZE)
>+    {
>+      chunk = chunk->next;
>+      idx -= IN6AI_CHUNK_SIZE;
>+    }
>+
>+  if (idx == IN6AI_CHUNK_SIZE)
>+    {
>+      chunk->next = malloc(sizeof(struct in6ai_chunk));
>+      if (!chunk->next)
>+	return NULL;
>+      chunk = chunk->next;
>+      chunk->next = NULL;
>+      idx = 0;
>+    }
>+    list->count++;
>+    return &chunk->info[idx];
>+}
>+
>+
>+static struct cached_data *
>+in6ailist_export(struct in6ai_list *list)
>+{
>+  struct cached_data *result;
>+  unsigned count = list->count;
>+  result = malloc(sizeof(struct cached_data)
>+		  + count * sizeof(struct in6addrinfo));
>+  if (!result)
>+    return NULL;
>+
>+  struct in6addrinfo *p = result->in6ai;
>+  unsigned n;
>+
>+  n = (count > IN6AI_FIXED_SIZE) ? IN6AI_FIXED_SIZE : count;
>+  memcpy(p, list->info, n * sizeof(struct in6addrinfo));
>+  p += n;
>+  count -= n;
>+
>+  struct in6ai_chunk *chunk = list->next;
>+  while (n > 0)
>+    {
>+      n = (count > IN6AI_CHUNK_SIZE) ? IN6AI_CHUNK_SIZE : count;
>+      memcpy(p, list->info, n * sizeof(struct in6addrinfo));
>+      p += n;
>+      count -= n;
>+      chunk = chunk->next;
>+    }
>+
>+  result->in6ailen = list->count;
>+  return result;
>+}
>+
>+
>+static void
>+in6ailist_cleanup(struct in6ai_list *list)
>+{
>+  struct in6ai_chunk *p = list->next;
>+  struct in6ai_chunk *next;
>+
>+  while (p)
>+    {
>+      next = p->next;
>+      free(p);
>+      p = next;
>+    }
>+}
>+
>+
> static struct cached_data *
> make_request (int fd, pid_t pid)
> {
>@@ -129,6 +229,9 @@ make_request (int fd, pid_t pid)
>   memset (&nladdr, '\0', sizeof (nladdr));
>   nladdr.nl_family = AF_NETLINK;
> 
>+  struct in6ai_list in6ai_list;
>+  in6ailist_init(&in6ai_list);
>+
> #ifdef PAGE_SIZE
>   /* Help the compiler optimize out the malloc call if PAGE_SIZE
>      is constant and smaller or equal to PTHREAD_STACK_MIN/4.  */
>@@ -158,12 +261,6 @@ make_request (int fd, pid_t pid)
>     goto out_fail;
> 
>   bool done = false;
>-  struct in6ailist
>-  {
>-    struct in6addrinfo info;
>-    struct in6ailist *next;
>-  } *in6ailist = NULL;
>-  size_t in6ailistlen = 0;
>   bool seen_ipv4 = false;
>   bool seen_ipv6 = false;
> 
>@@ -238,28 +335,27 @@ make_request (int fd, pid_t pid)
> 		    }
> 		}
> 
>-	      struct in6ailist *newp = alloca (sizeof (*newp));
>-	      newp->info.flags = (((ifam->ifa_flags
>-				    & (IFA_F_DEPRECATED
>-				       | IFA_F_OPTIMISTIC))
>-				   ? in6ai_deprecated : 0)
>-				  | ((ifam->ifa_flags
>-				      & IFA_F_HOMEADDRESS)
>-				     ? in6ai_homeaddress : 0));
>-	      newp->info.prefixlen = ifam->ifa_prefixlen;
>-	      newp->info.index = ifam->ifa_index;
>+	      struct in6addrinfo *info = in6ailist_add(&in6ai_list);
>+	      if (!info)
>+                goto out_fail;
>+	      info->flags = (((ifam->ifa_flags
>+			       & (IFA_F_DEPRECATED
>+				  | IFA_F_OPTIMISTIC))
>+			      ? in6ai_deprecated : 0)
>+			     | ((ifam->ifa_flags
>+				 & IFA_F_HOMEADDRESS)
>+				? in6ai_homeaddress : 0));
>+	      info->prefixlen = ifam->ifa_prefixlen;
>+	      info->index = ifam->ifa_index;
> 	      if (ifam->ifa_family == AF_INET)
> 		{
>-		  newp->info.addr[0] = 0;
>-		  newp->info.addr[1] = 0;
>-		  newp->info.addr[2] = htonl (0xffff);
>-		  newp->info.addr[3] = *(const in_addr_t *) address;
>+		  info->addr[0] = 0;
>+		  info->addr[1] = 0;
>+		  info->addr[2] = htonl (0xffff);
>+		  info->addr[3] = *(const in_addr_t *) address;
> 		}
> 	      else
>-		memcpy (newp->info.addr, address, sizeof (newp->info.addr));
>-	      newp->next = in6ailist;
>-	      in6ailist = newp;
>-	      ++in6ailistlen;
>+		memcpy (info->addr, address, sizeof (info->addr));
> 	    }
> 	  else if (nlmh->nlmsg_type == NLMSG_DONE)
> 	    /* We found the end, leave the loop.  */
>@@ -269,10 +365,9 @@ make_request (int fd, pid_t pid)
>   while (! done);
> 
>   struct cached_data *result;
>-  if (seen_ipv6 && in6ailist != NULL)
>+  if (seen_ipv6 && in6ai_list.count > 0)
>     {
>-      result = malloc (sizeof (*result)
>-		       + in6ailistlen * sizeof (struct in6addrinfo));
>+      result = in6ailist_export(&in6ai_list);
>       if (result == NULL)
> 	goto out_fail;
> 
>@@ -280,14 +375,6 @@ make_request (int fd, pid_t pid)
>       result->usecnt = 2;
>       result->seen_ipv4 = seen_ipv4;
>       result->seen_ipv6 = true;
>-      result->in6ailen = in6ailistlen;
>-
>-      do
>-	{
>-	  result->in6ai[--in6ailistlen] = in6ailist->info;
>-	  in6ailist = in6ailist->next;
>-	}
>-      while (in6ailist != NULL);
>     }
>   else
>     {
>@@ -297,11 +384,13 @@ make_request (int fd, pid_t pid)
>       result = &noai6ai_cached;
>     }
> 
>+  in6ailist_cleanup(&in6ai_list);
>   if (use_malloc)
>     free (buf);
>   return result;
> 
> out_fail:
>+  in6ailist_cleanup(&in6ai_list);
>   if (use_malloc)
>     free (buf);
>   return NULL;
>-- 
>1.7.10.4
>

Actions: View | Diff

Attachments on bug 14806: 6720