* lldpctl: put a lock around some commands to avoid race conditions * Read all notifications in lldpctl_recv * lib: fix memory leak * lib: fix memory leak when handling I/O * Update series
170 lines
5.1 KiB
Diff
170 lines
5.1 KiB
Diff
From 46aa45d0fa3e8879ecdca1c156cb2d91194c45e9 Mon Sep 17 00:00:00 2001
|
|
From: Pavel Shirshov <pavelsh@microsoft.com>
|
|
Date: Thu, 12 Dec 2019 13:47:17 -0800
|
|
Subject: [PATCH 1/1] lldpctl: put a lock around some commands to avoid race
|
|
conditions
|
|
|
|
---
|
|
src/client/client.h | 3 +++
|
|
src/client/commands.c | 58 ++++++++++++++++++++++++++++++++++++++++---
|
|
src/client/conf.c | 4 +--
|
|
3 files changed, 60 insertions(+), 5 deletions(-)
|
|
|
|
diff --git a/src/client/client.h b/src/client/client.h
|
|
index e3ee352..6c3e30d 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 beedbf1..58df4a7 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),
|
|
@@ -351,6 +371,7 @@ _commands_execute(struct lldpctl_conn_t *conn, struct writer *w,
|
|
.argv = argv,
|
|
.argp = 0
|
|
};
|
|
+ static int lockfd = -1;
|
|
cmdenv_push(&env, root);
|
|
if (!completion)
|
|
for (n = 0; n < argc; n++)
|
|
@@ -388,6 +409,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 +428,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 +444,38 @@ _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) {
|
|
+ struct sockaddr_un su;
|
|
+ if (needlock) {
|
|
+ if (lockfd == -1) {
|
|
+ log_debug("lldpctl", "reopen %s for locking", 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); lockfd = -1;
|
|
+ goto end;
|
|
+ }
|
|
+ }
|
|
+ if (lockf(lockfd, F_LOCK, 0) == -1) {
|
|
+ log_warn("lldpctl", "cannot get lock on %s", ctlname);
|
|
+ rc = -1;
|
|
+ close(lockfd); lockfd = -1;
|
|
+ goto end;
|
|
+ }
|
|
+ }
|
|
+ rc = best->execute(conn, w, &env, best->arg) != 1 ? -1 : rc;
|
|
+ if (needlock && lockf(lockfd, F_ULOCK, 0) == -1) {
|
|
+ log_warn("lldpctl", "cannot unlock %s", ctlname);
|
|
+ close(lockfd); lockfd = -1;
|
|
+ }
|
|
+ if (rc == -1) goto end;
|
|
}
|
|
env.argp++;
|
|
}
|
|
diff --git a/src/client/conf.c b/src/client/conf.c
|
|
index 1a14981..ba5743f 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);
|
|
|
|
--
|
|
2.17.1.windows.2
|
|
|