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 <http://www.cl.cam.ac.uk/~mgk25/ucs/examples/UTF-8-demo.txt>
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 <cosimo.alfarano@collabora.co.uk>
This commit is contained in:
Simon McVittie 2011-02-16 17:44:48 +00:00
parent f1ba52066c
commit b1d29497d6
3 changed files with 37 additions and 0 deletions

View file

@ -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)
/** @} */

View file

@ -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);
/** @} */

View file

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