From bbd893d6bd8e79ab6e303152c871027bc0380a38 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 29 Jan 2019 16:18:41 +0100 Subject: [PATCH 1/2] Write consensus files in binary mode This will help us out on windows now that we mmap files. Fixes part of ticket 28614. --- changes/ticket28614 | 4 ++++ src/feature/nodelist/networkstatus.c | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) create mode 100644 changes/ticket28614 diff --git a/changes/ticket28614 b/changes/ticket28614 new file mode 100644 index 0000000000..c022baf668 --- /dev/null +++ b/changes/ticket28614 @@ -0,0 +1,4 @@ + o Major bugfixes (windows, startup): + - When writing a consensus file to disk, always write in + "binary" mode so that we can safely map it into memory later. + Fixes part bug 28614; bugfix on 0.4.0.1-alpha. diff --git a/src/feature/nodelist/networkstatus.c b/src/feature/nodelist/networkstatus.c index d9659b67c0..64169f6a4b 100644 --- a/src/feature/nodelist/networkstatus.c +++ b/src/feature/nodelist/networkstatus.c @@ -1991,7 +1991,7 @@ networkstatus_set_current_consensus(const char *consensus, waiting->set_at = now; waiting->dl_failed = 0; if (!from_cache) { - write_bytes_to_file(unverified_fname, consensus, consensus_len, 0); + write_bytes_to_file(unverified_fname, consensus, consensus_len, 1); } if (dl_certs) authority_certs_fetch_missing(c, now, source_dir); @@ -2142,7 +2142,7 @@ networkstatus_set_current_consensus(const char *consensus, } if (!from_cache) { - write_bytes_to_file(consensus_fname, consensus, consensus_len, 0); + write_bytes_to_file(consensus_fname, consensus, consensus_len, 1); } warn_early_consensus(c, flavor, now); From 72b978c3a58983d9b4d57f56db618e074ddae31e Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 30 Jan 2019 09:22:36 +0100 Subject: [PATCH 2/2] On windows, if we fail to load a consensus and it has a CRLF, retry. Fixes bug 28614; bugfix on 0.4.0.1-alpha when we started mmapping the consensus. --- changes/ticket28614 | 6 ++- src/feature/nodelist/networkstatus.c | 81 +++++++++++++++++----------- 2 files changed, 56 insertions(+), 31 deletions(-) diff --git a/changes/ticket28614 b/changes/ticket28614 index c022baf668..6c65ce49de 100644 --- a/changes/ticket28614 +++ b/changes/ticket28614 @@ -1,4 +1,8 @@ o Major bugfixes (windows, startup): - When writing a consensus file to disk, always write in "binary" mode so that we can safely map it into memory later. - Fixes part bug 28614; bugfix on 0.4.0.1-alpha. + Fixes part of bug 28614; bugfix on 0.4.0.1-alpha. + - When reading a consensus file from disk, detect whether it + was written in text mode, and re-read it in text mode if it + Fixes part of bug 28614; bugfix on 0.4.0.1-alpha. + diff --git a/src/feature/nodelist/networkstatus.c b/src/feature/nodelist/networkstatus.c index 64169f6a4b..2c34754621 100644 --- a/src/feature/nodelist/networkstatus.c +++ b/src/feature/nodelist/networkstatus.c @@ -178,6 +178,10 @@ static void update_consensus_bootstrap_multiple_downloads( static int networkstatus_check_required_protocols(const networkstatus_t *ns, int client_mode, char **warning_out); +static int reload_consensus_from_file(const char *fname, + const char *flavor, + unsigned flags, + const char *source_dir); /** Forget that we've warned about anything networkstatus-related, so we will * give fresh warnings if the same behavior happens again. */ @@ -269,27 +273,15 @@ router_reload_consensus_networkstatus(void) /* FFFF Suppress warnings if cached consensus is bad? */ for (flav = 0; flav < N_CONSENSUS_FLAVORS; ++flav) { const char *flavor = networkstatus_get_flavor_name(flav); - tor_mmap_t *m = networkstatus_map_cached_consensus_impl(flav, flavor, 0); - if (m) { - if (networkstatus_set_current_consensus(m->data, m->size, - flavor, flags, NULL) < -1) { - log_warn(LD_FS, "Couldn't load consensus %s networkstatus from cache", - flavor); - } - tor_munmap_file(m); - } + char *fname = networkstatus_get_cache_fname(flav, flavor, 0); + reload_consensus_from_file(fname, flavor, flags, NULL); + tor_free(fname); - m = networkstatus_map_cached_consensus_impl(flav, flavor, 1); - if (m) { - if (networkstatus_set_current_consensus(m->data, m->size, - flavor, - flags | NSSET_WAS_WAITING_FOR_CERTS, - NULL)) { - log_info(LD_FS, "Couldn't load unverified consensus %s networkstatus " - "from cache", flavor); - } - tor_munmap_file(m); - } + fname = networkstatus_get_cache_fname(flav, flavor, 1); + reload_consensus_from_file(fname, flavor, + flags | NSSET_WAS_WAITING_FOR_CERTS, + NULL); + tor_free(fname); } update_certificate_downloads(time(NULL)); @@ -1750,6 +1742,41 @@ networkstatus_set_current_consensus_from_ns(networkstatus_t *c, } #endif /* defined(TOR_UNIT_TESTS) */ +/** + * Helper: Read a the current consensus of type flavor from + * fname. Flags and return values are as for + * networkstatus_set_current_consensus(). + **/ +static int +reload_consensus_from_file(const char *fname, + const char *flavor, + unsigned flags, + const char *source_dir) +{ + tor_mmap_t *map = tor_mmap_file(fname); + if (!map) + return 0; + + int rv = networkstatus_set_current_consensus(map->data, map->size, + flavor, flags, source_dir); +#ifdef _WIN32 + if (rv < 0 && tor_memstr(map->data, map->size, "\r\n")) { + log_info(LD_GENERAL, "Found CRLF in consensus file %s; falling back to " + "read_file_to_string.", escaped(fname)); + char *content = read_file_to_str(fname, RFTS_IGNORE_MISSING, NULL); + rv = networkstatus_set_current_consensus(content, strlen(content), + flavor, flags, source_dir); + tor_free(content); + } +#endif + if (rv < -1) { + log_warn(LD_GENERAL, "Couldn't set consensus from cache file %s", + escaped(fname)); + } + tor_munmap_file(map); + return rv; +} + /** * Helper for handle_missing_protocol_warning: handles either the * client case (if is_client is set) or the server case otherwise. @@ -2178,16 +2205,10 @@ networkstatus_note_certs_arrived(const char *source_dir) if (!waiting->consensus) continue; if (networkstatus_check_consensus_signature(waiting->consensus, 0)>=0) { - tor_mmap_t *mapping = networkstatus_map_cached_consensus_impl( - i, flavor_name, 1); - if (mapping) { - networkstatus_set_current_consensus(mapping->data, - mapping->size, - flavor_name, - NSSET_WAS_WAITING_FOR_CERTS, - source_dir); - } - tor_munmap_file(mapping); + char *fname = networkstatus_get_cache_fname(i, flavor_name, 1); + reload_consensus_from_file(fname, flavor_name, + NSSET_WAS_WAITING_FOR_CERTS, source_dir); + tor_free(fname); } } }