This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH 1/1] Fix __check_pf()/make_request() stack overflow segfault(convert to alloca_account) and memory leak.


This patch converts make_request() to use alloca_account() so it doesn't
overflow on the stack since it's grabbing essentially unbounded data from
netlink. It also fixes a memory leak caused by the special casing for NSCD
support in the non-NSCD case. I don't know why NSCD caching is in this
layer, however I just made it work as designed. The unnecessary
__libc_lock_lock() call was removed when not built for NSCD, and potential
race on free fixed when it is built for NSCD. Tested with valgrind and
very large netlink responses, both with and without ipv6 addresses.

diff --git a/ChangeLog b/ChangeLog
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2013-02-14  Debabrata Banerjee <dbanerje@akamai.com>
+       
+       * sysdeps/unix/sysv/linux/check_pf.c: use alloca_account for
check_pf()
+       fix memory leak and locking when not building in/with NSCD.
+       
 2013-02-13  Joseph Myers  <joseph@codesourcery.com>
 
 	[BZ #13550]
diff --git a/sysdeps/unix/sysv/linux/check_pf.c
b/sysdeps/unix/sysv/linux/check_pf.c
--- a/sysdeps/unix/sysv/linux/check_pf.c
+++ b/sysdeps/unix/sysv/linux/check_pf.c
@@ -138,9 +138,12 @@ make_request (int fd, pid_t pid)
 #endif
   bool use_malloc = false;
   char *buf;
+  size_t alloca_used = 0;
+  size_t result_size = 0;
+  struct cached_data *result = NULL;
 
   if (__libc_use_alloca (buf_size))
-    buf = alloca (buf_size);
+    buf = alloca_account (buf_size, alloca_used);
   else
     {
       buf = malloc (buf_size);
@@ -238,7 +241,10 @@ make_request (int fd, pid_t pid)
 		    }
 		}
 
-	      struct in6ailist *newp = alloca (sizeof (*newp));
+	      if (__libc_use_alloca (alloca_used + sizeof (struct in6ailist)))
+	        {
+	          struct in6ailist *newp =
+	              alloca_account (sizeof (*newp), alloca_used);
 	      newp->info.flags = (((ifam->ifa_flags
 				    & (IFA_F_DEPRECATED
 				       | IFA_F_OPTIMISTIC))
@@ -261,6 +267,58 @@ make_request (int fd, pid_t pid)
 	      in6ailist = newp;
 	      ++in6ailistlen;
 	    }
+	      else
+	        {
+	          size_t allocate_size;
+
+	          if (result_size < sizeof (*result)
+	              + ((in6ailistlen + 1) * sizeof (struct in6addrinfo)))
+	            {
+	              allocate_size = sizeof (*result)
+	                      + (2 * in6ailistlen * sizeof (struct in6addrinfo));
+	              void *newbuf = realloc (result, allocate_size);
+	              if (newbuf != NULL)
+	                result = newbuf;
+	              else
+	                goto out_fail;
+
+                      if (result_size == 0)
+                        {
+                          int i = 0;
+                          do
+                            {
+                              result->in6ai[i++] = in6ailist->info;
+                              in6ailist = in6ailist->next;
+                            }
+                          while (in6ailist != NULL);
+                        }
+
+                      result_size = allocate_size;
+	            }
+
+	          result->in6ai[in6ailistlen].flags = (((ifam->ifa_flags
+	              & (IFA_F_DEPRECATED
+	                  | IFA_F_OPTIMISTIC))
+	              ? in6ai_deprecated : 0)
+	              | ((ifam->ifa_flags
+	                  & IFA_F_HOMEADDRESS)
+	                  ? in6ai_homeaddress : 0));
+	          result->in6ai[in6ailistlen].prefixlen = ifam->ifa_prefixlen;
+	          result->in6ai[in6ailistlen].index = ifam->ifa_index;
+	          if (ifam->ifa_family == AF_INET)
+	            {
+	              result->in6ai[in6ailistlen].addr[0] = 0;
+	              result->in6ai[in6ailistlen].addr[1] = 0;
+	              result->in6ai[in6ailistlen].addr[2] = htonl (0xffff);
+	              result->in6ai[in6ailistlen].addr[3] = *(const in_addr_t *)
address;
+	            }
+	          else
+	            memcpy (result->in6ai[in6ailistlen].addr, address,
+	                sizeof (result->in6ai[in6ailistlen].addr));
+
+	          ++in6ailistlen;
+	        }
+	    }
 	  else if (nlmh->nlmsg_type == NLMSG_DONE)
 	    /* We found the end, leave the loop.  */
 	    done = true;
@@ -268,20 +326,15 @@ make_request (int fd, pid_t pid)
     }
   while (! done);
 
