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>
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 9c2d5d1..a3354f5 100644 (file)
--- a/dbus/dbus-message.c
+++ b/dbus/dbus-message.c
@@ -2908,6 +2908,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);
@@ -2950,24 +2951,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;
 }