diff options
author | Simon McVittie <smcv@collabora.com> | 2017-07-04 15:38:57 +0100 |
---|---|---|
committer | Simon McVittie <smcv@collabora.com> | 2017-07-05 13:13:35 +0100 |
commit | 031aa2ceb3dfff373e7b398dfc5d020d77262512 (patch) | |
tree | 5cd55eed1dbaf9d3a4d5df7d8aed804e56704da5 /dbus | |
parent | 8384e795516066960bb9fcfbfe138f569420edb9 (diff) | |
download | dbus-031aa2ceb3dfff373e7b398dfc5d020d77262512.tar.gz |
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
Diffstat (limited to 'dbus')
-rw-r--r-- | dbus/dbus-message.c | 25 |
1 files changed, 16 insertions, 9 deletions
diff --git a/dbus/dbus-message.c b/dbus/dbus-message.c index 9c2d5d1e..a3354f5d 100644 --- 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; } |