From b1d29497d6076c40fed8e151c0b2226e4f86ef62 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Wed, 16 Feb 2011 17:44:48 +0000 Subject: dbus_message_iter_append_basic: check string-like arguments for validity Strings: UTF-8 with no embedded NULs, by adding a new internal function, _dbus_check_is_valid_utf8 Object paths, signatures: the obvious syntactic checks This moves some of the burden of validation to the sender. When sending 10240 times with up to 1024 parallel calls pending, on a single-core ARM Linux device, I found that user CPU time in dbus-spam increased by up to 80% as a result of the validation. However, when sending messages to dbus-daemon, overall throughput only reduced by 15%, and when sending messages to an echo service, overall throughput actually improved by around 14% (presumably because making the sender CPU-bound influenced kernel scheduling). Bug: https://bugs.freedesktop.org/show_bug.cgi?id=16338 Bug-NB: NB#223152 Reviewed-by: Cosimo Alfarano --- dbus/dbus-marshal-validate.c | 2 ++ dbus/dbus-marshal-validate.h | 4 ++++ dbus/dbus-message.c | 31 +++++++++++++++++++++++++++++++ 3 files changed, 37 insertions(+) diff --git a/dbus/dbus-marshal-validate.c b/dbus/dbus-marshal-validate.c index b4579978..4304467d 100644 --- a/dbus/dbus-marshal-validate.c +++ b/dbus/dbus-marshal-validate.c @@ -1221,6 +1221,8 @@ DEFINE_DBUS_NAME_CHECK(error_name) DEFINE_DBUS_NAME_CHECK(bus_name) /** define _dbus_check_is_valid_signature() */ DEFINE_DBUS_NAME_CHECK(signature) +/** define _dbus_check_is_valid_utf8() */ +DEFINE_DBUS_NAME_CHECK(utf8) /** @} */ diff --git a/dbus/dbus-marshal-validate.h b/dbus/dbus-marshal-validate.h index 8947a2af..1d2e26b8 100644 --- a/dbus/dbus-marshal-validate.h +++ b/dbus/dbus-marshal-validate.h @@ -147,6 +147,8 @@ dbus_bool_t _dbus_validate_bus_name (const DBusString *str, dbus_bool_t _dbus_validate_signature (const DBusString *str, int start, int len); +/* just to have a name consistent with the above: */ +#define _dbus_validate_utf8(s,b,e) _dbus_string_validate_utf8 (s, b, e) #ifdef DBUS_DISABLE_CHECKS @@ -193,6 +195,8 @@ DECLARE_DBUS_NAME_CHECK(error_name); DECLARE_DBUS_NAME_CHECK(bus_name); /** defines _dbus_check_is_valid_signature() */ DECLARE_DBUS_NAME_CHECK(signature); +/** defines _dbus_check_is_valid_utf8() */ +DECLARE_DBUS_NAME_CHECK(utf8); /** @} */ diff --git a/dbus/dbus-message.c b/dbus/dbus-message.c index 442ec2ae..c6b52f55 100644 --- a/dbus/dbus-message.c +++ b/dbus/dbus-message.c @@ -2515,6 +2515,37 @@ dbus_message_iter_append_basic (DBusMessageIter *iter, _dbus_return_val_if_fail (dbus_type_is_basic (type), FALSE); _dbus_return_val_if_fail (value != NULL, FALSE); +#ifndef DBUS_DISABLE_CHECKS + switch (type) + { + const char * const *string_p; + + case DBUS_TYPE_STRING: + string_p = value; + _dbus_return_val_if_fail (_dbus_check_is_valid_utf8 (*string_p), FALSE); + break; + + case DBUS_TYPE_OBJECT_PATH: + string_p = value; + _dbus_return_val_if_fail (_dbus_check_is_valid_path (*string_p), FALSE); + break; + + case DBUS_TYPE_SIGNATURE: + string_p = value; + _dbus_return_val_if_fail (_dbus_check_is_valid_signature (*string_p), FALSE); + break; + + case DBUS_TYPE_BOOLEAN: + /* FIXME: strictly speaking we should ensure that it's in {0,1}, + * but for now, fall through */ + + default: + { + /* nothing to check, all possible values are allowed */ + } + } +#endif + if (!_dbus_message_iter_open_signature (real)) return FALSE; -- cgit v1.2.3 From bb32afe831ca6e5913e7dd6035d1167c09c54e29 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Thu, 3 Mar 2011 16:29:17 +0000 Subject: dbus_message_iter_append_basic: validate booleans too Sending, for instance, ((dbus_bool_t) 666) is a programming error and should be diagnosed as such. Bug: https://bugs.freedesktop.org/show_bug.cgi?id=16338 Reviewed-by: Cosimo Alfarano --- dbus/dbus-message.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/dbus/dbus-message.c b/dbus/dbus-message.c index c6b52f55..a8378e30 100644 --- a/dbus/dbus-message.c +++ b/dbus/dbus-message.c @@ -2519,6 +2519,7 @@ dbus_message_iter_append_basic (DBusMessageIter *iter, switch (type) { const char * const *string_p; + const dbus_bool_t *bool_p; case DBUS_TYPE_STRING: string_p = value; @@ -2536,8 +2537,9 @@ dbus_message_iter_append_basic (DBusMessageIter *iter, break; case DBUS_TYPE_BOOLEAN: - /* FIXME: strictly speaking we should ensure that it's in {0,1}, - * but for now, fall through */ + bool_p = value; + _dbus_return_val_if_fail (*bool_p == 0 || *bool_p == 1, FALSE); + break; default: { -- cgit v1.2.3 From a9cbeecfcc2fee6e89bf1f5e1dd2a8f0fe860c55 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Thu, 3 Mar 2011 16:30:00 +0000 Subject: dbus_message_iter_append_fixed_array: add a check for valid booleans The reasoning is the same as for dbus_message_iter_append_basic. Bug: https://bugs.freedesktop.org/show_bug.cgi?id=16338 Reviewed-by: Cosimo Alfarano --- dbus/dbus-message.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/dbus/dbus-message.c b/dbus/dbus-message.c index a8378e30..075ff311 100644 --- a/dbus/dbus-message.c +++ b/dbus/dbus-message.c @@ -2660,6 +2660,19 @@ dbus_message_iter_append_fixed_array (DBusMessageIter *iter, DBUS_MAXIMUM_ARRAY_LENGTH / _dbus_type_get_alignment (element_type), FALSE); +#ifndef DBUS_DISABLE_CHECKS + if (element_type == DBUS_TYPE_BOOLEAN) + { + const dbus_bool_t * const *bools = value; + int i; + + for (i = 0; i < n_elements; i++) + { + _dbus_return_val_if_fail ((*bools)[i] == 0 || (*bools)[i] == 1, FALSE); + } + } +#endif + ret = _dbus_type_writer_write_fixed_multi (&real->u.writer, element_type, value, n_elements); return ret; -- cgit v1.2.3