March 2019 - tor-commits - lists.torproject.org

[tor/master] Document several issues found by Taylor
by asn＠torproject.org 27 Mar '19

27 Mar '19

commit 3d6bf7b36e419bbabd48b4bed82f12fca1a922b3 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Mar 12 18:53:24 2019 -0400 Document several issues found by Taylor --- src/lib/pubsub/pub_binding_st.h | 4 +++- src/lib/pubsub/pubsub.h | 3 +++ src/lib/pubsub/pubsub_connect.h | 3 ++- src/lib/pubsub/pubsub_macros.h | 6 ++++-- 4 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/lib/pubsub/pub_binding_st.h b/src/lib/pubsub/pub_binding_st.h index e8c0d47ef..4f5df8ff3 100644 --- a/src/lib/pubsub/pub_binding_st.h +++ b/src/lib/pubsub/pub_binding_st.h @@ -5,8 +5,10 @@ /* See LICENSE for licensing information */ /** - * @file pubsub_build.h + * @file pub_binding_st.h * @brief Declaration of pub_binding_t. + * + * This is an internal type for the pubsub implementation. */ #ifndef TOR_PUB_BINDING_ST_H diff --git a/src/lib/pubsub/pubsub.h b/src/lib/pubsub/pubsub.h index 1c51f7a78..08e3f42f7 100644 --- a/src/lib/pubsub/pubsub.h +++ b/src/lib/pubsub/pubsub.h @@ -30,6 +30,9 @@ * Rather than using the dispatch module directly, a publishing module * receives a "binding" object that it uses to send messages with the right * settings. + * + * Most users of this module will want to use this header, and the + * pubsub_macros.h header for convenience. */ /* diff --git a/src/lib/pubsub/pubsub_connect.h b/src/lib/pubsub/pubsub_connect.h index b63f9dc43..f3de74c87 100644 --- a/src/lib/pubsub/pubsub_connect.h +++ b/src/lib/pubsub/pubsub_connect.h @@ -9,7 +9,8 @@ * @brief Header for functions that add relationships to a pubsub builder. * * These functions are used by modules that need to add publication and - * subscription requests. + * subscription requests. Most users will want to call these functions + * indirectly, via the macros in pubsub_macros.h. **/ #ifndef TOR_PUBSUB_CONNECT_H diff --git a/src/lib/pubsub/pubsub_macros.h b/src/lib/pubsub/pubsub_macros.h index aed9c5128..4d0638d1d 100644 --- a/src/lib/pubsub/pubsub_macros.h +++ b/src/lib/pubsub/pubsub_macros.h @@ -175,7 +175,8 @@ * Use this macro in a header to declare the existence of a given message, * taking a pointer as auxiliary data. * - * "messagename" is a unique identifier for the message. + * "messagename" is a unique identifier for the message; it must be a valid + * C identifier. * * "typename" is a unique identifier for the type of the auxiliary data. * @@ -199,7 +200,8 @@ * Use this macro in a header to declare the existence of a given message, * taking an integer as auxiliary data. * - * "messagename" is a unique identifier for the message. + * "messagename" is a unique identifier for the message; it must be a valid + * C identifier. * * "typename" is a unique identifier for the type of the auxiliary data. *

1 0

[tor/master] dispatch: Test behavior of formatting type with no set fmt function.
by asn＠torproject.org 27 Mar '19

27 Mar '19

commit c40bcab85df19feb4ad6503f160c07b23501dc85 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Mar 13 15:52:48 2019 -0400 dispatch: Test behavior of formatting type with no set fmt function. --- src/test/test_dispatch.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/test/test_dispatch.c b/src/test/test_dispatch.c index 061e9df14..886f45868 100644 --- a/src/test/test_dispatch.c +++ b/src/test/test_dispatch.c @@ -17,6 +17,8 @@ #include <stdio.h> #include <string.h> +static dispatch_t *dispatcher_in_use=NULL; + /* Construct an empty dispatch_t. */ static void test_dispatch_empty(void *arg) @@ -44,9 +46,14 @@ simple_recv1(const msg_t *m) total_recv1_simple += m->aux_data__.u64; } +static char *recv2_received = NULL; + static void simple_recv2(const msg_t *m) { + tor_free(recv2_received); + recv2_received = dispatch_fmt_msg_data(dispatcher_in_use, m); + total_recv2_simple += m->aux_data__.u64*10; } @@ -73,6 +80,7 @@ test_dispatch_simple(void *arg) d = dispatch_new(cfg); tt_assert(d); + dispatcher_in_use = d; msg_aux_data_t data = {.u64 = 7}; r = dispatch_send(d, 99, 0, 0, 0, data); @@ -91,9 +99,12 @@ test_dispatch_simple(void *arg) tt_int_op(total_recv1_simple, OP_EQ, 0); tt_int_op(total_recv2_simple, OP_EQ, 140); + tt_str_op(recv2_received, OP_EQ, "<>"); // no format function was set. + done: dispatch_free(d); dcfg_free(cfg); + tor_free(recv2_received); } struct coord { int x; int y; }; @@ -121,7 +132,6 @@ alert_run_immediate(dispatch_t *d, channel_id_t ch, void *arg) dispatch_flush(d, ch, INT_MAX); } -static dispatch_t *dispatcher_in_use=NULL; static char *received_data=NULL; static void

