From 89a8419047d46a7eb08aa1576b9c000242f7eacd Mon Sep 17 00:00:00 2001 From: Vincent Bernat Date: Sat, 30 Aug 2014 08:45:53 +0200 Subject: [PATCH] lib: stricter checks when resuming an operation in progress When an operation is in progress, we used both state management and some canary value to check if the user is resuming the right operation. The canary was just a pointer and it was easy to get the same pointer as the previous operation while not really resuming the previous operation. We turn the check into a string comparison instead. Moreover, add checks when setting configuration items too. --- src/lib/atom-private.c | 25 ++++++++++++++++++++++--- src/lib/atom.c | 8 +++++--- src/lib/private.h | 4 ++-- 3 files changed, 29 insertions(+), 8 deletions(-) diff --git a/src/lib/atom-private.c b/src/lib/atom-private.c index 050349a6..c5dd0fb4 100644 --- a/src/lib/atom-private.c +++ b/src/lib/atom-private.c @@ -401,6 +401,7 @@ _lldpctl_atom_set_str_config(lldpctl_atom_t *atom, lldpctl_key_t key, struct lldpd_config config = {}; char *iface_pattern = NULL, *mgmt_pattern = NULL; char *description = NULL; + char *canary = NULL; int rc, len; len = strlen(value) + 1; @@ -456,11 +457,16 @@ _lldpctl_atom_set_str_config(lldpctl_atom_t *atom, lldpctl_key_t key, return NULL; } + if (asprintf(&canary, "%d%s", key, value) == -1) { + SET_ERROR(atom->conn, LLDPCTL_ERR_NOMEM); + return NULL; + } rc = _lldpctl_do_something(atom->conn, CONN_STATE_SET_CONFIG_SEND, CONN_STATE_SET_CONFIG_RECV, - NULL, + canary, SET_CONFIG, &config, &MARSHAL_INFO(lldpd_config), NULL, NULL); + free(canary); if (rc == 0) return atom; return NULL; @@ -504,6 +510,7 @@ _lldpctl_atom_set_int_config(lldpctl_atom_t *atom, lldpctl_key_t key, long int value) { int rc; + char *canary = NULL; struct _lldpctl_atom_config_t *c = (struct _lldpctl_atom_config_t *)atom; struct lldpd_config config = {}; @@ -548,11 +555,16 @@ _lldpctl_atom_set_int_config(lldpctl_atom_t *atom, lldpctl_key_t key, return NULL; } + if (asprintf(&canary, "%d%ld", key, value) == -1) { + SET_ERROR(atom->conn, LLDPCTL_ERR_NOMEM); + return NULL; + } rc = _lldpctl_do_something(atom->conn, CONN_STATE_SET_CONFIG_SEND, CONN_STATE_SET_CONFIG_RECV, - NULL, + canary, SET_CONFIG, &config, &MARSHAL_INFO(lldpd_config), NULL, NULL); + free(canary); if (rc == 0) return atom; return NULL; } @@ -809,6 +821,7 @@ _lldpctl_atom_set_atom_port(lldpctl_atom_t *atom, lldpctl_key_t key, lldpctl_ato struct lldpd_hardware *hardware = p->hardware; struct lldpd_port_set set = {}; int rc; + char *canary; #ifdef ENABLE_DOT3 struct _lldpctl_atom_dot3_power_t *dpow; @@ -870,11 +883,17 @@ _lldpctl_atom_set_atom_port(lldpctl_atom_t *atom, lldpctl_key_t key, lldpctl_ato } set.ifname = hardware->h_ifname; + + if (asprintf(&canary, "%d%p%s", key, value, set.ifname) == -1) { + SET_ERROR(atom->conn, LLDPCTL_ERR_NOMEM); + return NULL; + } rc = _lldpctl_do_something(atom->conn, CONN_STATE_SET_PORT_SEND, CONN_STATE_SET_PORT_RECV, - value, + canary, SET_PORT, &set, &MARSHAL_INFO(lldpd_port_set), NULL, NULL); + free(canary); if (rc == 0) return atom; return NULL; } diff --git a/src/lib/atom.c b/src/lib/atom.c index 667796a6..bfb120e3 100644 --- a/src/lib/atom.c +++ b/src/lib/atom.c @@ -303,7 +303,7 @@ lldpctl_atom_create(lldpctl_atom_t *atom) */ int _lldpctl_do_something(lldpctl_conn_t *conn, - int state_send, int state_recv, void *state_data, + int state_send, int state_recv, const char *state_data, enum hmsg_type type, void *to_send, struct marshal_info *mi_send, void **to_recv, struct marshal_info *mi_recv) @@ -319,14 +319,16 @@ _lldpctl_do_something(lldpctl_conn_t *conn, conn->state = state_send; conn->state_data = state_data; } - if (conn->state == state_send && conn->state_data == state_data) { + if (conn->state == state_send && + (state_data == NULL || !strcmp(conn->state_data, state_data))) { /* We need to send the currently built message */ rc = lldpctl_send(conn); if (rc < 0) return SET_ERROR(conn, rc); conn->state = state_recv; } - if (conn->state == state_recv && conn->state_data == state_data) { + if (conn->state == state_recv && + (state_data == NULL || !strcmp(conn->state_data, state_data))) { /* We need to receive the answer */ while ((rc = ctl_msg_recv_unserialized(&conn->input_buffer, &conn->input_buffer_len, diff --git a/src/lib/private.h b/src/lib/private.h index 077f228c..d3362937 100644 --- a/src/lib/private.h +++ b/src/lib/private.h @@ -50,7 +50,7 @@ struct lldpctl_conn_t { #define CONN_STATE_SET_CONFIG_SEND 10 #define CONN_STATE_SET_CONFIG_RECV 11 int state; /* Current state */ - void *state_data; /* Data attached to the state. It is used to + const char *state_data; /* Data attached to the state. It is used to * check that we are using the same data as a * previous call until the state machine goes to * CONN_STATE_IDLE. */ @@ -71,7 +71,7 @@ struct lldpctl_conn_sync_t { ssize_t _lldpctl_needs(lldpctl_conn_t *lldpctl, size_t length); int _lldpctl_do_something(lldpctl_conn_t *conn, - int state_send, int state_recv, void *state_data, + int state_send, int state_recv, const char *state_data, enum hmsg_type type, void *to_send, struct marshal_info *mi_send, void **to_recv, struct marshal_info *mi_recv); -- 2.39.5