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.

diff --git a/src/lib/atom-private.c b/src/lib/atom-private.c
index 050349a6bbbccda1b236eec29bf4d037a425f659..c5dd0fb4b6f07aabc64fcabe1d7ca7981414b59a 100644 (file)
--- 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 667796a6fdf119a38a9017c2738ac7c10a069525..bfb120e3bec4f360dc13829fc0531e77674a4bb9 100644 (file)
--- 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 077f228c4d2ea51b67473b3f425d624ccb75696e..d33629376ccb6438d4886149a826b3819b77695a 100644 (file)
--- 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);