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.

Change-Id: I2ccd4b516c7714f64c4543dd8d2e5c99633733a5
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Philip Withnall <withnall@endlessm.com>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=101568

diff --git a/dbus/dbus-message.c b/dbus/dbus-message.c
index 18388a1..93ffb40 100644 (file)
--- a/dbus/dbus-message.c
+++ b/dbus/dbus-message.c
@@ -3047,6 +3047,7 @@ dbus_message_iter_open_container (DBusMessageIter *iter,
   DBusMessageRealIter *real = (DBusMessageRealIter *)iter;
   DBusMessageRealIter *real_sub = (DBusMessageRealIter *)sub;
   DBusString contained_str;
+  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);
@@ -3073,24 +3074,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,
+      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,
+      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;
 }