1 0

[tor/master] Unit test for namemap_fmt_name()
by asn＠torproject.org 27 Mar '19

27 Mar '19

commit f74301f8fdf168dbd9dbfe77362658656c70fe71 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Mar 13 15:48:45 2019 -0400 Unit test for namemap_fmt_name() --- src/test/test_namemap.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/test/test_namemap.c b/src/test/test_namemap.c index 5134e1451..df77d4e2d 100644 --- a/src/test/test_namemap.c +++ b/src/test/test_namemap.c @@ -142,6 +142,25 @@ test_namemap_internals(void *arg) namemap_clear(&m); } +static void +test_namemap_fmt(void *arg) +{ + (void)arg; + namemap_t m = NAMEMAP_INIT(); + + unsigned a = namemap_get_or_create_id(&m, "greetings"); + unsigned b = namemap_get_or_create_id(&m, "earthlings"); + + tt_str_op(namemap_fmt_name(&m, a), OP_EQ, "greetings"); + tt_str_op(namemap_fmt_name(&m, b), OP_EQ, "earthlings"); + tt_int_op(a, OP_NE, 100); + tt_int_op(b, OP_NE, 100); + tt_str_op(namemap_fmt_name(&m, 100), OP_EQ, "{100}"); + + done: + namemap_clear(&m); +} + #define T(name) \ { #name, test_namemap_ ## name , 0, NULL, NULL } @@ -150,5 +169,6 @@ struct testcase_t namemap_tests[] = { T(toolong), T(blackbox), T(internals), + T(fmt), END_OF_TESTCASES };

1 0

[tor/master] Try a different approach to making publish function seem used.
by asn＠torproject.org 27 Mar '19

27 Mar '19

commit beedadbeacc00222e1bdebec638c1e8cd48a8b14 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Mar 12 19:25:31 2019 -0400 Try a different approach to making publish function seem used. We want the DISPATCH_ADD_PUB() macro to count as making a DECLARE_PUBLISH() invocation "used", so let's try a new approach that preserves that idea. The old one apparently did not work for some versions of osx clang. --- src/lib/pubsub/pubsub_macros.h | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/lib/pubsub/pubsub_macros.h b/src/lib/pubsub/pubsub_macros.h index e4fdd15ae..12cfa7d39 100644 --- a/src/lib/pubsub/pubsub_macros.h +++ b/src/lib/pubsub/pubsub_macros.h @@ -268,12 +268,20 @@ } \ EAT_SEMICOLON +/** + * Add a fake use of the publish function for 'messagename', so that + * the compiler does not call it unused. + */ +#define DISPATCH__FAKE_USE_OF_PUBFN_(messagename) \ + ( 0 ? (publish_fn__ ##messagename((msg_arg_type__##messagename)0), 1) \ + : 1) + /* * This macro is for internal use. It backs DISPATCH_ADD_PUB*() */ #define DISPATCH_ADD_PUB_(connector, channel, messagename, flags) \ ( \ - ((void)publish_fn__ ##messagename), \ + DISPATCH__FAKE_USE_OF_PUBFN_(messagename), \ pubsub_add_pub_((connector), \ &pub_binding__ ##messagename, \ get_channel_id(# channel), \

1 0

[tor/master] Correct doxygen @file directives
by asn＠torproject.org 27 Mar '19

27 Mar '19

