From 52e3f6aeeb805db5051f33f9fdf79671dbe839bc Mon Sep 17 00:00:00 2001 From: "satorux@chromium.org" Date: Thu, 4 Aug 2011 21:57:21 +0000 Subject: Fix subtle memory issues detected by valgrind. The size of dbus_bool_t and the size of bool are different. The former is always 4, whereas the latter is usually 1. The misuse of these types causes subtle memory issues where some D-Bus functions read 4 bytes from a bool pointer, or write 4 bytes to it. These functions take void* so type checking didn't help. TEST=tools/valgrind/valgrind.sh ninja/dbus_unittests BUG=none Review URL: http://codereview.chromium.org/7573001 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@95512 0039d316-1c4b-4281-b951-d872f2087c98 --- dbus/message.cc | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) (limited to 'dbus') diff --git a/dbus/message.cc b/dbus/message.cc index 28f8960..8118202 100644 --- a/dbus/message.cc +++ b/dbus/message.cc @@ -261,7 +261,13 @@ void MessageWriter::AppendByte(uint8 value) { } void MessageWriter::AppendBool(bool value) { - AppendBasic(DBUS_TYPE_BOOLEAN, &value); + // The size of dbus_bool_t and the size of bool are different. The + // former is always 4 per dbus-types.h, whereas the latter is usually 1. + // dbus_message_iter_append_basic() used in AppendBasic() expects four + // bytes for DBUS_TYPE_BOOLEAN, so we must pass a dbus_bool_t, instead + // of a bool, to AppendBasic(). + dbus_bool_t dbus_value = value; + AppendBasic(DBUS_TYPE_BOOLEAN, &dbus_value); } void MessageWriter::AppendInt16(int16 value) { @@ -390,7 +396,9 @@ void MessageWriter::AppendVariantOfByte(uint8 value) { } void MessageWriter::AppendVariantOfBool(bool value) { - AppendVariantOfBasic(DBUS_TYPE_BOOLEAN, &value); + // See the comment at MessageWriter::AppendBool(). + dbus_bool_t dbus_value = value; + AppendVariantOfBasic(DBUS_TYPE_BOOLEAN, &dbus_value); } void MessageWriter::AppendVariantOfInt16(int16 value) { @@ -473,7 +481,13 @@ bool MessageReader::PopByte(uint8* value) { } bool MessageReader::PopBool(bool* value) { - return PopBasic(DBUS_TYPE_BOOLEAN, value); + // Like MessageWriter::AppendBool(), we should copy |value| to + // dbus_bool_t, as dbus_message_iter_get_basic() used in PopBasic() + // expects four bytes for DBUS_TYPE_BOOLEAN. + dbus_bool_t dbus_value = FALSE; + const bool success = PopBasic(DBUS_TYPE_BOOLEAN, &dbus_value); + *value = static_cast(dbus_value); + return success; } bool MessageReader::PopInt16(int16* value) { @@ -569,7 +583,11 @@ bool MessageReader::PopVariantOfByte(uint8* value) { } bool MessageReader::PopVariantOfBool(bool* value) { - return PopVariantOfBasic(DBUS_TYPE_BOOLEAN, value); + // See the comment at MessageReader::PopBool(). + dbus_bool_t dbus_value = FALSE; + const bool success = PopVariantOfBasic(DBUS_TYPE_BOOLEAN, &dbus_value); + *value = static_cast(dbus_value); + return success; } bool MessageReader::PopVariantOfInt16(int16* value) { -- cgit v1.1