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
This commit is contained in:
Simon McVittie 2017-07-04 15:38:57 +01:00
parent 8384e79551
commit 031aa2ceb3

View file

@ -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;
}