lldpctl: put a lock around some commands to avoid race conditions

Fix #343

diff --git a/src/client/client.h b/src/client/client.h
index 8da3e3fed800d0ba139a035a3dfae75d6820904f..84e6e6c6d5795e51680ffe16eda1194cddf194e1 100644 (file)
--- a/src/client/client.h
+++ b/src/client/client.h
@@ -62,6 +62,8 @@ extern void add_history ();
 #endif
 #undef NEWLINE
 
+extern const char *ctlname;
+
 /* commands.c */
 #define NEWLINE "<CR>"
 struct cmd_node;
@@ -76,6 +78,7 @@ struct cmd_node *commands_new(
            struct cmd_env*, void *),
        void *);
 struct cmd_node* commands_privileged(struct cmd_node *);
+struct cmd_node* commands_lock(struct cmd_node *);
 struct cmd_node* commands_hidden(struct cmd_node *);
 void commands_free(struct cmd_node *);
 const char *cmdenv_arg(struct cmd_env*);

diff --git a/src/client/commands.c b/src/client/commands.c
index b96b77f8dae011f21919f52250f6aa98412e4c54..905652ee073997895d26f6ce35af3d5831450a26 100644 (file)
--- a/src/client/commands.c
+++ b/src/client/commands.c
@@ -18,6 +18,9 @@
 #include "client.h"
 #include <string.h>
 #include <sys/queue.h>
+#include <sys/types.h>
+#include <sys/socket.h>
+#include <sys/un.h>
 
 /**
  * An element of the environment (a key and a value).
@@ -68,6 +71,7 @@ struct cmd_node {
        const char *token;      /**< Token to enter this cnode */
        const char *doc;        /**< Documentation string */
        int privileged;         /**< Privileged command? */
+       int lock;               /**< Lock required for execution? */
        int hidden;             /**< Hidden command? */
 
        /**
@@ -113,6 +117,21 @@ commands_privileged(struct cmd_node *node)
        return node;
 }
 
+/**
+ * Make a node accessible only with a lock.
+ *
+ * @param node node to use lock to execute
+ * @return the modified node
+ *
+ * The node is modified. It is returned to ease chaining.
+ */
+struct cmd_node*
+commands_lock(struct cmd_node *node)
+{
+       if (node) node->lock = 1;
+       return node;
+}
+
 /**
  * Hide a node from help or completion.
  *
@@ -344,6 +363,7 @@ _commands_execute(struct lldpctl_conn_t *conn, struct writer *w,
        int n, rc = 0, completion = (word != NULL);
        int help = 0;           /* Are we asking for help? */
        int complete = 0;       /* Are we asking for possible completions? */
+       int needlock = 0;       /* Do we need a lock? */
        struct cmd_env env = {
                .elements = TAILQ_HEAD_INITIALIZER(env.elements),
                .stack = TAILQ_HEAD_INITIALIZER(env.stack),
@@ -388,6 +408,7 @@ _commands_execute(struct lldpctl_conn_t *conn, struct writer *w,
                                    !strcmp(candidate->token, token)) {
                                        /* Exact match */
                                        best = candidate;
+                                       needlock = needlock || candidate->lock;
                                        break;
                                }
                                if (!best) best = candidate;
@@ -406,6 +427,7 @@ _commands_execute(struct lldpctl_conn_t *conn, struct writer *w,
                                if (!candidate->token &&
                                    CAN_EXECUTE(candidate)) {
                                        best = candidate;
+                                       needlock = needlock || candidate->lock;
                                        break;
                                }
                        }
@@ -421,9 +443,37 @@ _commands_execute(struct lldpctl_conn_t *conn, struct writer *w,
 
                /* Push and execute */
                cmdenv_push(&env, best);
-               if (best->execute && best->execute(conn, w, &env, best->arg) != 1) {
-                       rc = -1;
-                       goto end;
+               if (best->execute) {
+                       int lockfd;
+                       struct sockaddr_un su;
+                       if (needlock) {
+                               log_debug("lldpctl", "getting lock for %s", ctlname);
+                               if ((lockfd = socket(PF_UNIX, SOCK_STREAM, 0)) == -1) {
+                                       log_warn("lldpctl", "cannot open for lock %s", ctlname);
+                                       rc = -1;
+                                       goto end;
+                               }
+                               su.sun_family = AF_UNIX;
+                               strlcpy(su.sun_path, ctlname, sizeof(su.sun_path));
+                               if (connect(lockfd, (struct sockaddr *)&su, sizeof(struct sockaddr_un)) == -1) {
+                                       log_warn("lldpctl", "cannot connect to socket %s", ctlname);
+                                       rc = -1;
+                                       close(lockfd);
+                                       goto end;
+                               }
+                               if (lockf(lockfd, F_LOCK, 0) == -1) {
+                                       log_warn("lldpctl", "cannot get lock on %s", ctlname);
+                                       rc = -1;
+                                       close(lockfd);
+                                       goto end;
+                               }
+                       }
+                       if (best->execute(conn, w, &env, best->arg) != 1) {
+                               rc = -1;
+                               if (needlock) close(lockfd);
+                               goto end;
+                       }
+                       close(lockfd);
                }
                env.argp++;
        }

diff --git a/src/client/conf.c b/src/client/conf.c
index 1a14981d251513b281bb8e3ed96287e489b7c325..ba5743f73cc2e59358b78f1cdb50808013d241f3 100644 (file)
--- a/src/client/conf.c
+++ b/src/client/conf.c
@@ -37,8 +37,8 @@ register_commands_configure(struct cmd_node *root)
                "unconfigure",
                "Unconfigure system settings",
                NULL, NULL, NULL);
-       commands_privileged(configure);
-       commands_privileged(unconfigure);
+       commands_privileged(commands_lock(configure));
+       commands_privileged(commands_lock(unconfigure));
        cmd_restrict_ports(configure);
        cmd_restrict_ports(unconfigure);