Attachment 88264 Details for Bug 21259 – [PATCH v4 2/3] Fix memory or unix fd may leak in dbus_message_iter_get_args_valist

[patch] [PATCH v4 2/3] Fix memory or unix fd may leak in dbus_message_iter_get_args_valist

0002-Fix-memory-or-unix-fd-may-leak-in-dbus_message_iter_.patch (text/plain), 4.17 KB, created by Chengwei Yang on 2013-10-29 02:34:55 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Chengwei Yang

Created: 2013-10-29 02:34:55 UTC

Size: 4.17 KB

patch

obsolete

>From 17ad103def30b89bd3bcd8581275b81f2cf8eb8e Mon Sep 17 00:00:00 2001
>From: Chengwei Yang <chengwei.yang@intel.com>
>Date: Tue, 29 Oct 2013 10:12:25 +0800
>Subject: [PATCH v4 2/3] Fix memory or unix fd may leak in
> dbus_message_iter_get_args_valist
>
>This is an aged bug since 2009, so let's fix it. Say if a previous
>parsing for unix fd or array of string successfully but then a later
>element parsing fail, then the unix fd or array of string leaked.
>
>https://bugs.freedesktop.org/show_bug.cgi?id=21259
>---
> dbus/dbus-message.c |   71 ++++++++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 67 insertions(+), 4 deletions(-)
>
>diff --git a/dbus/dbus-message.c b/dbus/dbus-message.c
>index 4f234d5..54569cf 100644
>--- a/dbus/dbus-message.c
>+++ b/dbus/dbus-message.c
>@@ -784,8 +784,6 @@ _dbus_message_iter_check (DBusMessageRealIter *iter)
>  * dbus_message_get_args() is the place to go for complete
>  * documentation.
>  *
>- * @todo This may leak memory and file descriptors if parsing fails. See #21259
>- *
>  * @see dbus_message_get_args
>  * @param iter the message iter
>  * @param error error to be filled in
>@@ -802,6 +800,7 @@ _dbus_message_iter_get_args_valist (DBusMessageIter *iter,
>   DBusMessageRealIter *real = (DBusMessageRealIter *)iter;
>   int spec_type, msg_type, i, j;
>   dbus_bool_t retval;
>+  va_list copy_args;
> 
>   _dbus_assert (_dbus_message_iter_check (real));
> 
>@@ -810,12 +809,17 @@ _dbus_message_iter_get_args_valist (DBusMessageIter *iter,
>   spec_type = first_arg_type;
>   i = 0;
> 
>+  /* copy var_args first, then we can do another iteration over it to
>+   * free memory and close unix fds if parse failed at some point.
>+   */
>+  va_copy (copy_args, var_args);
>+
>   while (spec_type != DBUS_TYPE_INVALID)
>     {
>       msg_type = dbus_message_iter_get_arg_type (iter);
> 
>       if (msg_type != spec_type)
>-	{
>+        {
>           dbus_set_error (error, DBUS_ERROR_INVALID_ARGS,
>                           "Argument %d is specified to be of type \"%s\", but "
>                           "is actually of type \"%s\"\n", i,
>@@ -823,7 +827,7 @@ _dbus_message_iter_get_args_valist (DBusMessageIter *iter,
>                           _dbus_type_to_string (msg_type));
> 
>           goto out;
>-	}
>+        }
> 
>       if (spec_type == DBUS_TYPE_UNIX_FD)
>         {
>@@ -997,7 +1001,66 @@ _dbus_message_iter_get_args_valist (DBusMessageIter *iter,
>   retval = TRUE;
> 
>  out:
>+  /* there may memory or unix fd leak in the above iteration if parse failed.
>+   * so we have another iteration over copy_args to free memory and close
>+   * unix fds.
>+   */
>+  if (!retval)
>+    {
>+      spec_type = first_arg_type;
>+      j = 0;
>+
>+      while (j < i)
>+        {
>+          if (spec_type == DBUS_TYPE_UNIX_FD)
>+            {
>+#ifdef HAVE_UNIX_FD_PASSING
>+              int *pfd;
>+
>+              pfd = va_arg (copy_args, int *);
>+              _dbus_assert(pfd);
>+              if (*pfd > 0)
>+                {
>+                  _dbus_close (*pfd, NULL);
>+                  *pfd = -1;
>+                }
>+#endif
>+            }
>+          else if (dbus_type_is_basic (spec_type))
>+            {
>+              /* move the index forward */
>+              va_arg (copy_args, DBusBasicValue *);
>+            }
>+          else if (spec_type == DBUS_TYPE_ARRAY)
>+            {
>+              int spec_element_type;
>+
>+              spec_element_type = va_arg (copy_args, int);
>+              if (dbus_type_is_fixed (spec_element_type))
>+                {
>+                  /* move the index forward */
>+                  va_arg (copy_args, const DBusBasicValue **);
>+                  va_arg (copy_args, int *);
>+                }
>+              else if (_DBUS_TYPE_IS_STRINGLIKE (spec_element_type))
>+                {
>+                  char ***str_array_p;
>+
>+                  str_array_p = va_arg (copy_args, char ***);
>+                  /* move the index forward */
>+                  va_arg (copy_args, int *);
>+                  _dbus_assert (str_array_p != NULL);
>+                  dbus_free_string_array (*str_array_p);
>+                  *str_array_p = NULL;
>+                }
>+            }
>+
>+          spec_type = va_arg (copy_args, int);
>+          j++;
>+        }
>+    }
> 
>+  va_end (copy_args);
>   return retval;
> }
> 
>-- 
>1.7.9.5
>

Actions: View | Splinter Review

Attachments on bug 21259: 86707 | 87428 | 87430 | 87431 | 87434 | 87435 | 87436 | 87437 | 87438 | 87439 | 88263 | 88264 | 88265 | 88293

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.