Attachment 132432 Details for Bug 101568 – [06] dbus_message_iter_open_container: Don't leak signature on failure

[patch] [06] dbus_message_iter_open_container: Don't leak signature on failure

0010-dbus_message_iter_open_container-Don-t-leak-signatur.patch (text/plain), 3.22 KB, created by Simon McVittie on 2017-07-04 17:58:00 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Simon McVittie

Created: 2017-07-04 17:58:00 UTC

Size: 3.22 KB

patch

obsolete

>From 9cbe4304facf8157e908a1fd05c045e6f5411950 Mon Sep 17 00:00:00 2001
>From: Simon McVittie <smcv@collabora.com>
>Date: Tue, 4 Jul 2017 15:38:57 +0100
>Subject: [PATCH 10/19] dbus_message_iter_open_container: Don't leak signature
> on failure
>
>If we run out of memory while calling _dbus_type_writer_recurse()
>(which is impossible for most contained types, but can happen for
>structs and dict-entries), then the memory we allocated in the call to
>_dbus_message_iter_open_signature() will still be allocated, and we
>have to free it in order to return to the state of the world prior to
>calling open_container().
>
>One might reasonably worry that this change can break callers that use
>this (incorrect) pattern:
>
>    if (!dbus_message_iter_open_container (outer, ..., inner))
>      {
>        dbus_message_iter_abandon_container (outer, inner);
>        goto fail;
>      }
>    /* now we know inner is open, and we must close it later */
>
>However, testing that pattern with _dbus_test_oom_handling()
>demonstrates that it already dies with a DBusString assertion failure
>even before this commit.
>
>This is all concerningly fragile, and I think the next step should be
>to zero out DBusMessageIter instances when they are invalidated, so
>that a "double-free" is always detected.
>
>Signed-off-by: Simon McVittie <smcv@collabora.com>
>---
> dbus/dbus-message.c | 25 ++++++++++++++++---------
> 1 file changed, 16 insertions(+), 9 deletions(-)
>
>diff --git a/dbus/dbus-message.c b/dbus/dbus-message.c
>index 234ddb23..119d8ac4 100644
>--- a/dbus/dbus-message.c
>+++ b/dbus/dbus-message.c
>@@ -2905,6 +2905,7 @@ dbus_message_iter_open_container (DBusMessageIter *iter,
>   DBusMessageRealIter *real_sub = (DBusMessageRealIter *)sub;
>   DBusString contained_str;
>   DBusValidity contained_signature_validity;
>+  dbus_bool_t ret;
> 
>   _dbus_return_val_if_fail (_dbus_message_iter_append_check (real), FALSE);
>   _dbus_return_val_if_fail (real->iter_type == DBUS_MESSAGE_ITER_TYPE_WRITER, FALSE);
>@@ -2947,24 +2948,30 @@ dbus_message_iter_open_container (DBusMessageIter *iter,
>   if (!_dbus_message_iter_open_signature (real))
>     return FALSE;
> 
>+  ret = FALSE;
>   *real_sub = *real;
> 
>   if (contained_signature != NULL)
>     {
>       _dbus_string_init_const (&contained_str, contained_signature);
> 
>-      return _dbus_type_writer_recurse (&real->u.writer,
>-                                        type,
>-                                        &contained_str, 0,
>-                                        &real_sub->u.writer);
>+      ret = _dbus_type_writer_recurse (&real->u.writer,
>+                                       type,
>+                                       &contained_str, 0,
>+                                       &real_sub->u.writer);
>     }
>   else
>     {
>-      return _dbus_type_writer_recurse (&real->u.writer,
>-                                        type,
>-                                        NULL, 0,
>-                                        &real_sub->u.writer);
>-    } 
>+      ret = _dbus_type_writer_recurse (&real->u.writer,
>+                                       type,
>+                                       NULL, 0,
>+                                       &real_sub->u.writer);
>+    }
>+
>+  if (!ret)
>+    _dbus_message_iter_abandon_signature (real);
>+
>+  return ret;
> }
> 
> 
>-- 
>2.13.2
>

Actions: View | Splinter Review

Attachments on bug 101568: 132161 | 132162 | 132412 | 132413 | 132414 | 132415 | 132416 | 132417 | 132418 | 132427 | 132428 | 132429 | 132430 | 132431 | 132432 | 132433 | 132434 | 132435 | 132436 | 132437 | 132438 | 132439 | 132440 | 132441 | 132442 | 132447 | 132454

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.