From 900d89380dea5703a365aed61e998c5f1004b056 Mon Sep 17 00:00:00 2001 From: Mark Vitale Date: Fri, 17 Jul 2020 17:27:51 -0400 Subject: [PATCH] viced: Set FS_STATE_DUMP_MODE earlier When the fileserver saves its callback state to disk on shutdown, fs_stateCreateDump sets state->mode to FS_STATE_DUMP_MODE. But this is after some code paths have already looked at state->mode, such as fs_stateSave -> h_stateVerify -> h_Enumerate_r -> h_stateVerifyHost -> h_stateVerifyAddrHash. This doesn't cause any problems right now, since the value of FS_STATE_DUMP_MODE is 0, and we zero the whole 'state' struct when it is initialized. This is confusing, though, so move the assignment of FS_STATE_DUMP_MODE to earlier in the state-saving process, before anything looks at it. To try to avoid possible uninitialized use of state->mode in the future, change the FS_STATE_*_MODE constants so that none of them are 0, and make sure that code checking the 'mode' checks that the mode is a valid constant. [adeason@sinenomine.net: Added state mode checking.] Change-Id: Ifa5e5aabdb7c7b4a50a1b397620aafa7c48c223e Reviewed-on: https://gerrit.openafs.org/14726 Reviewed-by: Mark Vitale Reviewed-by: Cheyenne Wills Reviewed-by: Michael Meffie Tested-by: BuildBot Reviewed-by: Andrew Deason --- src/viced/host.c | 14 ++++++++++++-- src/viced/serialize_state.c | 5 +++-- src/viced/serialize_state.h | 4 ++-- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/src/viced/host.c b/src/viced/host.c index b8c1550c57..cfa7a55947 100644 --- a/src/viced/host.c +++ b/src/viced/host.c @@ -3391,10 +3391,15 @@ h_stateVerifyAddrHash(struct fs_dump_state * state, struct host * h, tmp, (unsigned)htons(port))); ret = 1; goto done; - } else { + } else if (state->mode == FS_STATE_DUMP_MODE) { ViceLog(0, ("h_stateVerifyAddrHash: warning: addr %s:%u not found in hash\n", tmp, (unsigned)htons(port))); state->flags.warnings_generated = 1; + } else { + ViceLog(0, ("h_stateVerifyAddrHash: error: bad state mode %d\n", + state->mode)); + ret = 1; + goto done; } } @@ -3448,10 +3453,15 @@ h_stateVerifyUuidHash(struct fs_dump_state * state, struct host * h) tmp.buffer)); ret = 1; goto done; - } else { + } else if (state->mode == FS_STATE_DUMP_MODE) { ViceLog(0, ("h_stateVerifyUuidHash: warning: uuid %s not found in hash\n", tmp.buffer)); state->flags.warnings_generated = 1; + } else { + ViceLog(0, ("h_stateVerifyUuidHash: bad state mode %d\n", + state->mode)); + ret = 1; + goto done; } done: diff --git a/src/viced/serialize_state.c b/src/viced/serialize_state.c index 05ed4dfe73..22d2a70e7e 100644 --- a/src/viced/serialize_state.c +++ b/src/viced/serialize_state.c @@ -123,6 +123,8 @@ fs_stateSave(void) goto done; } + state.mode = FS_STATE_DUMP_MODE; + /* XXX * on busy servers, these checks will inevitably fail since stuff drops H_LOCK * all over the place (with structs left in inconsistent states) while RPCs to @@ -328,7 +330,6 @@ fs_stateCreateDump(struct fs_dump_state * state) } state->fd = fd; - state->mode = FS_STATE_DUMP_MODE; memset(state->hdr, 0, sizeof(struct fs_state_header)); fs_stateIncEOF(state, sizeof(struct fs_state_header)); @@ -769,7 +770,7 @@ fs_stateMapFile(struct fs_dump_state * state, int preserve_flag ) flags = PROT_WRITE; break; default: - ViceLog(0, ("fs_stateMapFile: invalid dump state mode\n")); + ViceLog(0, ("fs_stateMapFile: invalid dump state mode %d\n", state->mode)); return 1; } diff --git a/src/viced/serialize_state.h b/src/viced/serialize_state.h index c8e021b691..1e906b8cbb 100644 --- a/src/viced/serialize_state.h +++ b/src/viced/serialize_state.h @@ -230,8 +230,8 @@ struct idx_map_entry_t { struct fs_dump_state { enum { - FS_STATE_DUMP_MODE, - FS_STATE_LOAD_MODE + FS_STATE_DUMP_MODE = 0x1, + FS_STATE_LOAD_MODE = 0x2 } mode; struct { byte do_host_restore; /* whether host restore should be done */