commit 22ad8658cd3bc0c0a261975f1eb67cf408fc47d5 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Mar 12 18:57:28 2019 -0400 Correct doxygen @file directives --- src/lib/pubsub/pubsub_macros.h | 2 +- src/lib/pubsub/pubsub_publish.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib/pubsub/pubsub_macros.h b/src/lib/pubsub/pubsub_macros.h index 4d0638d1d..e4fdd15ae 100644 --- a/src/lib/pubsub/pubsub_macros.h +++ b/src/lib/pubsub/pubsub_macros.h @@ -5,7 +5,7 @@ /* See LICENSE for licensing information */ /** - * \file msg.h + * \file pubsub_macros.h * \brief Macros to help with the publish/subscribe dispatch API. * * The dispatch API allows different subsystems of Tor to communicate with diff --git a/src/lib/pubsub/pubsub_publish.c b/src/lib/pubsub/pubsub_publish.c index 8c469e8ad..7b070f35c 100644 --- a/src/lib/pubsub/pubsub_publish.c +++ b/src/lib/pubsub/pubsub_publish.c @@ -5,7 +5,7 @@ /* See LICENSE for licensing information */ /** - * @file pubsub_publish.h + * @file pubsub_publish.c * @brief Header for functions to publish using a pub_binding_t. **/

1 0

[tor/master] move pubsub_connector_t typedef to pubsub_connect.h
by asn＠torproject.org 27 Mar '19

27 Mar '19

commit 94feec59cf2f4c28578ba9c75c7ece781ed6f19b Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Mar 12 19:11:52 2019 -0400 move pubsub_connector_t typedef to pubsub_connect.h --- src/lib/pubsub/pubsub_build.h | 17 +++++------------ src/lib/pubsub/pubsub_connect.h | 8 +++++++- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/src/lib/pubsub/pubsub_build.h b/src/lib/pubsub/pubsub_build.h index 3fad25453..93aad50b2 100644 --- a/src/lib/pubsub/pubsub_build.h +++ b/src/lib/pubsub/pubsub_build.h @@ -17,6 +17,7 @@ #include "lib/dispatch/msgtypes.h" struct dispatch_t; +struct pubsub_connector_t; /** * A "dispatch builder" is an incomplete dispatcher, used when @@ -27,14 +28,6 @@ struct dispatch_t; typedef struct pubsub_builder_t pubsub_builder_t; /** - * A "dispatch connector" is a view of the dispatcher that a subsystem - * uses while initializing itself. It is specific to the subsystem, and - * ensures that each subsystem doesn't need to identify itself - * repeatedly while registering its messages. - **/ -typedef struct pubsub_connector_t pubsub_connector_t; - -/** * A "pubsub items" holds the configuration items used to configure a * pubsub_builder. After the builder is finalized, this field is extracted, * and used later to tear down pointers that enable publishing. @@ -65,16 +58,16 @@ void pubsub_builder_free_(pubsub_builder_t *); * register its messages. The main-init code does this during susbsystem * initialization. */ -pubsub_connector_t *pubsub_connector_for_subsystem(pubsub_builder_t *, - subsys_id_t); +struct pubsub_connector_t *pubsub_connector_for_subsystem(pubsub_builder_t *, + subsys_id_t); /** * The main-init code does this after subsystem initialization. */ #define pubsub_connector_free(c) \ - FREE_AND_NULL(pubsub_connector_t, pubsub_connector_free_, (c)) + FREE_AND_NULL(struct pubsub_connector_t, pubsub_connector_free_, (c)) -void pubsub_connector_free_(pubsub_connector_t *); +void pubsub_connector_free_(struct pubsub_connector_t *); /** * Constructs a dispatcher from a dispatch_builder, after checking that the diff --git a/src/lib/pubsub/pubsub_connect.h b/src/lib/pubsub/pubsub_connect.h index f3de74c87..e4d860005 100644 --- a/src/lib/pubsub/pubsub_connect.h +++ b/src/lib/pubsub/pubsub_connect.h @@ -19,7 +19,13 @@ #include "lib/dispatch/msgtypes.h" struct pub_binding_t; -struct pubsub_connector_t; +/** + * A "dispatch connector" is a view of the dispatcher that a subsystem + * uses while initializing itself. It is specific to the subsystem, and + * ensures that each subsystem doesn't need to identify itself + * repeatedly while registering its messages. + **/ +typedef struct pubsub_connector_t pubsub_connector_t; int pubsub_add_pub_(struct pubsub_connector_t *con, struct pub_binding_t *out,

1 0

[tor/master] pubsub_check.c: Stop accepting NULL prefix, which we never send.
by asn＠torproject.org 27 Mar '19

27 Mar '19