-  struct cached_data *result;
-  if (seen_ipv6 && in6ailist != NULL)
+  if (seen_ipv6)
+    {
+      if (result == NULL && in6ailist != NULL)
     {
       result = malloc (sizeof (*result)
 		       + in6ailistlen * sizeof (struct in6addrinfo));
       if (result == NULL)
 	goto out_fail;
 
-      result->timestamp = get_nl_timestamp ();
-      result->usecnt = 2;
-      result->seen_ipv4 = seen_ipv4;
-      result->seen_ipv6 = true;
-      result->in6ailen = in6ailistlen;
-
       do
 	{
 	  result->in6ai[--in6ailistlen] = in6ailist->info;
@@ -289,9 +342,29 @@ make_request (int fd, pid_t pid)
 	}
       while (in6ailist != NULL);
     }
+
+#ifdef IS_IN_nscd
+      result->timestamp = nl_timestamp;
+      result->usecnt = 2;
+#elif defined(USE_NSCD)
+      result->timestamp = __nscd_get_nl_timestamp ();
+      result->usecnt = 2;
+#else
+      result->usecnt = 1;
+#endif
+      result->seen_ipv4 = seen_ipv4;
+      result->seen_ipv6 = true;
+      result->in6ailen = in6ailistlen;
+    }
   else
     {
+      if (result)
+        free (result);
+#if defined(IS_IN_nscd) || defined(USE_NSCD)
       atomic_add (&noai6ai_cached.usecnt, 2);
+#else
+      atomic_add (&noai6ai_cached.usecnt, 1);
+#endif
       noai6ai_cached.seen_ipv4 = seen_ipv4;
       noai6ai_cached.seen_ipv6 = seen_ipv6;
       result = &noai6ai_cached;
@@ -304,6 +377,8 @@ make_request (int fd, pid_t pid)
 out_fail:
   if (use_malloc)
     free (buf);
+  if (result)
+    free (result);
   return NULL;
 }
 
@@ -319,6 +394,7 @@ __check_pf (bool *seen_ipv4, bool *seen_ipv6,
   struct cached_data *olddata = NULL;
   struct cached_data *data = NULL;
 
+#if defined(IS_IN_nscd) || defined(USE_NSCD)
   __libc_lock_lock (lock);
 
   if (cache_valid_p ())
@@ -327,6 +403,7 @@ __check_pf (bool *seen_ipv4, bool *seen_ipv6,
       atomic_increment (&cache->usecnt);
     }
   else
+#endif
     {
       int fd = __socket (PF_NETLINK, SOCK_RAW, NETLINK_ROUTE);
 
@@ -346,14 +423,21 @@ __check_pf (bool *seen_ipv4, bool *seen_ipv6,
 	  close_not_cancel_no_status (fd);
 	}
 
-      if (data != NULL)
+#if defined(IS_IN_nscd) || defined(USE_NSCD)
+      if (data != cache)
 	{
 	  olddata = cache;
 	  cache = data;
+
+          if (olddata != NULL && atomic_add_zero (&olddata->usecnt, -1))
+            free (olddata);
 	}
     }
 
   __libc_lock_unlock (lock);
+#else
+    }
+#endif
 
   if (data != NULL)
     {
@@ -385,16 +469,16 @@ __free_in6ai (struct in6addrinfo *ai)
       struct cached_data *data =
 	(struct cached_data *) ((char *) ai
 				- offsetof (struct cached_data, in6ai));
-
-      if (atomic_add_zero (&data->usecnt, -1))
-	{
+#if defined(IS_IN_nscd) || defined(USE_NSCD)
 	  __libc_lock_lock (lock);
 
-	  if (data->usecnt == 0)
-	    /* Still unused.  */
+       if (atomic_add_zero (&data->usecnt, -1))
 	    free (data);
 
 	  __libc_lock_unlock (lock);
-	}
+#else
+       if (atomic_add_zero (&data->usecnt, -1))
+         free(data);
+#endif
     }
 }



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]