From: Vincent Bernat <vincent@bernat.ch>
Date: Tue, 10 Sep 2019 07:55:54 +0000 (+0200)
Subject: lldpctl: put a lock around some commands to avoid race conditions
X-Git-Tag: 1.0.5~41
X-Git-Url: http://git.ipfire.org/?p=thirdparty%2Flldpd.git;a=commitdiff_plain;h=0a46b330f02b955e6c9ab4faf739a1586a8f0589

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 8da3e3fe..84e6e6c6 100644
--- 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 b96b77f8..905652ee 100644
--- 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 1a14981d..ba5743f7 100644
--- 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);