commit 2e7f80d5f4e8b56f060516f71f13590593b1d876 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Mar 13 15:54:26 2019 -0400 pubsub_check.c: Stop accepting NULL prefix, which we never send. (Our code to handle it was broken, too) --- src/lib/pubsub/pubsub_check.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/lib/pubsub/pubsub_check.c b/src/lib/pubsub/pubsub_check.c index 1b3853d8b..e87c347c4 100644 --- a/src/lib/pubsub/pubsub_check.c +++ b/src/lib/pubsub/pubsub_check.c @@ -155,8 +155,7 @@ format_flags(unsigned flags) static void pubsub_cfg_dump(const pubsub_cfg_t *cfg, int severity, const char *prefix) { - if (!prefix) - prefix = 0; + tor_assert(prefix); tor_log(severity, LD_MESG, "%s%s %s: %s{%s} on %s (%s) <%u %u %u %u %x> [%s:%d]",

1 0

[tor/master] pubsub: relationship checking functionality
by asn＠torproject.org 27 Mar '19

27 Mar '19

commit 271a67182211a954dba1082739f8fe7daf421f6c Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Jan 15 09:01:41 2019 -0500 pubsub: relationship checking functionality This code tries to prevent a large number of possible errors by enforcing different restrictions on the messages that different modules publish and subscribe to. Some of these rules are probably too strict, and some too lax: we should feel free to change them as needed as we move forward and learn more. --- .gitignore | 2 + Makefile.am | 2 + src/include.am | 1 + src/lib/pubsub/include.am | 1 + src/lib/pubsub/pubsub_build.c | 2 - src/lib/pubsub/pubsub_check.c | 368 ++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 374 insertions(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index f4f6dacbb..47b23c647 100644 --- a/.gitignore +++ b/.gitignore @@ -202,6 +202,8 @@ uptime-*.json /src/lib/libtor-osinfo-testing.a /src/lib/libtor-process.a /src/lib/libtor-process-testing.a +/src/lib/libtor-pubsub.a +/src/lib/libtor-pubsub-testing.a /src/lib/libtor-sandbox.a /src/lib/libtor-sandbox-testing.a /src/lib/libtor-string.a diff --git a/Makefile.am b/Makefile.am index 36d9725f3..44cfe1787 100644 --- a/Makefile.am +++ b/Makefile.am @@ -41,6 +41,7 @@ TOR_UTIL_LIBS = \ src/lib/libtor-geoip.a \ src/lib/libtor-process.a \ src/lib/libtor-buf.a \ + src/lib/libtor-pubsub.a \ src/lib/libtor-dispatch.a \ src/lib/libtor-time.a \ src/lib/libtor-fs.a \ @@ -73,6 +74,7 @@ TOR_UTIL_TESTING_LIBS = \ src/lib/libtor-geoip-testing.a \ src/lib/libtor-process-testing.a \ src/lib/libtor-buf-testing.a \ + src/lib/libtor-pubsub-testing.a \ src/lib/libtor-dispatch-testing.a \ src/lib/libtor-time-testing.a \ src/lib/libtor-fs-testing.a \ diff --git a/src/include.am b/src/include.am index c6c351c80..77c126ba4 100644 --- a/src/include.am +++ b/src/include.am @@ -25,6 +25,7 @@ include src/lib/malloc/include.am include src/lib/net/include.am include src/lib/osinfo/include.am include src/lib/process/include.am +include src/lib/pubsub/include.am include src/lib/sandbox/include.am include src/lib/string/include.am include src/lib/subsys/include.am diff --git a/src/lib/pubsub/include.am b/src/lib/pubsub/include.am index 9856c94a5..0ab2fd7b3 100644 --- a/src/lib/pubsub/include.am +++ b/src/lib/pubsub/include.am @@ -7,6 +7,7 @@ endif src_lib_libtor_pubsub_a_SOURCES = \ src/lib/pubsub/pubsub_build.c \ + src/lib/pubsub/pubsub_check.c \ src/lib/pubsub/pubsub_publish.c src_lib_libtor_pubsub_testing_a_SOURCES = \ diff --git a/src/lib/pubsub/pubsub_build.c b/src/lib/pubsub/pubsub_build.c index 72f2eacea..7e4ab5ba8 100644 --- a/src/lib/pubsub/pubsub_build.c +++ b/src/lib/pubsub/pubsub_build.c @@ -268,10 +268,8 @@ pubsub_builder_finalize(pubsub_builder_t *builder) if (builder->n_errors) goto err; - /* Coming in the next commit. if (pubsub_builder_check(builder) < 0) goto err; - */ dispatcher = dispatch_new(builder->cfg); diff --git a/src/lib/pubsub/pubsub_check.c b/src/lib/pubsub/pubsub_check.c new file mode 100644 index 000000000..1b3853d8b --- /dev/null +++ b/src/lib/pubsub/pubsub_check.c @@ -0,0 +1,368 @@ +/* Copyright (c) 2001, Matej Pfajfar. + * Copyright (c) 2001-2004, Roger Dingledine. + * Copyright (c) 2004-2006, Roger Dingledine, Nick Mathewson. + * Copyright (c) 2007-2018, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +/** + * @file pubsub_check.c + * @brief Enforce various requirements on a pubsub_builder. + **/ + +#define PUBSUB_PRIVATE + +#include "lib/dispatch/dispatch_naming.h" +#include "lib/dispatch/msgtypes.h" +#include "lib/pubsub/pubsub_flags.h" +#include "lib/pubsub/pubsub_builder_st.h" +#include "lib/pubsub/pubsub_build.h" + +#include "lib/container/bitarray.h" +#include "lib/container/smartlist.h" +#include "lib/log/util_bug.h" +#include "lib/malloc/malloc.h" +#include "lib/string/compat_string.h" + +#include <string.h> + +static void pubsub_adjmap_add(pubsub_adjmap_t *map, + const pubsub_cfg_t *item); + +/** + * Helper: contruct and return a new pubsub_adjacency_map from cfg. + * Return NULL on error. + **/ +static pubsub_adjmap_t * +pubsub_build_adjacency_map(const pubsub_items_t *cfg) +{ + pubsub_adjmap_t *map = tor_malloc_zero(sizeof(*map)); + const size_t n_subsystems = get_num_subsys_ids(); + const size_t n_msgs = get_num_message_ids(); + + map->n_subsystems = n_subsystems; + map->n_msgs = n_msgs; + + map->pub_by_subsys = tor_calloc(n_subsystems, sizeof(smartlist_t*)); + map->sub_by_subsys = tor_calloc(n_subsystems, sizeof(smartlist_t*)); + map->pub_by_msg = tor_calloc(n_msgs, sizeof(smartlist_t*)); + map->sub_by_msg = tor_calloc(n_msgs, sizeof(smartlist_t*)); + + SMARTLIST_FOREACH_BEGIN(cfg->items, const pubsub_cfg_t *, item) { + pubsub_adjmap_add(map, item); + } SMARTLIST_FOREACH_END(item); + + return map; +} + +/** + * Helper: add a single pubsub_cfg_t to an adjacency map. + **/ +static void +pubsub_adjmap_add(pubsub_adjmap_t *map, + const pubsub_cfg_t *item) +{ + smartlist_t **by_subsys; + smartlist_t **by_msg; + + tor_assert(item->subsys < map->n_subsystems); + tor_assert(item->msg < map->n_msgs); + + if (item->is_publish) { + by_subsys = &map->pub_by_subsys[item->subsys]; + by_msg = &map->pub_by_msg[item->msg]; + } else { + by_subsys = &map->sub_by_subsys[item->subsys]; + by_msg = &map->sub_by_msg[item->msg]; + } + + if (! *by_subsys) + *by_subsys = smartlist_new(); + if (! *by_msg) + *by_msg = smartlist_new(); + smartlist_add(*by_subsys, (void*) item); + smartlist_add(*by_msg, (void *) item); +} + +/** + * Release all storage held by m and set m to NULL. + **/ +#define pubsub_adjmap_free(m) \ + FREE_AND_NULL(pubsub_adjmap_t, pubsub_adjmap_free_, m) + +/** + * Free every element of an n-element array of smartlists, then + * free the array itself. + **/ +static void +pubsub_adjmap_free_helper(smartlist_t **lsts, size_t n) +{ + if (!lsts) + return; + + for (unsigned i = 0; i < n; ++i) { + smartlist_free(lsts[i]); + } + tor_free(lsts); +} + +/** + * Release all storage held by map. + **/ +static void +pubsub_adjmap_free_(pubsub_adjmap_t *map) +{ + if (!map) + return; + pubsub_adjmap_free_helper(map->pub_by_subsys, map->n_subsystems); + pubsub_adjmap_free_helper(map->sub_by_subsys, map->n_subsystems); + pubsub_adjmap_free_helper(map->pub_by_msg, map->n_msgs); + pubsub_adjmap_free_helper(map->sub_by_msg, map->n_msgs); + tor_free(map); +} + +/** + * Helper: return the length of sl, or 0 if sl is NULL. + **/ +static int +smartlist_len_opt(const smartlist_t *sl) +{ + if (sl) + return smartlist_len(sl); + else + return 0; +} + +/** Return a pointer to a statically allocated string encoding the + * dispatcher flags in flags. */ +static const char * +format_flags(unsigned flags) +{ + static char buf[32]; + buf[0] = 0; + if (flags & DISP_FLAG_EXCL) { + strlcat(buf, " EXCL", sizeof(buf)); + } + if (flags & DISP_FLAG_STUB) { + strlcat(buf, " STUB", sizeof(buf)); + } + return buf[0] ? buf+1 : buf; +} + +/** + * Log a message containing a description of cfg at severity, prefixed + * by the string prefix. + */ +static void +pubsub_cfg_dump(const pubsub_cfg_t *cfg, int severity, const char *prefix) +{ + if (!prefix) + prefix = 0; + + tor_log(severity, LD_MESG, + "%s%s %s: %s{%s} on %s (%s) <%u %u %u %u %x> [%s:%d]", + prefix, + get_subsys_id_name(cfg->subsys), + cfg->is_publish ? "PUB" : "SUB", + get_message_id_name(cfg->msg), + get_msg_type_id_name(cfg->type), + get_channel_id_name(cfg->channel), + format_flags(cfg->flags), + cfg->subsys, cfg->msg, cfg->type, cfg->channel, cfg->flags, + cfg->added_by_file, cfg->added_by_line); +} + +/** + * Check whether there are any errors or inconsistencies for the message + * described by msg in map. If there are problems, log about + * them, and return -1. Otherwise return 0. + **/ +static int +lint_message(const pubsub_adjmap_t *map, message_id_t msg) +{ + /* NOTE: Some of the checks in this function are maybe over-zealous, and we + * might not want to have them forever. I've marked them with [?] below. + */ + if (BUG(msg >= map->n_msgs)) + return 0; // LCOV_EXCL_LINE + + const smartlist_t *pub = map->pub_by_msg[msg]; + const smartlist_t *sub = map->sub_by_msg[msg]; + + const size_t n_pub = smartlist_len_opt(pub); + const size_t n_sub = smartlist_len_opt(sub); + + if (n_pub == 0 && n_sub == 0) { + log_info(LD_MESG, "Nobody is publishing or subscribing to message %u " + "(%s).", + msg, get_message_id_name(msg)); + return 0; // No publishers or subscribers: nothing to do. + } + + /* We'll set this to false if there are any problems. */ + bool ok = true; + + /* First make sure that if there are publishers, there are subscribers. */ + if (n_pub == 0) { + log_warn(LD_MESG|LD_BUG, + "Message %u (%s) has subscribers, but no publishers.", + msg, get_message_id_name(msg)); + ok = false; + } else if (n_sub == 0) { + log_warn(LD_MESG|LD_BUG, + "Message %u (%s) has publishers, but no subscribers.", + msg, get_message_id_name(msg)); + ok = false; + } + + /* The 'all' list has the publishers and the subscribers. */ + smartlist_t *all = smartlist_new(); + if (pub) + smartlist_add_all(all, pub); + if (sub) + smartlist_add_all(all, sub); + const pubsub_cfg_t *item0 = smartlist_get(all, 0); + + /* Indicates which subsystems we've found publishing/subscribing here. */ + bitarray_t *published_by = bitarray_init_zero((unsigned)map->n_subsystems); + bitarray_t *subscribed_by = bitarray_init_zero((unsigned)map->n_subsystems); + bool pub_excl = false, sub_excl = false, chan_same = true, type_same = true; + + /* Make sure that the messages all have the same channel and type; + * check whether the DISP_FLAG_EXCL flag is used; + * and if any subsystem is publishing or subscribing to it twice [??]. + */ + SMARTLIST_FOREACH_BEGIN(all, const pubsub_cfg_t *, cfg) { + if (cfg->channel != item0->channel) { + chan_same = false; + } + if (cfg->type != item0->type) { + type_same = false; + } + if (cfg->flags & DISP_FLAG_EXCL) { + if (cfg->is_publish) + pub_excl = true; + else + sub_excl = true; + } + if (cfg->is_publish) { + if (bitarray_is_set(published_by, cfg->subsys)) { + log_warn(LD_MESG|LD_BUG, + "Message %u (%s) is configured to be published by subsystem " + "%u (%s) more than once.", + msg, get_message_id_name(msg), + cfg->subsys, get_subsys_id_name(cfg->subsys)); + ok = false; + } + bitarray_set(published_by, cfg->subsys); + } else { + if (bitarray_is_set(subscribed_by, cfg->subsys)) { + log_warn(LD_MESG|LD_BUG, + "Message %u (%s) is configured to be subscribed by subsystem " + "%u (%s) more than once.", + msg, get_message_id_name(msg), + cfg->subsys, get_subsys_id_name(cfg->subsys)); + ok = false; + } + bitarray_set(subscribed_by, cfg->subsys); + } + } SMARTLIST_FOREACH_END(cfg); + + /* Check whether any subsystem is publishing and subscribing the same + * message. [??] + */ + for (unsigned i = 0; i < map->n_subsystems; ++i) { + if (bitarray_is_set(published_by, i) && + bitarray_is_set(subscribed_by, i)) { + log_warn(LD_MESG|LD_BUG, + "Message %u (%s) is published and subscribed by the same " + "subsystem %u (%s)", + msg, get_message_id_name(msg), + i, get_subsys_id_name(i)); + ok = false; + } + } + + if (! chan_same) { + log_warn(LD_MESG|LD_BUG, + "Message %u (%s) is associated with multiple inconsistent " + "channels.", + msg, get_message_id_name(msg)); + ok = false; + } + if (! type_same) { + log_warn(LD_MESG|LD_BUG, + "Message %u (%s) is associated with multiple inconsistent " + "message types.", + msg, get_message_id_name(msg)); + ok = false; + } + + /* Enforce exclusive-ness for publishers and subscribers that have asked for + * it. + */ + if (pub_excl && smartlist_len(pub) > 1) { + log_warn(LD_MESG|LD_BUG, + "Message %u (%s) has multiple publishers, but at least one is " + "marked as exclusive.", + msg, get_message_id_name(msg)); + ok = false; + } + if (sub_excl && smartlist_len(sub) > 1) { + log_warn(LD_MESG|LD_BUG, + "Message %u (%s) has multiple subscribers, but at least one is " + "marked as exclusive.", + msg, get_message_id_name(msg)); + ok = false; + } + + if (!ok) { + /* There was a problem -- let's log all the publishers and subscribers on + * this message */ + SMARTLIST_FOREACH(all, pubsub_cfg_t *, cfg, + pubsub_cfg_dump(cfg, LOG_WARN, " ")); + } + + smartlist_free(all); + bitarray_free(published_by); + bitarray_free(subscribed_by); + + return ok ? 0 : -1; +} + +/** + * Check all the messages in map for consistency. Return 0 on success, + * -1 on problems. + **/ +static int +pubsub_adjmap_check(const pubsub_adjmap_t *map) +{ + bool all_ok = true; + for (unsigned i = 0; i < map->n_msgs; ++i) { + if (lint_message(map, i) < 0) { + all_ok = false; + } + } + return all_ok ? 0 : -1; +} + +/** + * Check builder for consistency and various constraints. Return 0 on success, + * -1 on failure. + **/ +int +pubsub_builder_check(pubsub_builder_t *builder) +{ + pubsub_adjmap_t *map = pubsub_build_adjacency_map(builder->items); + int rv = -1; + + if (!map) + goto err; // should be impossible + + if (pubsub_adjmap_check(map) < 0) + goto err; + + rv = 0; + err: + pubsub_adjmap_free(map); + return rv; +}

1 0

[tor/master] pubsub: install libevent events separately from the_dispatcher.
by asn＠torproject.org 27 Mar '19

27 Mar '19

commit b4f28b9df8188af82a0140b1831ee4b50c6e4f6d Author: Nick Mathewson <nickm(a)torproject.org> Date: Mon Mar 11 16:57:33 2019 -0400 pubsub: install libevent events separately from the_dispatcher. Also, add documentation, and fix a free-on-error bug. --- src/app/main/main.c | 7 +++++++ src/core/mainloop/mainloop_pubsub.c | 28 +++++++++++++++++++++++----- src/core/mainloop/mainloop_pubsub.h | 1 + 3 files changed, 31 insertions(+), 5 deletions(-) diff --git a/src/app/main/main.c b/src/app/main/main.c index 7bf9d3fe2..e0f635287 100644 --- a/src/app/main/main.c +++ b/src/app/main/main.c @@ -1428,6 +1428,13 @@ tor_run_main(const tor_main_configuration_t *tor_cfg) } } + if (get_options()->command == CMD_RUN_TOR) { + tor_mainloop_connect_pubsub_events(); + /* XXXX For each pubsub channel, its delivery strategy should be set at + * this XXXX point, using tor_mainloop_set_delivery_strategy(). + */ + } + if (get_options()->Sandbox && get_options()->command == CMD_RUN_TOR) { sandbox_cfg_t* cfg = sandbox_init_filter(); diff --git a/src/core/mainloop/mainloop_pubsub.c b/src/core/mainloop/mainloop_pubsub.c index ab3614ae0..aac607126 100644 --- a/src/core/mainloop/mainloop_pubsub.c +++ b/src/core/mainloop/mainloop_pubsub.c @@ -44,6 +44,9 @@ flush_channel_event(mainloop_event_t *ev, void *arg) dispatch_flush(the_dispatcher, chan, INT_MAX); } +/** + * Construct our global pubsub object from builder. Return 0 on + * success, -1 on failure. */ int tor_mainloop_connect_pubsub(struct pubsub_builder_t *builder) { @@ -54,6 +57,26 @@ tor_mainloop_connect_pubsub(struct pubsub_builder_t *builder) if (! the_dispatcher) goto err; + rv = 0; + goto done; + err: + tor_mainloop_disconnect_pubsub(); + done: + return rv; +} + +/** + * Install libevent events for all of the pubsub channels. + * + * Invoke this after tor_mainloop_connect_pubsub, and after libevent has been + * initialized. + */ +void +tor_mainloop_connect_pubsub_events(void) +{ + tor_assert(the_dispatcher); + tor_assert(! alert_events); + const size_t num_channels = get_num_channel_ids(); alert_events = smartlist_new(); for (size_t i = 0; i < num_channels; ++i) { @@ -61,11 +84,6 @@ tor_mainloop_connect_pubsub(struct pubsub_builder_t *builder) mainloop_event_postloop_new(flush_channel_event, (void*)(uintptr_t)(i))); } - - rv = 0; - err: - tor_mainloop_disconnect_pubsub(); - return rv; } /** diff --git a/src/core/mainloop/mainloop_pubsub.h b/src/core/mainloop/mainloop_pubsub.h index 6eff77842..a31b2b4ba 100644 --- a/src/core/mainloop/mainloop_pubsub.h +++ b/src/core/mainloop/mainloop_pubsub.h @@ -16,6 +16,7 @@ typedef enum { } deliv_strategy_t; int tor_mainloop_connect_pubsub(struct pubsub_builder_t *builder); +void tor_mainloop_connect_pubsub_events(void); int tor_mainloop_set_delivery_strategy(const char *msg_channel_name, deliv_strategy_t strategy); void tor_mainloop_disconnect_pubsub(void);

1 0

[tor/master] pubsub test: add a test to make sure typefns can't be changed.
by asn＠torproject.org 27 Mar '19

27 Mar '19

commit 9fb511526ae37cb7c7b5ed4714550f58882ef697 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed Mar 13 15:41:36 2019 -0400 pubsub test: add a test to make sure typefns can't be changed. --- src/test/test_dispatch.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/test/test_dispatch.c b/src/test/test_dispatch.c index ec704c042..061e9df14 100644 --- a/src/test/test_dispatch.c +++ b/src/test/test_dispatch.c @@ -170,6 +170,30 @@ test_dispatch_with_types(void *arg) dispatcher_in_use = NULL; } +static void +test_dispatch_bad_type_setup(void *arg) +{ + (void)arg; + static dispatch_typefns_t fns; + dispatch_cfg_t *cfg = dcfg_new(); + + tt_int_op(0, OP_EQ, dcfg_type_set_fns(cfg, 7, &coord_fns)); + + fns = coord_fns; + fns.fmt_fn = NULL; + tt_int_op(-1, OP_EQ, dcfg_type_set_fns(cfg, 7, &fns)); + + fns = coord_fns; + fns.free_fn = NULL; + tt_int_op(-1, OP_EQ, dcfg_type_set_fns(cfg, 7, &fns)); + + fns = coord_fns; + tt_int_op(0, OP_EQ, dcfg_type_set_fns(cfg, 7, &fns)); + + done: + dcfg_free(cfg); +} + #define T(name) \ { #name, test_dispatch_ ## name, TT_FORK, NULL, NULL } @@ -177,5 +201,6 @@ struct testcase_t dispatch_tests[] = { T(empty), T(simple), T(with_types), + T(bad_type_setup), END_OF_TESTCASES };

1 0