Attachment 43809 Details for Bug 16338 – dbus_message_iter_append_basic: check string-like arguments for validity

[patch] dbus_message_iter_append_basic: check string-like arguments for validity

0002-dbus_message_iter_append_basic-check-string-like-arg.patch (text/plain), 3.50 KB, created by Simon McVittie on 2011-02-25 08:49:36 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Simon McVittie

Created: 2011-02-25 08:49:36 UTC

Size: 3.50 KB

patch

obsolete

>From 616e46fba8ecdbf9e9e4524692aa514f2c60e145 Mon Sep 17 00:00:00 2001
>From: Simon McVittie <simon.mcvittie@collabora.co.uk>
>Date: Wed, 16 Feb 2011 17:44:48 +0000
>Subject: [PATCH 2/2] 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
>---
> dbus/dbus-marshal-validate.c |    5 +++++
> dbus/dbus-marshal-validate.h |    2 ++
> dbus/dbus-message.c          |   31 +++++++++++++++++++++++++++++++
> 3 files changed, 38 insertions(+), 0 deletions(-)
>
>diff --git a/dbus/dbus-marshal-validate.c b/dbus/dbus-marshal-validate.c
>index b457997..0c841c1 100644
>--- a/dbus/dbus-marshal-validate.c
>+++ b/dbus/dbus-marshal-validate.c
>@@ -1222,6 +1222,11 @@ DEFINE_DBUS_NAME_CHECK(bus_name)
> /** define _dbus_check_is_valid_signature() */
> DEFINE_DBUS_NAME_CHECK(signature)
> 
>+/* this validation function doesn't follow the same naming pattern :-( */
>+#define _dbus_validate_utf8(s,b,e) _dbus_string_validate_utf8 (s, b, e)
>+/** define _dbus_check_is_valid_utf8() */
>+DEFINE_DBUS_NAME_CHECK(utf8)
>+
> /** @} */
> 
> /* tests in dbus-marshal-validate-util.c */
>diff --git a/dbus/dbus-marshal-validate.h b/dbus/dbus-marshal-validate.h
>index 8947a2a..20ca73f 100644
>--- a/dbus/dbus-marshal-validate.h
>+++ b/dbus/dbus-marshal-validate.h
>@@ -193,6 +193,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 442ec2a..c6b52f5 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;
> 
>-- 
>1.7.4.1
>

Actions: View | Splinter Review

Attachments on bug 16338: 43809 | 44082 | 44083 | 44084

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.