From b5a1f3c54a48ea3079622b0ec3023c79b95ed135 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 1 Apr 2009 12:02:00 -0400 Subject: Bug 17803 - Fix both test case and validation logic The previous commit had errors in both the test case and the validation logic. The test case was missing a trailing comma before the previous one, so we weren't testing the signature we thought we were. The validation logic was wrong because if the type was not valid, we'd drop through the entire if clause, and thus skip returning an error code, and accept the signature. --- dbus/dbus-marshal-validate-util.c | 2 +- dbus/dbus-marshal-validate.c | 13 +++++++------ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/dbus/dbus-marshal-validate-util.c b/dbus/dbus-marshal-validate-util.c index 5365d6d3..ac901c38 100644 --- a/dbus/dbus-marshal-validate-util.c +++ b/dbus/dbus-marshal-validate-util.c @@ -227,7 +227,7 @@ _dbus_marshal_validate_test (void) "not a valid signature", "123", ".", - "(" + "(", "a{(ii)i}" /* https://bugs.freedesktop.org/show_bug.cgi?id=17803 */ }; diff --git a/dbus/dbus-marshal-validate.c b/dbus/dbus-marshal-validate.c index 35998cbb..ee955485 100644 --- a/dbus/dbus-marshal-validate.c +++ b/dbus/dbus-marshal-validate.c @@ -246,14 +246,15 @@ _dbus_validate_signature_with_reason (const DBusString *type_str, } } - if (last == DBUS_DICT_ENTRY_BEGIN_CHAR && - _dbus_type_is_valid (*p) && - !dbus_type_is_basic (*p)) + if (last == DBUS_DICT_ENTRY_BEGIN_CHAR) { - result = DBUS_INVALID_DICT_KEY_MUST_BE_BASIC_TYPE; - goto out; + if (!(_dbus_type_is_valid (*p) && dbus_type_is_basic (*p))) + { + result = DBUS_INVALID_DICT_KEY_MUST_BE_BASIC_TYPE; + goto out; + } } - + last = *p; ++p; } -- cgit From a709566edd8358ba431b7427a1530a7db0d1832d Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 12 Mar 2009 10:31:54 -0400 Subject: Always append closing quote in log command Patch suggested by Tomas Hoger --- bus/connection.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/bus/connection.c b/bus/connection.c index ab99fa5f..9159c898 100644 --- a/bus/connection.c +++ b/bus/connection.c @@ -575,12 +575,11 @@ cache_peer_loginfo_string (BusConnectionData *d, } if (!_dbus_string_append_printf (&loginfo_buf, "pid=%ld comm=\"", pid)) goto oom; - /* Ignore errors here */ - if (_dbus_command_for_pid (pid, &loginfo_buf, MAX_LOG_COMMAND_LEN, NULL)) - { - if (!_dbus_string_append_byte (&loginfo_buf, '"')) - goto oom; - } + /* Ignore errors here; we may not have permissions to read the + * proc file. */ + _dbus_command_for_pid (pid, &loginfo_buf, MAX_LOG_COMMAND_LEN, NULL); + if (!_dbus_string_append_byte (&loginfo_buf, '"')) + goto oom; } if (dbus_connection_get_windows_user (connection, &windows_sid)) -- cgit From 15f518301605ed748fbcecdf5e38d0a5ef982c3b Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 25 Feb 2009 11:10:15 -0500 Subject: Bug 20137 - Fix alignment usage when demarshaling basics We can't safely type-pun from e.g. char * to DBusBasicValue *, because the latter has higher alignment requirements. Instead, create an explicit pointer for each case. Also, we mark each one volatile to sidestep strict aliasing issues, for the future when we turn on strict aliasing support. Original patch and review from Jay Estabrook . --- dbus/dbus-marshal-basic.c | 40 ++++++++++++++++++++++++++++------------ 1 file changed, 28 insertions(+), 12 deletions(-) diff --git a/dbus/dbus-marshal-basic.c b/dbus/dbus-marshal-basic.c index 724d94b8..38fbe2d6 100644 --- a/dbus/dbus-marshal-basic.c +++ b/dbus/dbus-marshal-basic.c @@ -508,59 +508,74 @@ _dbus_marshal_read_basic (const DBusString *str, int *new_pos) { const char *str_data; - DBusBasicValue *vp; _dbus_assert (dbus_type_is_basic (type)); str_data = _dbus_string_get_const_data (str); - vp = value; + /* Below we volatile types to avoid aliasing issues; + * see http://bugs.freedesktop.org/show_bug.cgi?id=20137 + */ + switch (type) { case DBUS_TYPE_BYTE: - vp->byt = _dbus_string_get_byte (str, pos); + { + volatile unsigned char *vp = value; + *vp = (unsigned char) _dbus_string_get_byte (str, pos); (pos)++; + } break; case DBUS_TYPE_INT16: case DBUS_TYPE_UINT16: + { + volatile dbus_uint16_t *vp = value; pos = _DBUS_ALIGN_VALUE (pos, 2); - vp->u16 = *(dbus_uint16_t *)(str_data + pos); + *vp = *(dbus_uint16_t *)(str_data + pos); if (byte_order != DBUS_COMPILER_BYTE_ORDER) - vp->u16 = DBUS_UINT16_SWAP_LE_BE (vp->u16); + *vp = DBUS_UINT16_SWAP_LE_BE (*vp); pos += 2; + } break; case DBUS_TYPE_INT32: case DBUS_TYPE_UINT32: case DBUS_TYPE_BOOLEAN: + { + volatile dbus_uint32_t *vp = value; pos = _DBUS_ALIGN_VALUE (pos, 4); - vp->u32 = *(dbus_uint32_t *)(str_data + pos); + *vp = *(dbus_uint32_t *)(str_data + pos); if (byte_order != DBUS_COMPILER_BYTE_ORDER) - vp->u32 = DBUS_UINT32_SWAP_LE_BE (vp->u32); + *vp = DBUS_UINT32_SWAP_LE_BE (*vp); pos += 4; + } break; case DBUS_TYPE_INT64: case DBUS_TYPE_UINT64: case DBUS_TYPE_DOUBLE: + { + volatile dbus_uint64_t *vp = value; pos = _DBUS_ALIGN_VALUE (pos, 8); #ifdef DBUS_HAVE_INT64 if (byte_order != DBUS_COMPILER_BYTE_ORDER) - vp->u64 = DBUS_UINT64_SWAP_LE_BE (*(dbus_uint64_t*)(str_data + pos)); + *vp = DBUS_UINT64_SWAP_LE_BE (*(dbus_uint64_t*)(str_data + pos)); else - vp->u64 = *(dbus_uint64_t*)(str_data + pos); + *vp = *(dbus_uint64_t*)(str_data + pos); #else - vp->u64 = *(DBus8ByteStruct*) (str_data + pos); + *vp = *(DBus8ByteStruct*) (str_data + pos); swap_8_octets (vp, byte_order); #endif pos += 8; + } break; case DBUS_TYPE_STRING: case DBUS_TYPE_OBJECT_PATH: { int len; + volatile char **vp = value; len = _dbus_marshal_read_uint32 (str, pos, byte_order, &pos); - vp->str = (char*) str_data + pos; + *vp = (char*) str_data + pos; pos += len + 1; /* length plus nul */ } @@ -568,11 +583,12 @@ _dbus_marshal_read_basic (const DBusString *str, case DBUS_TYPE_SIGNATURE: { int len; + volatile char **vp = value; len = _dbus_string_get_byte (str, pos); pos += 1; - vp->str = (char*) str_data + pos; + *vp = (char*) str_data + pos; pos += len + 1; /* length plus nul */ } -- cgit From 5b4ee5fb40269afaa106b55dd4755125c2f9107a Mon Sep 17 00:00:00 2001 From: Johan Gyllenspetz Date: Tue, 17 Mar 2009 17:26:03 -0400 Subject: Bug 20494 - Fix signed confusion for dbus_message_get_reply_serial return We were incorrectly converting the serial to a signed integer and comparing it to -1. Signed-off-by: Colin Walters --- dbus/dbus-connection.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c index a960a991..ae07adf0 100644 --- a/dbus/dbus-connection.c +++ b/dbus/dbus-connection.c @@ -448,7 +448,7 @@ _dbus_connection_queue_received_message_link (DBusConnection *connection, DBusList *link) { DBusPendingCall *pending; - dbus_int32_t reply_serial; + dbus_uint32_t reply_serial; DBusMessage *message; _dbus_assert (_dbus_transport_get_is_authenticated (connection->transport)); @@ -459,7 +459,7 @@ _dbus_connection_queue_received_message_link (DBusConnection *connection, /* If this is a reply we're waiting on, remove timeout for it */ reply_serial = dbus_message_get_reply_serial (message); - if (reply_serial != -1) + if (reply_serial != 0) { pending = _dbus_hash_table_lookup_int (connection->pending_replies, reply_serial); -- cgit From da75989b3918508058ed056ae9e2092e14023ebc Mon Sep 17 00:00:00 2001 From: Eamon Walsh Date: Fri, 20 Mar 2009 00:26:42 -0400 Subject: dbus-launch: use InputOnly X window Working on SELinux policy for X, and came across this issue in dbus-launch: Windows created for use as property/selection placeholders should be of class InputOnly, since no drawing is ever done to them. Signed-off-by: Eamon Walsh Signed-off-by: Thiago Macieira --- tools/dbus-launch-x11.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/dbus-launch-x11.c b/tools/dbus-launch-x11.c index 442e9ba2..b1824181 100644 --- a/tools/dbus-launch-x11.c +++ b/tools/dbus-launch-x11.c @@ -361,9 +361,9 @@ set_address_in_x11(char *address, pid_t pid) } /* Create our window */ - wid = XCreateSimpleWindow (xdisplay, RootWindow (xdisplay, 0), -20, -20, 10, 10, - 0, WhitePixel (xdisplay, 0), - BlackPixel (xdisplay, 0)); + wid = XCreateWindow (xdisplay, RootWindow (xdisplay, 0), -20, -20, 10, 10, + 0, CopyFromParent, InputOnly, CopyFromParent, + 0, NULL); verbose ("Created window %d\n", wid); /* Save the property in the window */ -- cgit From eb3b99e7c610988823804f5e6c92aa13459605c7 Mon Sep 17 00:00:00 2001 From: Kjartan Maraas Date: Tue, 21 Apr 2009 12:52:22 -0400 Subject: Bug 19502 - Sparse warning cleanups This patch makes various things that should be static static, corrects some "return FALSE" where it should be NULL, etc. Signed-off-by: Colin Walters --- bus/desktop-file.c | 2 +- bus/driver.c | 2 +- dbus/dbus-connection.c | 4 ++-- dbus/dbus-keyring.c | 4 ++-- dbus/dbus-string.c | 2 +- dbus/dbus-sysdeps-util-unix.c | 3 ++- dbus/dbus-transport.c | 2 +- test/name-test/test-names.c | 2 +- test/name-test/test-shutdown.c | 2 +- tools/dbus-launch.c | 2 +- tools/dbus-monitor.c | 2 +- 11 files changed, 14 insertions(+), 13 deletions(-) diff --git a/bus/desktop-file.c b/bus/desktop-file.c index 2fe26a11..2ba77292 100644 --- a/bus/desktop-file.c +++ b/bus/desktop-file.c @@ -66,7 +66,7 @@ typedef struct #define VALID_KEY_CHAR 1 #define VALID_LOCALE_CHAR 2 -unsigned char valid[256] = { +static unsigned char valid[256] = { 0x0 , 0x0 , 0x0 , 0x0 , 0x0 , 0x0 , 0x0 , 0x0 , 0x0 , 0x0 , 0x0 , 0x0 , 0x0 , 0x0 , 0x0 , 0x0 , 0x0 , 0x0 , 0x0 , 0x0 , 0x0 , 0x0 , 0x0 , 0x0 , 0x0 , 0x0 , 0x0 , 0x0 , 0x0 , 0x0 , 0x0 , 0x0 , 0x0 , 0x0 , 0x0 , 0x0 , 0x0 , 0x0 , 0x0 , 0x0 , 0x0 , 0x0 , 0x0 , 0x0 , 0x0 , 0x3 , 0x2 , 0x0 , diff --git a/bus/driver.c b/bus/driver.c index c97bff5d..b5138067 100644 --- a/bus/driver.c +++ b/bus/driver.c @@ -1643,7 +1643,7 @@ bus_driver_handle_get_id (DBusConnection *connection, * frequency of use (but doesn't matter with only a few items * anyhow) */ -struct +static struct { const char *name; const char *in_args; diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c index ae07adf0..d44d728b 100644 --- a/dbus/dbus-connection.c +++ b/dbus/dbus-connection.c @@ -2927,7 +2927,7 @@ dbus_connection_get_server_id (DBusConnection *connection) { char *id; - _dbus_return_val_if_fail (connection != NULL, FALSE); + _dbus_return_val_if_fail (connection != NULL, NULL); CONNECTION_LOCK (connection); id = _dbus_strdup (_dbus_transport_get_server_id (connection->transport)); @@ -3340,7 +3340,7 @@ dbus_connection_send_with_reply_and_block (DBusConnection *connection, * * @param connection the connection. */ -DBusDispatchStatus +static DBusDispatchStatus _dbus_connection_flush_unlocked (DBusConnection *connection) { /* We have to specify DBUS_ITERATION_DO_READING here because diff --git a/dbus/dbus-keyring.c b/dbus/dbus-keyring.c index de6a5ea4..8cc4048f 100644 --- a/dbus/dbus-keyring.c +++ b/dbus/dbus-keyring.c @@ -1075,7 +1075,7 @@ _dbus_keyring_test (void) dbus_error_init (&error); ring1 = _dbus_keyring_new_for_credentials (NULL, &context, &error); - _dbus_assert (ring1); + _dbus_assert (ring1 != NULL); _dbus_assert (error.name == NULL); id = _dbus_keyring_get_best_key (ring1, &error); @@ -1087,7 +1087,7 @@ _dbus_keyring_test (void) } ring2 = _dbus_keyring_new_for_credentials (NULL, &context, &error); - _dbus_assert (ring2); + _dbus_assert (ring2 != NULL); _dbus_assert (error.name == NULL); if (ring1->n_keys != ring2->n_keys) diff --git a/dbus/dbus-string.c b/dbus/dbus-string.c index 6b9b2bfe..85812c48 100644 --- a/dbus/dbus-string.c +++ b/dbus/dbus-string.c @@ -1245,7 +1245,7 @@ _dbus_string_append_printf_valist (DBusString *str, va_list args) { int len; - va_list args_copy; + va_list args_copy = 0; DBUS_STRING_PREAMBLE (str); diff --git a/dbus/dbus-sysdeps-util-unix.c b/dbus/dbus-sysdeps-util-unix.c index 03928044..d31e1441 100644 --- a/dbus/dbus-sysdeps-util-unix.c +++ b/dbus/dbus-sysdeps-util-unix.c @@ -1233,4 +1233,5 @@ fail: _dbus_string_free (&cmdline); _dbus_string_free (&path); return FALSE; -} \ No newline at end of file +} + diff --git a/dbus/dbus-transport.c b/dbus/dbus-transport.c index 90291980..35b7027d 100644 --- a/dbus/dbus-transport.c +++ b/dbus/dbus-transport.c @@ -242,7 +242,7 @@ check_address (const char *address, DBusError *error) _dbus_assert (*address != '\0'); if (!dbus_parse_address (address, &entries, &len, error)) - return FALSE; /* not a valid address */ + return NULL; /* not a valid address */ for (i = 0; i < len; i++) { diff --git a/test/name-test/test-names.c b/test/name-test/test-names.c index f4b0ba08..b09f3638 100644 --- a/test/name-test/test-names.c +++ b/test/name-test/test-names.c @@ -37,7 +37,7 @@ typedef struct { int expected_queue[NUM_CONN]; } CommandAndResult; -CommandAndResult test_data[] = { +static CommandAndResult test_data[] = { {ADD_CONNECTION, 0, ALLOW_REPLACEMENT | REPLACE_EXISTING, PRIMARY_OWNER, {0,-1,-1,-1}}, {ADD_CONNECTION, 0, REPLACE_EXISTING, diff --git a/test/name-test/test-shutdown.c b/test/name-test/test-shutdown.c index c50ef4b6..e76c1ea2 100644 --- a/test/name-test/test-shutdown.c +++ b/test/name-test/test-shutdown.c @@ -11,7 +11,7 @@ die (const char *message) } static void -open_destroy_shared_session_bus_connection () +open_destroy_shared_session_bus_connection (void) { DBusError error; DBusConnection *connection; diff --git a/tools/dbus-launch.c b/tools/dbus-launch.c index 139d0aaf..9c47d7e3 100644 --- a/tools/dbus-launch.c +++ b/tools/dbus-launch.c @@ -335,7 +335,7 @@ do_waitpid (pid_t pid) static pid_t bus_pid_to_kill = -1; static void -kill_bus() +kill_bus(void) { verbose ("Killing message bus and exiting babysitter\n"); kill (bus_pid_to_kill, SIGTERM); diff --git a/tools/dbus-monitor.c b/tools/dbus-monitor.c index ee03d854..cbd7a489 100644 --- a/tools/dbus-monitor.c +++ b/tools/dbus-monitor.c @@ -195,7 +195,7 @@ usage (char *name, int ecode) exit (ecode); } -dbus_bool_t sigint_received = FALSE; +static dbus_bool_t sigint_received = FALSE; static void sigint_handler (int signum) -- cgit From 86df8ad59229bc511689e0e1d431a5cf246685db Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 21 Apr 2009 13:11:54 -0400 Subject: Followup Bug 19502 - Don't attempt to init va_list, not portable --- dbus/dbus-string.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dbus/dbus-string.c b/dbus/dbus-string.c index 85812c48..6b9b2bfe 100644 --- a/dbus/dbus-string.c +++ b/dbus/dbus-string.c @@ -1245,7 +1245,7 @@ _dbus_string_append_printf_valist (DBusString *str, va_list args) { int len; - va_list args_copy = 0; + va_list args_copy; DBUS_STRING_PREAMBLE (str); -- cgit From 0cf4583b5a4772b9c2a381ce78f6e3a3afcf705d Mon Sep 17 00:00:00 2001 From: William Lachance Date: Tue, 21 Apr 2009 13:51:46 -0400 Subject: Bug 19567 - Make marshaling code usable without DBusConnection Some projects want to reuse the DBus message format, without actually going through a DBusConnection. This set of changes makes a few functions from DBusMessage public, and adds a new function to determine the number of bytes needed to demarshal a message. Signed-off-by: Colin Walters --- dbus/dbus-connection.c | 6 ++-- dbus/dbus-message-factory.c | 4 +-- dbus/dbus-message-internal.h | 2 -- dbus/dbus-message-util.c | 16 ++++++++-- dbus/dbus-message.c | 75 ++++++++++++++++++++++++++++++++++++++------ dbus/dbus-message.h | 6 ++++ dbus/dbus-transport-socket.c | 2 +- 7 files changed, 90 insertions(+), 21 deletions(-) diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c index d44d728b..947c0afe 100644 --- a/dbus/dbus-connection.c +++ b/dbus/dbus-connection.c @@ -1950,7 +1950,7 @@ _dbus_connection_send_preallocated_unlocked_no_update (DBusConnection *con if (dbus_message_get_serial (message) == 0) { serial = _dbus_connection_get_next_client_serial (connection); - _dbus_message_set_serial (message, serial); + dbus_message_set_serial (message, serial); if (client_serial) *client_serial = serial; } @@ -1963,7 +1963,7 @@ _dbus_connection_send_preallocated_unlocked_no_update (DBusConnection *con _dbus_verbose ("Message %p serial is %u\n", message, dbus_message_get_serial (message)); - _dbus_message_lock (message); + dbus_message_lock (message); /* Now we need to run an iteration to hopefully just write the messages * out immediately, and otherwise get them queued up @@ -3208,7 +3208,7 @@ dbus_connection_send_with_reply (DBusConnection *connection, if (serial == 0) { serial = _dbus_connection_get_next_client_serial (connection); - _dbus_message_set_serial (message, serial); + dbus_message_set_serial (message, serial); } if (!_dbus_pending_call_set_timeout_error_unlocked (pending, message, serial)) diff --git a/dbus/dbus-message-factory.c b/dbus/dbus-message-factory.c index ceffc764..8550ee86 100644 --- a/dbus/dbus-message-factory.c +++ b/dbus/dbus-message-factory.c @@ -222,8 +222,8 @@ generate_from_message (DBusString *data, DBusValidity *expected_validity, DBusMessage *message) { - _dbus_message_set_serial (message, 1); - _dbus_message_lock (message); + dbus_message_set_serial (message, 1); + dbus_message_lock (message); *expected_validity = DBUS_VALID; diff --git a/dbus/dbus-message-internal.h b/dbus/dbus-message-internal.h index 76615859..2e995b47 100644 --- a/dbus/dbus-message-internal.h +++ b/dbus/dbus-message-internal.h @@ -37,8 +37,6 @@ void _dbus_message_get_network_data (DBusMessage *message, void _dbus_message_lock (DBusMessage *message); void _dbus_message_unlock (DBusMessage *message); -void _dbus_message_set_serial (DBusMessage *message, - dbus_uint32_t serial); dbus_bool_t _dbus_message_add_size_counter (DBusMessage *message, DBusCounter *counter); void _dbus_message_add_size_counter_link (DBusMessage *message, diff --git a/dbus/dbus-message-util.c b/dbus/dbus-message-util.c index 0fa010cc..46cbe4e3 100644 --- a/dbus/dbus-message-util.c +++ b/dbus/dbus-message-util.c @@ -949,7 +949,7 @@ _dbus_message_test (const char *test_data_dir) "TestMethod")); _dbus_assert (strcmp (dbus_message_get_path (message), "/org/freedesktop/TestPath") == 0); - _dbus_message_set_serial (message, 1234); + dbus_message_set_serial (message, 1234); /* string length including nul byte not a multiple of 4 */ if (!dbus_message_set_sender (message, "org.foo.bar1")) @@ -1041,7 +1041,7 @@ _dbus_message_test (const char *test_data_dir) "/org/freedesktop/TestPath", "Foo.TestInterface", "TestMethod"); - _dbus_message_set_serial (message, 1); + dbus_message_set_serial (message, 1); dbus_message_set_reply_serial (message, 5678); v_INT16 = -0x123; @@ -1172,7 +1172,7 @@ _dbus_message_test (const char *test_data_dir) dbus_message_unref (copy); /* Message loader test */ - _dbus_message_lock (message); + dbus_message_lock (message); loader = _dbus_message_loader_new (); /* check ref/unref */ @@ -1226,6 +1226,7 @@ _dbus_message_test (const char *test_data_dir) DBusError error = DBUS_ERROR_INIT; char *marshalled = NULL; int len = 0; + char garbage_header[DBUS_MINIMUM_HEADER_SIZE] = "xxx"; if (!dbus_message_marshal (message, &marshalled, &len)) _dbus_assert_not_reached ("failed to marshal message"); @@ -1233,6 +1234,7 @@ _dbus_message_test (const char *test_data_dir) _dbus_assert (len != 0); _dbus_assert (marshalled != NULL); + _dbus_assert (dbus_message_demarshal_bytes_needed (marshalled, len) == len); message2 = dbus_message_demarshal (marshalled, len, &error); _dbus_assert (message2 != NULL); @@ -1255,6 +1257,14 @@ _dbus_message_test (const char *test_data_dir) _dbus_assert (message2 == NULL); _dbus_assert (dbus_error_is_set (&error)); dbus_error_free (&error); + + /* Bytes needed to demarshal empty message: 0 (more) */ + + _dbus_assert (dbus_message_demarshal_bytes_needed ("", 0) == 0); + + /* Bytes needed to demarshal invalid message: -1 (error). */ + + _dbus_assert (dbus_message_demarshal_bytes_needed (garbage_header, DBUS_MINIMUM_HEADER_SIZE) == -1); } dbus_message_unref (message); diff --git a/dbus/dbus-message.c b/dbus/dbus-message.c index e2c4771e..edae4258 100644 --- a/dbus/dbus-message.c +++ b/dbus/dbus-message.c @@ -142,7 +142,7 @@ _dbus_message_byteswap (DBusMessage *message) * Gets the data to be sent over the network for this message. * The header and then the body should be written out. * This function is guaranteed to always return the same - * data once a message is locked (with _dbus_message_lock()). + * data once a message is locked (with dbus_message_lock()). * * @param message the message. * @param header return location for message header data. @@ -163,16 +163,19 @@ _dbus_message_get_network_data (DBusMessage *message, * Sets the serial number of a message. * This can only be done once on a message. * + * DBusConnection will automatically set the serial to an appropriate value + * when the message is sent; this function is only needed when encapsulating + * messages in another protocol, or otherwise bypassing DBusConnection. + * * @param message the message * @param serial the serial */ -void -_dbus_message_set_serial (DBusMessage *message, - dbus_uint32_t serial) +void +dbus_message_set_serial (DBusMessage *message, + dbus_uint32_t serial) { - _dbus_assert (message != NULL); - _dbus_assert (!message->locked); - _dbus_assert (dbus_message_get_serial (message) == 0); + _dbus_return_if_fail (message != NULL); + _dbus_return_if_fail (!message->locked); _dbus_header_set_serial (&message->header, serial); } @@ -277,12 +280,13 @@ _dbus_message_remove_size_counter (DBusMessage *message, * reference to a message in the outgoing queue and change it * underneath us. Messages are locked when they enter the outgoing * queue (dbus_connection_send_message()), and the library complains - * if the message is modified while locked. + * if the message is modified while locked. This function may also + * called externally, for applications wrapping D-Bus in another protocol. * * @param message the message to lock. */ void -_dbus_message_lock (DBusMessage *message) +dbus_message_lock (DBusMessage *message) { if (!message->locked) { @@ -3941,7 +3945,7 @@ dbus_message_marshal (DBusMessage *msg, _dbus_return_val_if_fail (msg != NULL, FALSE); _dbus_return_val_if_fail (marshalled_data_p != NULL, FALSE); _dbus_return_val_if_fail (len_p != NULL, FALSE); - + if (!_dbus_string_init (&tmp)) return FALSE; @@ -4023,6 +4027,57 @@ dbus_message_demarshal (const char *str, return NULL; } +/** + * Returns the number of bytes required to be in the buffer to demarshal a + * D-Bus message. + * + * Generally, this function is only useful for encapsulating D-Bus messages in + * a different protocol. + * + * @param str data to be marshalled + * @param len the length of str + * @param error the location to save errors to + * @returns -1 if there was no valid data to be demarshalled, 0 if there wasn't enough data to determine how much should be demarshalled. Otherwise returns the number of bytes to be demarshalled + * + */ +int +dbus_message_demarshal_bytes_needed(const char *buf, + int len) +{ + DBusString str; + int byte_order, fields_array_len, header_len, body_len; + DBusValidity validity = DBUS_VALID; + int have_message; + + if (!buf || len < DBUS_MINIMUM_HEADER_SIZE) + return 0; + + if (len > DBUS_MAXIMUM_MESSAGE_LENGTH) + len = DBUS_MAXIMUM_MESSAGE_LENGTH; + _dbus_string_init_const_len (&str, buf, len); + + validity = DBUS_VALID; + have_message + = _dbus_header_have_message_untrusted(DBUS_MAXIMUM_MESSAGE_LENGTH, + &validity, &byte_order, + &fields_array_len, + &header_len, + &body_len, + &str, 0, + len); + _dbus_string_free (&str); + + if (validity == DBUS_VALID) + { + _dbus_assert(have_message); + return header_len + body_len; + } + else + { + return -1; /* broken! */ + } +} + /** @} */ /* tests in dbus-message-util.c */ diff --git a/dbus/dbus-message.h b/dbus/dbus-message.h index 1bf7d0c2..2e29fef0 100644 --- a/dbus/dbus-message.h +++ b/dbus/dbus-message.h @@ -131,6 +131,8 @@ dbus_bool_t dbus_message_has_sender (DBusMessage *message, dbus_bool_t dbus_message_has_signature (DBusMessage *message, const char *signature); dbus_uint32_t dbus_message_get_serial (DBusMessage *message); +void dbus_message_set_serial (DBusMessage *message, + dbus_uint32_t serial); dbus_bool_t dbus_message_set_reply_serial (DBusMessage *message, dbus_uint32_t reply_serial); dbus_uint32_t dbus_message_get_reply_serial (DBusMessage *message); @@ -196,6 +198,7 @@ dbus_bool_t dbus_message_iter_open_container (DBusMessageIter *iter, dbus_bool_t dbus_message_iter_close_container (DBusMessageIter *iter, DBusMessageIter *sub); +void dbus_message_lock (DBusMessage *message); dbus_bool_t dbus_set_error_from_message (DBusError *error, DBusMessage *message); @@ -220,6 +223,9 @@ DBusMessage* dbus_message_demarshal (const char *str, int len, DBusError *error); +int dbus_message_demarshal_bytes_needed (const char *str, + int len); + /** @} */ DBUS_END_DECLS diff --git a/dbus/dbus-transport-socket.c b/dbus/dbus-transport-socket.c index 10b671c2..6d7c89cd 100644 --- a/dbus/dbus-transport-socket.c +++ b/dbus/dbus-transport-socket.c @@ -537,7 +537,7 @@ do_writing (DBusTransport *transport) message = _dbus_connection_get_message_to_send (transport->connection); _dbus_assert (message != NULL); - _dbus_message_lock (message); + dbus_message_lock (message); #if 0 _dbus_verbose ("writing message %p\n", message); -- cgit From 0f19140b527ee946dd368dde9314c5c5e9d24177 Mon Sep 17 00:00:00 2001 From: Federico Mena Quintero Date: Wed, 18 Mar 2009 16:15:23 -0600 Subject: bfo20738 - Translate DBusValidity into error message Signed-off-by: Federico Mena Quintero --- dbus/dbus-marshal-validate.c | 71 ++++++++++++++++++++++++++++++++++++++++++++ dbus/dbus-marshal-validate.h | 2 ++ 2 files changed, 73 insertions(+) diff --git a/dbus/dbus-marshal-validate.c b/dbus/dbus-marshal-validate.c index ee955485..78d55941 100644 --- a/dbus/dbus-marshal-validate.c +++ b/dbus/dbus-marshal-validate.c @@ -810,6 +810,77 @@ _dbus_validate_path (const DBusString *str, return TRUE; } +const char * +_dbus_validity_to_error_message (DBusValidity validity) +{ + switch (validity) + { + case DBUS_VALIDITY_UNKNOWN_OOM_ERROR: return "Out of memory"; + case DBUS_INVALID_FOR_UNKNOWN_REASON: return "Unknown reason"; + case DBUS_VALID_BUT_INCOMPLETE: return "Valid but incomplete"; + case DBUS_VALIDITY_UNKNOWN: return "Validity unknown"; + case DBUS_VALID: return "Valid"; + case DBUS_INVALID_UNKNOWN_TYPECODE: return "Unknown typecode"; + case DBUS_INVALID_MISSING_ARRAY_ELEMENT_TYPE: return "Missing array element type"; + case DBUS_INVALID_SIGNATURE_TOO_LONG: return "Signature is too long"; + case DBUS_INVALID_EXCEEDED_MAXIMUM_ARRAY_RECURSION: return "Exceeded maximum array recursion"; + case DBUS_INVALID_EXCEEDED_MAXIMUM_STRUCT_RECURSION: return "Exceeded maximum struct recursion"; + case DBUS_INVALID_STRUCT_ENDED_BUT_NOT_STARTED: return "Struct ended but not started"; + case DBUS_INVALID_STRUCT_STARTED_BUT_NOT_ENDED: return "Struct started but not ended"; + case DBUS_INVALID_STRUCT_HAS_NO_FIELDS: return "Struct has no fields"; + case DBUS_INVALID_ALIGNMENT_PADDING_NOT_NUL: return "Alignment padding not null"; + case DBUS_INVALID_BOOLEAN_NOT_ZERO_OR_ONE: return "Boolean is not zero or one"; + case DBUS_INVALID_NOT_ENOUGH_DATA: return "Not enough data"; + case DBUS_INVALID_TOO_MUCH_DATA: return "Too much data"; + case DBUS_INVALID_BAD_BYTE_ORDER: return "Bad byte order"; + case DBUS_INVALID_BAD_PROTOCOL_VERSION: return "Bad protocol version"; + case DBUS_INVALID_BAD_MESSAGE_TYPE: return "Bad message type"; + case DBUS_INVALID_BAD_SERIAL: return "Bad serial"; + case DBUS_INVALID_INSANE_FIELDS_ARRAY_LENGTH: return "Insane fields array length"; + case DBUS_INVALID_INSANE_BODY_LENGTH: return "Insane body length"; + case DBUS_INVALID_MESSAGE_TOO_LONG: return "Message too long"; + case DBUS_INVALID_HEADER_FIELD_CODE: return "Header field code"; + case DBUS_INVALID_HEADER_FIELD_HAS_WRONG_TYPE: return "Header field has wrong type"; + case DBUS_INVALID_USES_LOCAL_INTERFACE: return "Uses local interface"; + case DBUS_INVALID_USES_LOCAL_PATH: return "Uses local path"; + case DBUS_INVALID_HEADER_FIELD_APPEARS_TWICE: return "Header field appears twice"; + case DBUS_INVALID_BAD_DESTINATION: return "Bad destination"; + case DBUS_INVALID_BAD_INTERFACE: return "Bad interface"; + case DBUS_INVALID_BAD_MEMBER: return "Bad member"; + case DBUS_INVALID_BAD_ERROR_NAME: return "Bad error name"; + case DBUS_INVALID_BAD_SENDER: return "Bad sender"; + case DBUS_INVALID_MISSING_PATH: return "Missing path"; + case DBUS_INVALID_MISSING_INTERFACE: return "Missing interface"; + case DBUS_INVALID_MISSING_MEMBER: return "Missing member"; + case DBUS_INVALID_MISSING_ERROR_NAME: return "Missing error name"; + case DBUS_INVALID_MISSING_REPLY_SERIAL: return "Missing reply serial"; + case DBUS_INVALID_LENGTH_OUT_OF_BOUNDS: return "Length out of bounds"; + case DBUS_INVALID_ARRAY_LENGTH_EXCEEDS_MAXIMUM: return "Array length exceeds maximum"; + case DBUS_INVALID_BAD_PATH: return "Bad path"; + case DBUS_INVALID_SIGNATURE_LENGTH_OUT_OF_BOUNDS: return "Signature length out of bounds"; + case DBUS_INVALID_BAD_UTF8_IN_STRING: return "Bad utf8 in string"; + case DBUS_INVALID_ARRAY_LENGTH_INCORRECT: return "Array length incorrect"; + case DBUS_INVALID_VARIANT_SIGNATURE_LENGTH_OUT_OF_BOUNDS: return "Variant signature length out of bounds"; + case DBUS_INVALID_VARIANT_SIGNATURE_BAD: return "Variant signature bad"; + case DBUS_INVALID_VARIANT_SIGNATURE_EMPTY: return "Variant signature empty"; + case DBUS_INVALID_VARIANT_SIGNATURE_SPECIFIES_MULTIPLE_VALUES: return "Variant signature specifies multiple values"; + case DBUS_INVALID_VARIANT_SIGNATURE_MISSING_NUL: return "Variant signature missing nul"; + case DBUS_INVALID_STRING_MISSING_NUL: return "String missing nul"; + case DBUS_INVALID_SIGNATURE_MISSING_NUL: return "Signature missing nul"; + case DBUS_INVALID_EXCEEDED_MAXIMUM_DICT_ENTRY_RECURSION: return "Exceeded maximum dict entry recursion"; + case DBUS_INVALID_DICT_ENTRY_ENDED_BUT_NOT_STARTED: return "Dict entry ended but not started"; + case DBUS_INVALID_DICT_ENTRY_STARTED_BUT_NOT_ENDED: return "Dict entry started but not ended"; + case DBUS_INVALID_DICT_ENTRY_HAS_NO_FIELDS: return "Dict entry has no fields"; + case DBUS_INVALID_DICT_ENTRY_HAS_ONLY_ONE_FIELD: return "Dict entry has only one field"; + case DBUS_INVALID_DICT_ENTRY_HAS_TOO_MANY_FIELDS: return "Dict entry has too many fields"; + case DBUS_INVALID_DICT_ENTRY_NOT_INSIDE_ARRAY: return "Dict entry not inside array"; + case DBUS_INVALID_DICT_KEY_MUST_BE_BASIC_TYPE: return "Dict key must be basic type"; + + default: + return "Invalid"; + } +} + /** * Checks that the given range of the string is a valid interface name * in the D-Bus protocol. This includes a length restriction and an diff --git a/dbus/dbus-marshal-validate.h b/dbus/dbus-marshal-validate.h index f5b168f5..d09acc60 100644 --- a/dbus/dbus-marshal-validate.h +++ b/dbus/dbus-marshal-validate.h @@ -131,6 +131,8 @@ DBusValidity _dbus_validate_body_with_reason (const DBusString *expected_si int value_pos, int len); +const char *_dbus_validity_to_error_message (DBusValidity validity); + dbus_bool_t _dbus_validate_path (const DBusString *str, int start, int len); -- cgit From 73ec6964d7a14eba3ec7118041e48e0a21438e52 Mon Sep 17 00:00:00 2001 From: Federico Mena Quintero Date: Wed, 18 Mar 2009 16:17:00 -0600 Subject: bfo20738 - Return a useful error message from dbus_signature_validate() Signed-off-by: Federico Mena Quintero --- dbus/dbus-signature.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/dbus/dbus-signature.c b/dbus/dbus-signature.c index a8864f8b..c7f8d0e3 100644 --- a/dbus/dbus-signature.c +++ b/dbus/dbus-signature.c @@ -233,12 +233,18 @@ dbus_signature_validate (const char *signature, { DBusString str; + DBusValidity reason; _dbus_string_init_const (&str, signature); - if (_dbus_validate_signature (&str, 0, _dbus_string_get_length (&str))) + reason = _dbus_validate_signature_with_reason (&str, 0, _dbus_string_get_length (&str)); + + if (reason == DBUS_VALID) return TRUE; - dbus_set_error (error, DBUS_ERROR_INVALID_SIGNATURE, "Corrupt type signature"); - return FALSE; + else + { + dbus_set_error (error, DBUS_ERROR_INVALID_SIGNATURE, _dbus_validity_to_error_message (reason)); + return FALSE; + } } /** -- cgit From b38c433bf713324b5d17eae626e8c7404bcb6554 Mon Sep 17 00:00:00 2001 From: Eamon Walsh Date: Tue, 21 Apr 2009 19:11:22 -0400 Subject: libselinux behavior in permissive mode wrt invalid domains Stephen Smalley wrote: > On Tue, 2009-04-21 at 16:32 -0400, Joshua Brindle wrote: > >> Stephen Smalley wrote: >> >>> On Thu, 2009-04-16 at 20:47 -0400, Eamon Walsh wrote: >>> >>>> Stephen Smalley wrote: >>>> >> >> >> >>> No, I don't want to change the behavior upon context_to_sid calls in >>> general, as we otherwise lose all context validity checking in >>> permissive mode. >>> >>> I think I'd rather change compute_sid behavior to preclude the situation >>> from arising in the first place, possibly altering the behavior in >>> permissive mode upon an invalid context to fall back on the ssid >>> (process) or the tsid (object). But I'm not entirely convinced any >>> change is required here. >>> >>> >> I just want to follow up to make sure we are all on the same page here. Was the >> suggestion to change avc_has_perm in libselinux or context_to_sid in the kernel >> or leave the code as is and fix the callers of avc_has_perm to correctly handle >> error codes? >> >> I prefer the last approach because of Eamon's explanation, EINVAL is already >> passed in errno to specify the context was invalid (and if object managers >> aren't handling that correctly now there is a good chance they aren't handling >> the ENOMEM case either). >> > > I'd be inclined to change compute_sid (not context_to_sid) in the kernel > to prevent invalid contexts from being formed even in permissive mode > (scenario is a type transition where role is not authorized for the new > type). That was originally to allow the system to boot in permissive > mode. But an alternative would be to just stay in the caller's context > (ssid) in that situation. > > Changing the callers of avc_has_perm() to handle EINVAL and/or ENOMEM > may make sense, but that logic should not depend on enforcing vs. > permissive mode. > > FWIW, the following patch to D-Bus should help: bfo21072 - Log SELinux denials better by checking errno for the cause Note that this does not fully address the bug report since EINVAL can still be returned in permissive mode. However the log messages will now reflect the proper cause of the denial. Signed-off-by: Eamon Walsh Signed-off-by: Colin Walters --- bus/selinux.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/bus/selinux.c b/bus/selinux.c index c0f6f4db..46a18a93 100644 --- a/bus/selinux.c +++ b/bus/selinux.c @@ -433,8 +433,18 @@ bus_selinux_check (BusSELinuxID *sender_sid, SELINUX_SID_FROM_BUS (bus_sid), target_class, requested, &aeref, auxdata) < 0) { - _dbus_verbose ("SELinux denying due to security policy.\n"); - return FALSE; + switch (errno) + { + case EACCES: + _dbus_verbose ("SELinux denying due to security policy.\n"); + return FALSE; + case EINVAL: + _dbus_verbose ("SELinux denying due to invalid security context.\n"); + return FALSE; + default: + _dbus_verbose ("SELinux denying due to: %s\n", _dbus_strerror (errno)); + return FALSE; + } } else return TRUE; -- cgit From f76d17437ee95bb2621cb55c79707b9f01dcf89a Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 27 Apr 2009 12:13:25 -0400 Subject: Release 1.2.14 --- configure.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.in b/configure.in index c1eabf0c..6992487b 100644 --- a/configure.in +++ b/configure.in @@ -3,7 +3,7 @@ AC_PREREQ(2.52) m4_define([dbus_major_version], [1]) m4_define([dbus_minor_version], [2]) -m4_define([dbus_micro_version], [13]) +m4_define([dbus_micro_version], [14]) m4_define([dbus_version], [dbus_major_version.dbus_minor_version.dbus_micro_version]) AC_INIT(dbus, [dbus_version]) -- cgit