From 2b8293c9f6631fec6c0376b49710ae38388bdd59 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Wed, 11 Apr 2018 08:33:35 +0930 Subject: [PATCH] gossipd: don't use pwrite, better error messaging on init. Since we open with O_APPEND, any write() will append as we want it to. But we want to distinguish a new store creation from a truncation due to bad version. Signed-off-by: Rusty Russell --- gossipd/gossip_store.c | 54 ++++++++++++++++++-------- gossipd/test/run-bench-find_route.c | 3 ++ gossipd/test/run-find_route-specific.c | 3 ++ gossipd/test/run-find_route.c | 3 ++ 4 files changed, 46 insertions(+), 17 deletions(-) diff --git a/gossipd/gossip_store.c b/gossipd/gossip_store.c index 5590e6765..2cd3a069d 100644 --- a/gossipd/gossip_store.c +++ b/gossipd/gossip_store.c @@ -16,7 +16,6 @@ static u8 gossip_store_version = 0x01; struct gossip_store { int fd; - off_t write_pos; u8 version; }; @@ -29,22 +28,29 @@ struct gossip_store *gossip_store_new(const tal_t *ctx) { struct gossip_store *gs = tal(ctx, struct gossip_store); gs->fd = open(GOSSIP_STORE_FILENAME, O_RDWR|O_APPEND|O_CREAT, 0600); - gs->write_pos = lseek(gs->fd, 0, SEEK_END); + + tal_add_destructor(gs, gossip_store_destroy); /* Try to read the version, write it if this is a new file, or truncate * if the version doesn't match */ - if (pread(gs->fd, &gs->version, sizeof(gs->version), 0) != 1 || - gs->version != gossip_store_version) { - status_trace("Truncating gossip_store, either it was empty or " - "the version was not supported."); - gs->version = gossip_store_version; - gs->write_pos = 1; - pwrite(gs->fd, &gossip_store_version, sizeof(gossip_store_version), 0); - ftruncate(gs->fd, gs->write_pos); + if (read(gs->fd, &gs->version, sizeof(gs->version)) + == sizeof(gs->version)) { + /* Version match? All good */ + if (gs->version == gossip_store_version) + return gs; + + status_unusual("Gossip store version %u not %u: removing", + gs->version, gossip_store_version); + if (ftruncate(gs->fd, 0) != 0) + status_failed(STATUS_FAIL_INTERNAL_ERROR, + "Truncating store: %s", strerror(errno)); } - - tal_add_destructor(gs, gossip_store_destroy); - + /* Empty file, write version byte */ + gs->version = gossip_store_version; + if (write(gs->fd, &gs->version, sizeof(gs->version)) + != sizeof(gs->version)) + status_failed(STATUS_FAIL_INTERNAL_ERROR, + "Writing version to store: %s", strerror(errno)); return gs; } /** @@ -59,11 +65,17 @@ static void gossip_store_append(struct gossip_store *gs, const u8 *msg) u32 msglen = tal_len(msg); beint32_t belen = cpu_to_be32(msglen); - if (pwrite(gs->fd, &belen, sizeof(belen), gs->write_pos) != sizeof(belen) || - pwrite(gs->fd, msg, msglen, gs->write_pos + sizeof(belen)) != msglen) { + /* Only give error message once. */ + if (gs->fd == -1) return; - } else - gs->write_pos += sizeof(belen) + msglen; + + /* FORTIFY_SOURCE gets upset if we don't check return. */ + if (write(gs->fd, &belen, sizeof(belen)) != sizeof(belen) || + write(gs->fd, msg, msglen) != msglen) { + status_broken("Failed writing to gossip store: %s", + strerror(errno)); + gs->fd = -1; + } } void gossip_store_add_channel_announcement(struct gossip_store *gs, const u8 *gossip_msg, u64 satoshis) @@ -107,6 +119,7 @@ void gossip_store_load(struct routing_state *rstate, struct gossip_store *gs) /* We set/check version byte on creation */ off_t known_good = 1; const char *bad; + size_t stats[] = {0, 0, 0, 0}; lseek(gs->fd, known_good, SEEK_SET); while (read(gs->fd, &belen, sizeof(belen)) == sizeof(belen)) { @@ -127,18 +140,21 @@ void gossip_store_load(struct routing_state *rstate, struct gossip_store *gs) bad = "Bad channel_announcement"; goto truncate; } + stats[0]++; } else if (fromwire_gossip_store_channel_update(msg, msg, &gossip_msg)) { if (!routing_add_channel_update(rstate, gossip_msg)) { bad = "Bad channel_update"; goto truncate; } + stats[1]++; } else if (fromwire_gossip_store_node_announcement(msg, msg, &gossip_msg)) { if (!routing_add_node_announcement(rstate, gossip_msg)) { bad = "Bad node_announcement"; goto truncate; } + stats[2]++; } else if (fromwire_gossip_store_channel_delete(msg, &scid)) { struct chan *c = get_channel(rstate, &scid); if (!c) { @@ -146,6 +162,7 @@ void gossip_store_load(struct routing_state *rstate, struct gossip_store *gs) goto truncate; } tal_free(c); + stats[3]++; } else { bad = "Unknown message"; goto truncate; @@ -153,6 +170,9 @@ void gossip_store_load(struct routing_state *rstate, struct gossip_store *gs) known_good += sizeof(belen) + msglen; tal_free(msg); } + status_trace("gossip_store: Read %zu/%zu/%zu/%zu cannounce/cupdate/nannounce/cdelete from store in %"PRIu64" bytes", + stats[0], stats[1], stats[2], stats[3], + (u64)known_good); return; truncate: diff --git a/gossipd/test/run-bench-find_route.c b/gossipd/test/run-bench-find_route.c index cc4f5e4b9..63e2d4680 100644 --- a/gossipd/test/run-bench-find_route.c +++ b/gossipd/test/run-bench-find_route.c @@ -86,6 +86,9 @@ u8 fromwire_u8(const u8 **cursor UNNEEDED, size_t *max UNNEEDED) /* Generated stub for fromwire_wireaddr */ bool fromwire_wireaddr(const u8 **cursor UNNEEDED, size_t *max UNNEEDED, struct wireaddr *addr UNNEEDED) { fprintf(stderr, "fromwire_wireaddr called!\n"); abort(); } +/* Generated stub for gossip_wire_type_name */ +const char *gossip_wire_type_name(int e UNNEEDED) +{ fprintf(stderr, "gossip_wire_type_name called!\n"); abort(); } /* Generated stub for onion_type_name */ const char *onion_type_name(int e UNNEEDED) { fprintf(stderr, "onion_type_name called!\n"); abort(); } diff --git a/gossipd/test/run-find_route-specific.c b/gossipd/test/run-find_route-specific.c index 3c220d20d..e75b8b256 100644 --- a/gossipd/test/run-find_route-specific.c +++ b/gossipd/test/run-find_route-specific.c @@ -50,6 +50,9 @@ u8 fromwire_u8(const u8 **cursor UNNEEDED, size_t *max UNNEEDED) /* Generated stub for fromwire_wireaddr */ bool fromwire_wireaddr(const u8 **cursor UNNEEDED, size_t *max UNNEEDED, struct wireaddr *addr UNNEEDED) { fprintf(stderr, "fromwire_wireaddr called!\n"); abort(); } +/* Generated stub for gossip_wire_type_name */ +const char *gossip_wire_type_name(int e UNNEEDED) +{ fprintf(stderr, "gossip_wire_type_name called!\n"); abort(); } /* Generated stub for onion_type_name */ const char *onion_type_name(int e UNNEEDED) { fprintf(stderr, "onion_type_name called!\n"); abort(); } diff --git a/gossipd/test/run-find_route.c b/gossipd/test/run-find_route.c index c042bccee..3feb5ca4c 100644 --- a/gossipd/test/run-find_route.c +++ b/gossipd/test/run-find_route.c @@ -48,6 +48,9 @@ u8 fromwire_u8(const u8 **cursor UNNEEDED, size_t *max UNNEEDED) /* Generated stub for fromwire_wireaddr */ bool fromwire_wireaddr(const u8 **cursor UNNEEDED, size_t *max UNNEEDED, struct wireaddr *addr UNNEEDED) { fprintf(stderr, "fromwire_wireaddr called!\n"); abort(); } +/* Generated stub for gossip_wire_type_name */ +const char *gossip_wire_type_name(int e UNNEEDED) +{ fprintf(stderr, "gossip_wire_type_name called!\n"); abort(); } /* Generated stub for onion_type_name */ const char *onion_type_name(int e UNNEEDED) { fprintf(stderr, "onion_type_name called!\n"); abort(); }