diff options
| -rw-r--r-- | ChangeLog | 19 | ||||
| -rw-r--r-- | bus/config-loader-expat.c | 24 | ||||
| -rw-r--r-- | bus/config-parser.c | 52 | ||||
| -rw-r--r-- | bus/dispatch.c | 61 | ||||
| -rw-r--r-- | bus/test.c | 6 | ||||
| -rw-r--r-- | configure.in | 4 | ||||
| -rw-r--r-- | dbus/dbus-auth.c | 8 | ||||
| -rw-r--r-- | dbus/dbus-bus.h | 4 | ||||
| -rw-r--r-- | dbus/dbus-internals.c | 81 | ||||
| -rw-r--r-- | dbus/dbus-internals.h | 7 | ||||
| -rw-r--r-- | dbus/dbus-memory.c | 55 | ||||
| -rw-r--r-- | dbus/dbus-string.c | 24 | ||||
| -rw-r--r-- | doc/TODO | 7 |
13 files changed, 233 insertions, 119 deletions
@@ -1,3 +1,22 @@ +2003-03-28 Havoc Pennington <hp@pobox.com> + + * bus/test.c (bus_test_flush_bus): remove the sleep from here, + I think it may have just been superstition. Not sure. + + * dbus/dbus-string.c (_dbus_string_base64_decode): catch some OOM + failures that were not being handled. + + * dbus/dbus-auth.c (process_auth): fix a memleak in OOM handling + + * dbus/dbus-memory.c: add ability to set number of mallocs in a + row that will fail on out-of-memory. + + * dbus/dbus-internals.c (_dbus_test_oom_handling): convenience + function for testing out-of-memory handling. + + * bus/config-loader-expat.c (memsuite): don't wrap the dbus + allocation functions, they do map exactly to the expat ones. + 2003-03-27 Havoc Pennington <hp@redhat.com> * bus/config-loader-libxml.c (bus_config_load): add another error diff --git a/bus/config-loader-expat.c b/bus/config-loader-expat.c index 5e8d28c1..c7981bf4 100644 --- a/bus/config-loader-expat.c +++ b/bus/config-loader-expat.c @@ -25,29 +25,11 @@ #include <dbus/dbus-internals.h> #include <expat.h> -static void* -expat_malloc (size_t size) -{ - return dbus_malloc (size); -} - -static void* -expat_realloc (void *ptr, size_t size) -{ - return dbus_realloc (ptr, size); -} - -static void -expat_free (void *ptr) -{ - dbus_free (ptr); -} - static XML_Memory_Handling_Suite memsuite = { - expat_malloc, - expat_realloc, - expat_free + dbus_malloc, + dbus_realloc, + dbus_free }; typedef struct diff --git a/bus/config-parser.c b/bus/config-parser.c index 8fb3b29e..2429cce5 100644 --- a/bus/config-parser.c +++ b/bus/config-parser.c @@ -285,47 +285,18 @@ do_load (const DBusString *full_path, } } -static dbus_bool_t -check_oom_loading (const DBusString *full_path, - Validity validity) +typedef struct { - int approx_mallocs; - - /* Run once to see about how many mallocs are involved */ - - _dbus_set_fail_alloc_counter (_DBUS_INT_MAX); - - if (!do_load (full_path, validity, FALSE)) - return FALSE; + const DBusString *full_path; + Validity validity; +} LoaderOomData; - approx_mallocs = _DBUS_INT_MAX - _dbus_get_fail_alloc_counter (); - - _dbus_verbose ("=================\nabout %d mallocs total\n=================\n", - approx_mallocs); - - approx_mallocs += 10; /* fudge factor */ - - /* Now run failing each malloc */ - - while (approx_mallocs >= 0) - { - - _dbus_set_fail_alloc_counter (approx_mallocs); - - _dbus_verbose ("\n===\n(will fail malloc %d)\n===\n", - approx_mallocs); - - if (!do_load (full_path, validity, TRUE)) - return FALSE; - - approx_mallocs -= 1; - } - - _dbus_set_fail_alloc_counter (_DBUS_INT_MAX); - - _dbus_verbose ("=================\n all iterations passed\n=================\n"); +static dbus_bool_t +check_loader_oom_func (void *data) +{ + LoaderOomData *d = data; - return TRUE; + return do_load (d->full_path, d->validity, TRUE); } static dbus_bool_t @@ -376,6 +347,7 @@ process_test_subdir (const DBusString *test_base_dir, while (_dbus_directory_get_next_file (dir, &filename, &error)) { DBusString full_path; + LoaderOomData d; if (!_dbus_string_init (&full_path, _DBUS_INT_MAX)) _dbus_assert_not_reached ("couldn't init string"); @@ -407,7 +379,9 @@ process_test_subdir (const DBusString *test_base_dir, (validity == INVALID ? "invalid" : (validity == UNKNOWN ? "unknown" : "???"))); - if (!check_oom_loading (&full_path, validity)) + d.full_path = &full_path; + d.validity = validity; + if (!_dbus_test_oom_handling ("config-loader", check_loader_oom_func, &d)) _dbus_assert_not_reached ("test failed"); _dbus_string_free (&full_path); diff --git a/bus/dispatch.c b/bus/dispatch.c index 7db4ac29..2b1dc782 100644 --- a/bus/dispatch.c +++ b/bus/dispatch.c @@ -916,49 +916,42 @@ check_hello_connection (BusContext *context) return TRUE; } -static void -check1_try_iterations (BusContext *context, - const char *description, - Check1Func func) +typedef struct { - int approx_mallocs; - - /* Run once to see about how many mallocs are involved */ - - _dbus_set_fail_alloc_counter (_DBUS_INT_MAX); - - if (! (*func) (context)) - _dbus_assert_not_reached ("test failed"); + Check1Func func; + BusContext *context; +} Check1Data; - approx_mallocs = _DBUS_INT_MAX - _dbus_get_fail_alloc_counter (); +static dbus_bool_t +check_oom_check1_func (void *data) +{ + Check1Data *d = data; - _dbus_verbose ("=================\n%s: about %d mallocs total\n=================\n", - description, approx_mallocs); - - approx_mallocs += 10; /* fudge factor */ - - /* Now run failing each malloc */ + if (! (* d->func) (d->context)) + return FALSE; - while (approx_mallocs >= 0) + if (!check_no_leftovers (d->context)) { - _dbus_set_fail_alloc_counter (approx_mallocs); - - _dbus_verbose ("\n===\n %s: (will fail malloc %d)\n===\n", - description, approx_mallocs); + _dbus_warn ("Messages were left over, should be covered by test suite"); + return FALSE; + } - if (! (*func) (context)) - _dbus_assert_not_reached ("test failed"); + return TRUE; +} - if (!check_no_leftovers (context)) - _dbus_assert_not_reached ("Messages were left over, should be covered by test suite"); - - approx_mallocs -= 1; - } +static void +check1_try_iterations (BusContext *context, + const char *description, + Check1Func func) +{ + Check1Data d; - _dbus_set_fail_alloc_counter (_DBUS_INT_MAX); + d.func = func; + d.context = context; - _dbus_verbose ("=================\n%s: all iterations passed\n=================\n", - description); + if (!_dbus_test_oom_handling (description, check_oom_check1_func, + &d)) + _dbus_assert_not_reached ("test failed"); } dbus_bool_t @@ -267,7 +267,6 @@ bus_test_client_listed (DBusConnection *connection) return FALSE; } - void bus_test_flush_bus (BusContext *context) { @@ -276,11 +275,14 @@ bus_test_flush_bus (BusContext *context) * one end of the debug pipe to come out the other end... * a more robust setup would be good. Blocking on the other * end of pipes we've pushed data into or something. + * A simple hack might be to just make the debug server always + * poll for read on the other end of the pipe after writing. */ - while (bus_loop_iterate (FALSE)) ; +#if 0 _dbus_sleep_milliseconds (15); +#endif while (bus_loop_iterate (FALSE)) ; } diff --git a/configure.in b/configure.in index 7212f3be..81b316df 100644 --- a/configure.in +++ b/configure.in @@ -244,8 +244,8 @@ elif test x$with_xml = xlibxml; then fi else ### expat is the default because libxml can't currently survive - ### our brutal OOM-handling unit test setup. Need to find the - ### bugs and submit to DV + ### our brutal OOM-handling unit test setup. + ### http://bugzilla.gnome.org/show_bug.cgi?id=109368 if $have_expat ; then with_xml=expat dbus_use_expat=true diff --git a/dbus/dbus-auth.c b/dbus/dbus-auth.c index 8f8aec44..1aa83e7c 100644 --- a/dbus/dbus-auth.c +++ b/dbus/dbus-auth.c @@ -1241,14 +1241,14 @@ process_auth (DBusAuth *auth, _dbus_string_free (&mech); return FALSE; } - + if (!_dbus_string_init (&decoded_response, _DBUS_INT_MAX)) { _dbus_string_free (&mech); _dbus_string_free (&base64_response); return FALSE; } - + if (!_dbus_string_copy_len (args, 0, i, &mech, 0)) goto failed; @@ -1258,7 +1258,7 @@ process_auth (DBusAuth *auth, if (!_dbus_string_base64_decode (&base64_response, 0, &decoded_response, 0)) goto failed; - + auth->mech = find_mech (&mech); if (auth->mech != NULL) { @@ -1274,7 +1274,7 @@ process_auth (DBusAuth *auth, { /* Unsupported mechanism */ if (!send_rejected (auth)) - return FALSE; + goto failed; } _dbus_string_free (&mech); diff --git a/dbus/dbus-bus.h b/dbus/dbus-bus.h index c6800ede..9f25eeb6 100644 --- a/dbus/dbus-bus.h +++ b/dbus/dbus-bus.h @@ -29,6 +29,8 @@ #include <dbus/dbus-connection.h> +DBUS_BEGIN_DECLS; + dbus_bool_t dbus_bus_register (DBusConnection *connection, DBusError *error); dbus_bool_t dbus_bus_set_base_service (DBusConnection *connection, @@ -42,6 +44,6 @@ dbus_bool_t dbus_bus_service_exists (DBusConnection *connection, const char *service_name, DBusError *error); - +DBUS_END_DECLS; #endif /* DBUS_BUS_H */ diff --git a/dbus/dbus-internals.c b/dbus/dbus-internals.c index 9588e72b..1c018b7f 100644 --- a/dbus/dbus-internals.c +++ b/dbus/dbus-internals.c @@ -292,4 +292,85 @@ _dbus_type_to_string (int type) } } +static dbus_bool_t +run_failing_each_malloc (int n_mallocs, + const char *description, + DBusTestMemoryFunction func, + void *data) +{ + n_mallocs += 10; /* fudge factor to ensure reallocs etc. are covered */ + + while (n_mallocs >= 0) + { + _dbus_set_fail_alloc_counter (n_mallocs); + + _dbus_verbose ("\n===\n%s: (will fail malloc %d with %d failures)\n===\n", + description, n_mallocs, + _dbus_get_fail_alloc_failures ()); + + if (!(* func) (data)) + return FALSE; + + n_mallocs -= 1; + } + + _dbus_set_fail_alloc_counter (_DBUS_INT_MAX); + + return TRUE; +} + +/** + * Tests how well the given function responds to out-of-memory + * situations. Calls the function repeatedly, failing a different + * call to malloc() each time. If the function ever returns #FALSE, + * the test fails. The function should return #TRUE whenever something + * valid (such as returning an error, or succeeding) occurs, and #FALSE + * if it gets confused in some way. + * + * @param description description of the test used in verbose output + * @param func function to call + * @param data data to pass to function + * @returns #TRUE if the function never returns FALSE + */ +dbus_bool_t +_dbus_test_oom_handling (const char *description, + DBusTestMemoryFunction func, + void *data) +{ + int approx_mallocs; + + /* Run once to see about how many mallocs are involved */ + + _dbus_set_fail_alloc_counter (_DBUS_INT_MAX); + + if (!(* func) (data)) + return FALSE; + + approx_mallocs = _DBUS_INT_MAX - _dbus_get_fail_alloc_counter (); + + _dbus_verbose ("=================\n%s: about %d mallocs total\n=================\n", + description, approx_mallocs); + + _dbus_set_fail_alloc_failures (1); + if (!run_failing_each_malloc (approx_mallocs, description, func, data)) + return FALSE; + + _dbus_set_fail_alloc_failures (2); + if (!run_failing_each_malloc (approx_mallocs, description, func, data)) + return FALSE; + + _dbus_set_fail_alloc_failures (3); + if (!run_failing_each_malloc (approx_mallocs, description, func, data)) + return FALSE; + + _dbus_set_fail_alloc_failures (4); + if (!run_failing_each_malloc (approx_mallocs, description, func, data)) + return FALSE; + + _dbus_verbose ("=================\n%s: all iterations passed\n=================\n", + description); + + return TRUE; +} + /** @} */ diff --git a/dbus/dbus-internals.h b/dbus/dbus-internals.h index d95e58d6..9bfd0b3b 100644 --- a/dbus/dbus-internals.h +++ b/dbus/dbus-internals.h @@ -170,9 +170,16 @@ extern const char _dbus_no_memory_message[]; /* Memory debugging */ void _dbus_set_fail_alloc_counter (int until_next_fail); int _dbus_get_fail_alloc_counter (void); +void _dbus_set_fail_alloc_failures (int failures_per_failure); +int _dbus_get_fail_alloc_failures (void); dbus_bool_t _dbus_decrement_fail_alloc_counter (void); dbus_bool_t _dbus_disable_mem_pools (void); int _dbus_get_malloc_blocks_outstanding (void); + +typedef dbus_bool_t (* DBusTestMemoryFunction) (void *data); +dbus_bool_t _dbus_test_oom_handling (const char *description, + DBusTestMemoryFunction func, + void *data); #else #define _dbus_set_fail_alloc_counter(n) #define _dbus_get_fail_alloc_counter _DBUS_INT_MAX diff --git a/dbus/dbus-memory.c b/dbus/dbus-memory.c index 983c6e31..11d52045 100644 --- a/dbus/dbus-memory.c +++ b/dbus/dbus-memory.c @@ -76,9 +76,11 @@ #ifdef DBUS_BUILD_TESTS static dbus_bool_t debug_initialized = FALSE; -static int fail_counts = -1; +static int fail_nth = -1; static size_t fail_size = 0; static int fail_alloc_counter = _DBUS_INT_MAX; +static int n_failures_per_failure = 1; +static int n_failures_this_failure = 0; static dbus_bool_t guards = FALSE; static dbus_bool_t disable_mem_pools = FALSE; static dbus_bool_t backtrace_on_fail_alloc = FALSE; @@ -106,9 +108,9 @@ _dbus_initialize_malloc_debug (void) if (_dbus_getenv ("DBUS_MALLOC_FAIL_NTH") != NULL) { - fail_counts = atoi (_dbus_getenv ("DBUS_MALLOC_FAIL_NTH")); - fail_alloc_counter = fail_counts; - _dbus_verbose ("Will fail malloc every %d times\n", fail_counts); + fail_nth = atoi (_dbus_getenv ("DBUS_MALLOC_FAIL_NTH")); + fail_alloc_counter = fail_nth; + _dbus_verbose ("Will fail malloc every %d times\n", fail_nth); } if (_dbus_getenv ("DBUS_MALLOC_FAIL_GREATER_THAN") != NULL) @@ -185,6 +187,30 @@ _dbus_get_fail_alloc_counter (void) } /** + * Sets how many mallocs to fail when the fail alloc counter reaches + * 0. + * + * @param number to fail + */ +void +_dbus_set_fail_alloc_failures (int failures_per_failure) +{ + n_failures_per_failure = failures_per_failure; +} + +/** + * Gets the number of failures we'll have when the fail malloc + * counter reaches 0. + * + * @returns number of failures planned + */ +int +_dbus_get_fail_alloc_failures (void) +{ + return n_failures_per_failure; +} + +/** * Called when about to alloc some memory; if * it returns #TRUE, then the allocation should * fail. If it returns #FALSE, then the allocation @@ -199,14 +225,23 @@ _dbus_decrement_fail_alloc_counter (void) if (fail_alloc_counter <= 0) { - if (fail_counts >= 0) - fail_alloc_counter = fail_counts; - else - fail_alloc_counter = _DBUS_INT_MAX; - - _dbus_verbose ("reset fail alloc counter to %d\n", fail_alloc_counter); if (backtrace_on_fail_alloc) _dbus_print_backtrace (); + + _dbus_verbose ("failure %d\n", n_failures_this_failure); + + n_failures_this_failure += 1; + if (n_failures_this_failure >= n_failures_per_failure) + { + if (fail_nth >= 0) + fail_alloc_counter = fail_nth; + else + fail_alloc_counter = _DBUS_INT_MAX; + + n_failures_this_failure = 0; + + _dbus_verbose ("reset fail alloc counter to %d\n", fail_alloc_counter); + } return TRUE; } diff --git a/dbus/dbus-string.c b/dbus/dbus-string.c index 1bc3e205..7549dcad 100644 --- a/dbus/dbus-string.c +++ b/dbus/dbus-string.c @@ -2108,15 +2108,22 @@ _dbus_string_base64_decode (const DBusString *source, */ if (pad_count < 1) - _dbus_string_append_byte (&result, - triplet >> 16); + { + if (!_dbus_string_append_byte (&result, + triplet >> 16)) + goto failed; + } if (pad_count < 2) - _dbus_string_append_byte (&result, - (triplet >> 8) & 0xff); + { + if (!_dbus_string_append_byte (&result, + (triplet >> 8) & 0xff)) + goto failed; + } - _dbus_string_append_byte (&result, - triplet & 0xff); + if (!_dbus_string_append_byte (&result, + triplet & 0xff)) + goto failed; sextet_count = 0; pad_count = 0; @@ -2136,6 +2143,11 @@ _dbus_string_base64_decode (const DBusString *source, _dbus_string_free (&result); return TRUE; + + failed: + _dbus_string_free (&result); + + return FALSE; } /** @@ -39,3 +39,10 @@ - Automatic service activation, should probably be done through a message flag. + - Disconnecting the remote end on invalid UTF-8 is probably not a good + idea. The definitiion of "valid" is slightly fuzzy. I think it might + be better to just silently "fix" the UTF-8, or perhaps return an error. + + - We might consider returning a "no such operation" error in dbus-connection.c + for unhandled messages. + |