[lldpd]: Ports few fixes from lldpd master (#3889)

* 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
This commit is contained in:
pavel-shirshov 2019-12-13 13:46:48 -08:00 committed by GitHub
parent f126258d5f
commit 86e23f9ede
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 309 additions and 0 deletions

View File

@ -0,0 +1,169 @@
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

View File

@ -0,0 +1,27 @@
From b8e66b52f40103fd3abea77031c4634742c31860 Mon Sep 17 00:00:00 2001
From: Pavel Shirshov <pavelsh@microsoft.com>
Date: Thu, 12 Dec 2019 12:47:42 -0800
Subject: [PATCH 1/1] Read all notifications in lldpctl_recv
---
src/lib/connection.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/lib/connection.c b/src/lib/connection.c
index 591d9e9..88bbc99 100644
--- a/src/lib/connection.c
+++ b/src/lib/connection.c
@@ -253,8 +253,8 @@ lldpctl_recv(lldpctl_conn_t *conn, const uint8_t *data, size_t length)
memcpy(conn->input_buffer + conn->input_buffer_len, data, length);
conn->input_buffer_len += length;
- /* Is it a notification? */
- check_for_notification(conn);
+ /* Read all notifications */
+ while(!check_for_notification(conn));
RESET_ERROR(conn);
--
2.17.1.windows.2

View File

@ -0,0 +1,24 @@
From 833653dffb9be40110142af2a7cb4076a0dd24f5 Mon Sep 17 00:00:00 2001
From: Pavel Shirshov <pavelsh@microsoft.com>
Date: Thu, 12 Dec 2019 12:48:47 -0800
Subject: [PATCH 1/1] lib: fix memory leak
---
src/lib/connection.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/lib/connection.c b/src/lib/connection.c
index 88bbc99..aa88dad 100644
--- a/src/lib/connection.c
+++ b/src/lib/connection.c
@@ -114,6 +114,7 @@ lldpctl_new_name(const char *ctlname, lldpctl_send_callback send, lldpctl_recv_c
}
if (!send && !recv) {
if ((data = malloc(sizeof(struct lldpctl_conn_sync_t))) == NULL) {
+ free(conn->ctlname);
free(conn);
return NULL;
}
--
2.17.1.windows.2

View File

@ -0,0 +1,85 @@
From f6086575e63b5e089814ca116aa637d7588bfcd3 Mon Sep 17 00:00:00 2001
From: Pavel Shirshov <pavelsh@microsoft.com>
Date: Thu, 12 Dec 2019 13:52:42 -0800
Subject: [PATCH 1/1] lib: fix memory leak when handling I/O.
---
src/lib/atom.c | 11 ++++++-----
src/lib/atom.h | 9 ++++-----
src/lib/atoms/port.c | 2 +-
3 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/src/lib/atom.c b/src/lib/atom.c
index 955f434..f81d3bb 100644
--- a/src/lib/atom.c
+++ b/src/lib/atom.c
@@ -322,10 +322,12 @@ _lldpctl_do_something(lldpctl_conn_t *conn,
return SET_ERROR(conn, LLDPCTL_ERR_SERIALIZATION);
conn->state = state_send;
if (state_data)
- conn->state_data = strdup(state_data);
+ strlcpy(conn->state_data, state_data, sizeof(conn->state_data));
+ else
+ conn->state_data[0] = 0;
}
if (conn->state == state_send &&
- (state_data == NULL || !strcmp(conn->state_data, state_data))) {
+ (state_data == NULL || !strncmp(conn->state_data, state_data, sizeof(conn->state_data)))) {
/* We need to send the currently built message */
rc = lldpctl_send(conn);
if (rc < 0)
@@ -333,7 +335,7 @@ _lldpctl_do_something(lldpctl_conn_t *conn,
conn->state = state_recv;
}
if (conn->state == state_recv &&
- (state_data == NULL || !strcmp(conn->state_data, state_data))) {
+ (state_data == NULL || !strncmp(conn->state_data, state_data, sizeof(conn->state_data)))) {
/* We need to receive the answer */
while ((rc = ctl_msg_recv_unserialized(&conn->input_buffer,
&conn->input_buffer_len,
@@ -347,8 +349,7 @@ _lldpctl_do_something(lldpctl_conn_t *conn,
return SET_ERROR(conn, LLDPCTL_ERR_SERIALIZATION);
/* rc == 0 */
conn->state = CONN_STATE_IDLE;
- free(conn->state_data);
- conn->state_data = NULL;
+ conn->state_data[0] = 0;
return 0;
} else
return SET_ERROR(conn, LLDPCTL_ERR_INVALID_STATE);
diff --git a/src/lib/atom.h b/src/lib/atom.h
index 265c0a7..ab7037d 100644
--- a/src/lib/atom.h
+++ b/src/lib/atom.h
@@ -55,11 +55,10 @@ struct lldpctl_conn_t {
#define CONN_STATE_GET_DEFAULT_PORT_SEND 14
#define CONN_STATE_GET_DEFAULT_PORT_RECV 15
int state; /* Current state */
- 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. */
-
+ /* 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. */
+ char state_data[IFNAMSIZ + 64];
/* Error handling */
lldpctl_error_t error; /* Last error */
diff --git a/src/lib/atoms/port.c b/src/lib/atoms/port.c
index 545155c..d902188 100644
--- a/src/lib/atoms/port.c
+++ b/src/lib/atoms/port.c
@@ -329,7 +329,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;
+ char *canary = NULL;
#ifdef ENABLE_DOT3
struct _lldpctl_atom_dot3_power_t *dpow;
--
2.17.1.windows.2

View File

@ -2,3 +2,7 @@
0001-return-error-when-port-does-not-exist.patch
0002-Let-linux-kernel-to-find-appropriate-nl_pid-automa.patch
0003-update-tx-interval-immediately.patch
0004-lldpctl-put-a-lock-around-some-commands-to-avoid-rac.patch
0005-Read-all-notifications-in-lldpctl_recv.patch
0006-lib-fix-memory-leak.patch
0007-lib-fix-memory-leak-when-handling-I-O.patch