From 676df8770bf4dfce8d45277cfe7f9ebf7fb0f7c3 Mon Sep 17 00:00:00 2001 From: Amitay Isaacs Date: Thu, 29 Jun 2017 23:41:08 +1000 Subject: [PATCH] ctdb-protocol: Fix marshalling for ctdb_rec_buffer Signed-off-by: Amitay Isaacs Reviewed-by: Martin Schwenke --- ctdb/protocol/protocol_api.h | 7 +- ctdb/protocol/protocol_control.c | 32 +++---- ctdb/protocol/protocol_message.c | 4 +- ctdb/protocol/protocol_types.c | 96 ++++++++++++++------- ctdb/server/ctdb_recovery_helper.c | 6 +- ctdb/tests/src/protocol_types_compat_test.c | 56 ++++++++++++ ctdb/tests/src/protocol_types_test.c | 2 +- ctdb/tools/ctdb.c | 3 +- 8 files changed, 148 insertions(+), 58 deletions(-) diff --git a/ctdb/protocol/protocol_api.h b/ctdb/protocol/protocol_api.h index 00752beacd0..6d213708b96 100644 --- a/ctdb/protocol/protocol_api.h +++ b/ctdb/protocol/protocol_api.h @@ -39,10 +39,11 @@ void ctdb_rec_data_push(struct ctdb_rec_data *in, uint8_t *buf, size_t *npush); int ctdb_rec_data_pull(uint8_t *buf, size_t buflen, TALLOC_CTX *mem_ctx, struct ctdb_rec_data **out, size_t *npull); -size_t ctdb_rec_buffer_len(struct ctdb_rec_buffer *recbuf); -void ctdb_rec_buffer_push(struct ctdb_rec_buffer *recbuf, uint8_t *buf); +size_t ctdb_rec_buffer_len(struct ctdb_rec_buffer *in); +void ctdb_rec_buffer_push(struct ctdb_rec_buffer *in, uint8_t *buf, + size_t *npush); int ctdb_rec_buffer_pull(uint8_t *buf, size_t buflen, TALLOC_CTX *mem_ctx, - struct ctdb_rec_buffer **out); + struct ctdb_rec_buffer **out, size_t *npull); struct ctdb_rec_buffer *ctdb_rec_buffer_init(TALLOC_CTX *mem_ctx, uint32_t db_id); diff --git a/ctdb/protocol/protocol_control.c b/ctdb/protocol/protocol_control.c index 6d62e2b9e78..aeb25b33e8e 100644 --- a/ctdb/protocol/protocol_control.c +++ b/ctdb/protocol/protocol_control.c @@ -468,7 +468,7 @@ static void ctdb_req_control_data_push(struct ctdb_req_control_data *cd, break; case CTDB_CONTROL_PUSH_DB: - ctdb_rec_buffer_push(cd->data.recbuf, buf); + ctdb_rec_buffer_push(cd->data.recbuf, buf, &np); break; case CTDB_CONTROL_SET_RECMODE: @@ -547,7 +547,7 @@ static void ctdb_req_control_data_push(struct ctdb_req_control_data *cd, break; case CTDB_CONTROL_UPDATE_RECORD: - ctdb_rec_buffer_push(cd->data.recbuf, buf); + ctdb_rec_buffer_push(cd->data.recbuf, buf, &np); break; case CTDB_CONTROL_SEND_GRATUITOUS_ARP: @@ -559,7 +559,7 @@ static void ctdb_req_control_data_push(struct ctdb_req_control_data *cd, break; case CTDB_CONTROL_TRY_DELETE_RECORDS: - ctdb_rec_buffer_push(cd->data.recbuf, buf); + ctdb_rec_buffer_push(cd->data.recbuf, buf, &np); break; case CTDB_CONTROL_ADD_PUBLIC_IP: @@ -607,7 +607,7 @@ static void ctdb_req_control_data_push(struct ctdb_req_control_data *cd, break; case CTDB_CONTROL_TRANS3_COMMIT: - ctdb_rec_buffer_push(cd->data.recbuf, buf); + ctdb_rec_buffer_push(cd->data.recbuf, buf, &np); break; case CTDB_CONTROL_GET_DB_SEQNUM: @@ -668,7 +668,7 @@ static void ctdb_req_control_data_push(struct ctdb_req_control_data *cd, break; case CTDB_CONTROL_RECEIVE_RECORDS: - ctdb_rec_buffer_push(cd->data.recbuf, buf); + ctdb_rec_buffer_push(cd->data.recbuf, buf, &np); break; case CTDB_CONTROL_DB_DETACH: @@ -753,7 +753,7 @@ static int ctdb_req_control_data_pull(uint8_t *buf, size_t buflen, case CTDB_CONTROL_PUSH_DB: ret = ctdb_rec_buffer_pull(buf, buflen, mem_ctx, - &cd->data.recbuf); + &cd->data.recbuf, &np); break; case CTDB_CONTROL_SET_RECMODE: @@ -846,7 +846,7 @@ static int ctdb_req_control_data_pull(uint8_t *buf, size_t buflen, case CTDB_CONTROL_UPDATE_RECORD: ret = ctdb_rec_buffer_pull(buf, buflen, mem_ctx, - &cd->data.recbuf); + &cd->data.recbuf, &np); break; case CTDB_CONTROL_SEND_GRATUITOUS_ARP: @@ -861,7 +861,7 @@ static int ctdb_req_control_data_pull(uint8_t *buf, size_t buflen, case CTDB_CONTROL_TRY_DELETE_RECORDS: ret = ctdb_rec_buffer_pull(buf, buflen, mem_ctx, - &cd->data.recbuf); + &cd->data.recbuf, &np); break; case CTDB_CONTROL_ADD_PUBLIC_IP: @@ -918,7 +918,7 @@ static int ctdb_req_control_data_pull(uint8_t *buf, size_t buflen, case CTDB_CONTROL_TRANS3_COMMIT: ret = ctdb_rec_buffer_pull(buf, buflen, mem_ctx, - &cd->data.recbuf); + &cd->data.recbuf, &np); break; case CTDB_CONTROL_GET_DB_SEQNUM: @@ -988,7 +988,7 @@ static int ctdb_req_control_data_pull(uint8_t *buf, size_t buflen, case CTDB_CONTROL_RECEIVE_RECORDS: ret = ctdb_rec_buffer_pull(buf, buflen, mem_ctx, - &cd->data.recbuf); + &cd->data.recbuf, &np); break; case CTDB_CONTROL_DB_DETACH: @@ -1439,7 +1439,7 @@ static void ctdb_reply_control_data_push(struct ctdb_reply_control_data *cd, break; case CTDB_CONTROL_PULL_DB: - ctdb_rec_buffer_push(cd->data.recbuf, buf); + ctdb_rec_buffer_push(cd->data.recbuf, buf, &np); break; case CTDB_CONTROL_PUSH_DB: @@ -1488,7 +1488,7 @@ static void ctdb_reply_control_data_push(struct ctdb_reply_control_data *cd, break; case CTDB_CONTROL_TRY_DELETE_RECORDS: - ctdb_rec_buffer_push(cd->data.recbuf, buf); + ctdb_rec_buffer_push(cd->data.recbuf, buf, &np); break; case CTDB_CONTROL_GET_CAPABILITIES: @@ -1542,7 +1542,7 @@ static void ctdb_reply_control_data_push(struct ctdb_reply_control_data *cd, break; case CTDB_CONTROL_RECEIVE_RECORDS: - ctdb_rec_buffer_push(cd->data.recbuf, buf); + ctdb_rec_buffer_push(cd->data.recbuf, buf, &np); break; case CTDB_CONTROL_GET_RUNSTATE: @@ -1607,7 +1607,7 @@ static int ctdb_reply_control_data_pull(uint8_t *buf, size_t buflen, case CTDB_CONTROL_PULL_DB: ret = ctdb_rec_buffer_pull(buf, buflen, mem_ctx, - &cd->data.recbuf); + &cd->data.recbuf, &np); break; case CTDB_CONTROL_PUSH_DB: @@ -1664,7 +1664,7 @@ static int ctdb_reply_control_data_pull(uint8_t *buf, size_t buflen, case CTDB_CONTROL_TRY_DELETE_RECORDS: ctdb_rec_buffer_pull(buf, buflen, mem_ctx, - &cd->data.recbuf); + &cd->data.recbuf, &np); break; case CTDB_CONTROL_GET_CAPABILITIES: @@ -1728,7 +1728,7 @@ static int ctdb_reply_control_data_pull(uint8_t *buf, size_t buflen, case CTDB_CONTROL_RECEIVE_RECORDS: ret = ctdb_rec_buffer_pull(buf, buflen, mem_ctx, - &cd->data.recbuf); + &cd->data.recbuf, &np); break; case CTDB_CONTROL_GET_RUNSTATE: diff --git a/ctdb/protocol/protocol_message.c b/ctdb/protocol/protocol_message.c index 8d83b9bd6fe..f10e4304af9 100644 --- a/ctdb/protocol/protocol_message.c +++ b/ctdb/protocol/protocol_message.c @@ -148,7 +148,7 @@ static void ctdb_message_data_push(union ctdb_message_data *mdata, break; case CTDB_SRVID_VACUUM_FETCH: - ctdb_rec_buffer_push(mdata->recbuf, buf); + ctdb_rec_buffer_push(mdata->recbuf, buf, &np); break; case CTDB_SRVID_DETACH_DATABASE: @@ -234,7 +234,7 @@ static int ctdb_message_data_pull(uint8_t *buf, size_t buflen, case CTDB_SRVID_VACUUM_FETCH: ret = ctdb_rec_buffer_pull(buf, buflen, mem_ctx, - &mdata->recbuf); + &mdata->recbuf, &np); break; case CTDB_SRVID_DETACH_DATABASE: diff --git a/ctdb/protocol/protocol_types.c b/ctdb/protocol/protocol_types.c index 2826ae29f56..017283299d1 100644 --- a/ctdb/protocol/protocol_types.c +++ b/ctdb/protocol/protocol_types.c @@ -1463,57 +1463,87 @@ int ctdb_rec_data_pull(uint8_t *buf, size_t buflen, TALLOC_CTX *mem_ctx, return ret; } -struct ctdb_rec_buffer_wire { - uint32_t db_id; - uint32_t count; - uint8_t data[1]; -}; - -size_t ctdb_rec_buffer_len(struct ctdb_rec_buffer *recbuf) +size_t ctdb_rec_buffer_len(struct ctdb_rec_buffer *in) { - return offsetof(struct ctdb_rec_buffer_wire, data) + recbuf->buflen; + return ctdb_uint32_len(&in->db_id) + + ctdb_uint32_len(&in->count) + + in->buflen; } -void ctdb_rec_buffer_push(struct ctdb_rec_buffer *recbuf, uint8_t *buf) +void ctdb_rec_buffer_push(struct ctdb_rec_buffer *in, uint8_t *buf, + size_t *npush) { - struct ctdb_rec_buffer_wire *wire = (struct ctdb_rec_buffer_wire *)buf; + size_t offset = 0, np; - wire->db_id = recbuf->db_id; - wire->count = recbuf->count; - if (recbuf->buflen > 0) { - memcpy(wire->data, recbuf->buf, recbuf->buflen); - } + ctdb_uint32_push(&in->db_id, buf+offset, &np); + offset += np; + + ctdb_uint32_push(&in->count, buf+offset, &np); + offset += np; + + memcpy(buf+offset, in->buf, in->buflen); + offset += in->buflen; + + *npush = offset; } int ctdb_rec_buffer_pull(uint8_t *buf, size_t buflen, TALLOC_CTX *mem_ctx, - struct ctdb_rec_buffer **out) + struct ctdb_rec_buffer **out, size_t *npull) { - struct ctdb_rec_buffer *recbuf; - struct ctdb_rec_buffer_wire *wire = (struct ctdb_rec_buffer_wire *)buf; - size_t offset; + struct ctdb_rec_buffer *val; + size_t offset = 0, np; + size_t length; + int ret; - if (buflen < offsetof(struct ctdb_rec_buffer_wire, data)) { - return EMSGSIZE; + val = talloc(mem_ctx, struct ctdb_rec_buffer); + if (val == NULL) { + return ENOMEM; } - recbuf = talloc(mem_ctx, struct ctdb_rec_buffer); - if (recbuf == NULL) { - return ENOMEM; + ret = ctdb_uint32_pull(buf+offset, buflen-offset, &val->db_id, &np); + if (ret != 0) { + goto fail; + } + offset += np; + + ret = ctdb_uint32_pull(buf+offset, buflen-offset, &val->count, &np); + if (ret != 0) { + goto fail; } + offset += np; - recbuf->db_id = wire->db_id; - recbuf->count = wire->count; + /* Since there is no buflen provided, walk the records to + * validate the length of the buffer. + */ + val->buf = buf+offset; + val->buflen = buflen-offset; - offset = offsetof(struct ctdb_rec_buffer_wire, data); - recbuf->buflen = buflen - offset; - recbuf->buf = talloc_memdup(recbuf, wire->data, recbuf->buflen); - if (recbuf->buf == NULL) { - talloc_free(recbuf); - return ENOMEM; + length = 0; + ret = ctdb_rec_buffer_traverse(val, NULL, &length); + if (ret != 0) { + goto fail; } - *out = recbuf; + if (length > buflen-offset) { + ret = EMSGSIZE; + goto fail; + } + + val->buf = talloc_memdup(val, buf+offset, length); + if (val->buf == NULL) { + ret = ENOMEM; + goto fail; + } + val->buflen = length; + offset += length; + + *out = val; + *npull = offset; return 0; + +fail: + talloc_free(val); + return ret; } struct ctdb_rec_buffer *ctdb_rec_buffer_init(TALLOC_CTX *mem_ctx, diff --git a/ctdb/server/ctdb_recovery_helper.c b/ctdb/server/ctdb_recovery_helper.c index 9f7fc07d3c4..ab9ab4152b0 100644 --- a/ctdb/server/ctdb_recovery_helper.c +++ b/ctdb/server/ctdb_recovery_helper.c @@ -468,6 +468,7 @@ static void pull_database_handler(uint64_t srvid, TDB_DATA data, struct pull_database_state *state = tevent_req_data( req, struct pull_database_state); struct ctdb_rec_buffer *recbuf; + size_t np; int ret; bool status; @@ -475,7 +476,7 @@ static void pull_database_handler(uint64_t srvid, TDB_DATA data, return; } - ret = ctdb_rec_buffer_pull(data.dptr, data.dsize, state, &recbuf); + ret = ctdb_rec_buffer_pull(data.dptr, data.dsize, state, &recbuf, &np); if (ret != 0) { D_ERR("Invalid data received for DB_PULL messages\n"); return; @@ -892,6 +893,7 @@ static void push_database_new_send_msg(struct tevent_req *req) struct ctdb_rec_buffer *recbuf; struct ctdb_req_message message; TDB_DATA data; + size_t np; int ret; if (state->num_buffers_sent == state->num_buffers) { @@ -924,7 +926,7 @@ static void push_database_new_send_msg(struct tevent_req *req) return; } - ctdb_rec_buffer_push(recbuf, data.dptr); + ctdb_rec_buffer_push(recbuf, data.dptr, &np); message.srvid = state->srvid; message.data.data = data; diff --git a/ctdb/tests/src/protocol_types_compat_test.c b/ctdb/tests/src/protocol_types_compat_test.c index ff1a713b0d2..03b9508c8bb 100644 --- a/ctdb/tests/src/protocol_types_compat_test.c +++ b/ctdb/tests/src/protocol_types_compat_test.c @@ -470,6 +470,60 @@ static int ctdb_rec_data_pull_old(uint8_t *buf, size_t buflen, return ret; } +struct ctdb_rec_buffer_wire { + uint32_t db_id; + uint32_t count; + uint8_t data[1]; +}; + +static size_t ctdb_rec_buffer_len_old(struct ctdb_rec_buffer *in) +{ + return offsetof(struct ctdb_rec_buffer_wire, data) + in->buflen; +} + +static void ctdb_rec_buffer_push_old(struct ctdb_rec_buffer *in, uint8_t *buf) +{ + struct ctdb_rec_buffer_wire *wire = (struct ctdb_rec_buffer_wire *)buf; + + wire->db_id = in->db_id; + wire->count = in->count; + if (in->buflen > 0) { + memcpy(wire->data, in->buf, in->buflen); + } +} + +static int ctdb_rec_buffer_pull_old(uint8_t *buf, size_t buflen, + TALLOC_CTX *mem_ctx, + struct ctdb_rec_buffer **out) +{ + struct ctdb_rec_buffer *val; + struct ctdb_rec_buffer_wire *wire = (struct ctdb_rec_buffer_wire *)buf; + size_t offset; + + if (buflen < offsetof(struct ctdb_rec_buffer_wire, data)) { + return EMSGSIZE; + } + + val = talloc(mem_ctx, struct ctdb_rec_buffer); + if (val == NULL) { + return ENOMEM; + } + + val->db_id = wire->db_id; + val->count = wire->count; + + offset = offsetof(struct ctdb_rec_buffer_wire, data); + val->buflen = buflen - offset; + val->buf = talloc_memdup(val, wire->data, val->buflen); + if (val->buf == NULL) { + talloc_free(val); + return ENOMEM; + } + + *out = val; + return 0; +} + COMPAT_TYPE3_TEST(struct ctdb_statistics, ctdb_statistics); COMPAT_TYPE3_TEST(struct ctdb_vnn_map, ctdb_vnn_map); @@ -480,6 +534,7 @@ COMPAT_TYPE3_TEST(struct ctdb_pulldb_ext, ctdb_pulldb_ext); COMPAT_TYPE1_TEST(struct ctdb_ltdb_header, ctdb_ltdb_header); COMPAT_TYPE3_TEST(struct ctdb_rec_data, ctdb_rec_data); +COMPAT_TYPE3_TEST(struct ctdb_rec_buffer, ctdb_rec_buffer); int main(int argc, char *argv[]) { @@ -495,6 +550,7 @@ int main(int argc, char *argv[]) COMPAT_TEST_FUNC(ctdb_pulldb_ext)(); COMPAT_TEST_FUNC(ctdb_ltdb_header)(); COMPAT_TEST_FUNC(ctdb_rec_data)(); + COMPAT_TEST_FUNC(ctdb_rec_buffer)(); return 0; } diff --git a/ctdb/tests/src/protocol_types_test.c b/ctdb/tests/src/protocol_types_test.c index 3e55f05bead..0309a1278d4 100644 --- a/ctdb/tests/src/protocol_types_test.c +++ b/ctdb/tests/src/protocol_types_test.c @@ -55,7 +55,7 @@ PROTOCOL_TYPE3_TEST(struct ctdb_pulldb, ctdb_pulldb); PROTOCOL_TYPE3_TEST(struct ctdb_pulldb_ext, ctdb_pulldb_ext); PROTOCOL_TYPE1_TEST(struct ctdb_ltdb_header, ctdb_ltdb_header); PROTOCOL_TYPE3_TEST(struct ctdb_rec_data, ctdb_rec_data); -DEFINE_TEST(struct ctdb_rec_buffer, ctdb_rec_buffer); +PROTOCOL_TYPE3_TEST(struct ctdb_rec_buffer, ctdb_rec_buffer); DEFINE_TEST(struct ctdb_traverse_start, ctdb_traverse_start); DEFINE_TEST(struct ctdb_traverse_all, ctdb_traverse_all); DEFINE_TEST(struct ctdb_traverse_start_ext, ctdb_traverse_start_ext); diff --git a/ctdb/tools/ctdb.c b/ctdb/tools/ctdb.c index e506162d44b..44c067aa565 100644 --- a/ctdb/tools/ctdb.c +++ b/ctdb/tools/ctdb.c @@ -4256,6 +4256,7 @@ static int control_restoredb(TALLOC_CTX *mem_ctx, struct ctdb_context *ctdb, for (i=0; i