Skip to content

Commit 86e23f9

Browse files
pavel-shirshovpavel-shirshov
authored
[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
1 parent f126258 commit 86e23f9

5 files changed

+309
-0
lines changed

‎src/lldpd/patch/0004-lldpctl-put-a-lock-around-some-commands-to-avoid-rac.patch‎

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,169 @@
1+
From 46aa45d0fa3e8879ecdca1c156cb2d91194c45e9 Mon Sep 17 00:00:00 2001
2+
From: Pavel Shirshov <[email protected]>
3+
Date: Thu, 12 Dec 2019 13:47:17 -0800
4+
Subject: [PATCH 1/1] lldpctl: put a lock around some commands to avoid race
5+
conditions
6+
7+
---
8+
src/client/client.h | 3 +++
9+
src/client/commands.c | 58 ++++++++++++++++++++++++++++++++++++++++---
10+
src/client/conf.c | 4 +--
11+
3 files changed, 60 insertions(+), 5 deletions(-)
12+
13+
diff --git a/src/client/client.h b/src/client/client.h
14+
index e3ee352..6c3e30d 100644
15+
--- a/src/client/client.h
16+
+++ b/src/client/client.h
17+
@@ -62,6 +62,8 @@ extern void add_history ();
18+
#endif
19+
#undef NEWLINE
20+
21+
+extern const char *ctlname;
22+
+
23+
/* commands.c */
24+
#define NEWLINE "<CR>"
25+
struct cmd_node;
26+
@@ -76,6 +78,7 @@ struct cmd_node *commands_new(
27+
struct cmd_env*, void *),
28+
void *);
29+
struct cmd_node* commands_privileged(struct cmd_node *);
30+
+struct cmd_node* commands_lock(struct cmd_node *);
31+
struct cmd_node* commands_hidden(struct cmd_node *);
32+
void commands_free(struct cmd_node *);
33+
const char *cmdenv_arg(struct cmd_env*);
34+
diff --git a/src/client/commands.c b/src/client/commands.c
35+
index beedbf1..58df4a7 100644
36+
--- a/src/client/commands.c
37+
+++ b/src/client/commands.c
38+
@@ -18,6 +18,9 @@
39+
#include "client.h"
40+
#include <string.h>
41+
#include <sys/queue.h>
42+
+#include <sys/types.h>
43+
+#include <sys/socket.h>
44+
+#include <sys/un.h>
45+
46+
/**
47+
* An element of the environment (a key and a value).
48+
@@ -68,6 +71,7 @@ struct cmd_node {
49+
const char *token; /**< Token to enter this cnode */
50+
const char *doc; /**< Documentation string */
51+
int privileged; /**< Privileged command? */
52+
+ int lock; /**< Lock required for execution? */
53+
int hidden; /**< Hidden command? */
54+
55+
/**
56+
@@ -113,6 +117,21 @@ commands_privileged(struct cmd_node *node)
57+
return node;
58+
}
59+
60+
+/**
61+
+ * Make a node accessible only with a lock.
62+
+ *
63+
+ * @param node node to use lock to execute
64+
+ * @return the modified node
65+
+ *
66+
+ * The node is modified. It is returned to ease chaining.
67+
+ */
68+
+struct cmd_node*
69+
+commands_lock(struct cmd_node *node)
70+
+{
71+
+ if (node) node->lock = 1;
72+
+ return node;
73+
+}
74+
+
75+
/**
76+
* Hide a node from help or completion.
77+
*
78+
@@ -344,6 +363,7 @@ _commands_execute(struct lldpctl_conn_t *conn, struct writer *w,
79+
int n, rc = 0, completion = (word != NULL);
80+
int help = 0; /* Are we asking for help? */
81+
int complete = 0; /* Are we asking for possible completions? */
82+
+ int needlock = 0; /* Do we need a lock? */
83+
struct cmd_env env = {
84+
.elements = TAILQ_HEAD_INITIALIZER(env.elements),
85+
.stack = TAILQ_HEAD_INITIALIZER(env.stack),
86+
@@ -351,6 +371,7 @@ _commands_execute(struct lldpctl_conn_t *conn, struct writer *w,
87+
.argv = argv,
88+
.argp = 0
89+
};
90+
+ static int lockfd = -1;
91+
cmdenv_push(&env, root);
92+
if (!completion)
93+
for (n = 0; n < argc; n++)
94+
@@ -388,6 +409,7 @@ _commands_execute(struct lldpctl_conn_t *conn, struct writer *w,
95+
!strcmp(candidate->token, token)) {
96+
/* Exact match */
97+
best = candidate;
98+
+ needlock = needlock || candidate->lock;
99+
break;
100+
}
101+
if (!best) best = candidate;
102+
@@ -406,6 +428,7 @@ _commands_execute(struct lldpctl_conn_t *conn, struct writer *w,
103+
if (!candidate->token &&
104+
CAN_EXECUTE(candidate)) {
105+
best = candidate;
106+
+ needlock = needlock || candidate->lock;
107+
break;
108+
}
109+
}
110+
@@ -421,9 +444,38 @@ _commands_execute(struct lldpctl_conn_t *conn, struct writer *w,
111+
112+
/* Push and execute */
113+
cmdenv_push(&env, best);
114+
- if (best->execute && best->execute(conn, w, &env, best->arg) != 1) {
115+
- rc = -1;
116+
- goto end;
117+
+ if (best->execute) {
118+
+ struct sockaddr_un su;
119+
+ if (needlock) {
120+
+ if (lockfd == -1) {
121+
+ log_debug("lldpctl", "reopen %s for locking", ctlname);
122+
+ if ((lockfd = socket(PF_UNIX, SOCK_STREAM, 0)) == -1) {
123+
+ log_warn("lldpctl", "cannot open for lock %s", ctlname);
124+
+ rc = -1;
125+
+ goto end;
126+
+ }
127+
+ su.sun_family = AF_UNIX;
128+
+ strlcpy(su.sun_path, ctlname, sizeof(su.sun_path));
129+
+ if (connect(lockfd, (struct sockaddr *)&su, sizeof(struct sockaddr_un)) == -1) {
130+
+ log_warn("lldpctl", "cannot connect to socket %s", ctlname);
131+
+ rc = -1;
132+
+ close(lockfd); lockfd = -1;
133+
+ goto end;
134+
+ }
135+
+ }
136+
+ if (lockf(lockfd, F_LOCK, 0) == -1) {
137+
+ log_warn("lldpctl", "cannot get lock on %s", ctlname);
138+
+ rc = -1;
139+
+ close(lockfd); lockfd = -1;
140+
+ goto end;
141+
+ }
142+
+ }
143+
+ rc = best->execute(conn, w, &env, best->arg) != 1 ? -1 : rc;
144+
+ if (needlock && lockf(lockfd, F_ULOCK, 0) == -1) {
145+
+ log_warn("lldpctl", "cannot unlock %s", ctlname);
146+
+ close(lockfd); lockfd = -1;
147+
+ }
148+
+ if (rc == -1) goto end;
149+
}
150+
env.argp++;
151+
}
152+
diff --git a/src/client/conf.c b/src/client/conf.c
153+
index 1a14981..ba5743f 100644
154+
--- a/src/client/conf.c
155+
+++ b/src/client/conf.c
156+
@@ -37,8 +37,8 @@ register_commands_configure(struct cmd_node *root)
157+
"unconfigure",
158+
"Unconfigure system settings",
159+
NULL, NULL, NULL);
160+
- commands_privileged(configure);
161+
- commands_privileged(unconfigure);
162+
+ commands_privileged(commands_lock(configure));
163+
+ commands_privileged(commands_lock(unconfigure));
164+
cmd_restrict_ports(configure);
165+
cmd_restrict_ports(unconfigure);
166+
167+
--
168+
2.17.1.windows.2
169+

‎src/lldpd/patch/0005-Read-all-notifications-in-lldpctl_recv.patch‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
From b8e66b52f40103fd3abea77031c4634742c31860 Mon Sep 17 00:00:00 2001
2+
From: Pavel Shirshov <[email protected]>
3+
Date: Thu, 12 Dec 2019 12:47:42 -0800
4+
Subject: [PATCH 1/1] Read all notifications in lldpctl_recv
5+
6+
---
7+
src/lib/connection.c | 4 ++--
8+
1 file changed, 2 insertions(+), 2 deletions(-)
9+
10+
diff --git a/src/lib/connection.c b/src/lib/connection.c
11+
index 591d9e9..88bbc99 100644
12+
--- a/src/lib/connection.c
13+
+++ b/src/lib/connection.c
14+
@@ -253,8 +253,8 @@ lldpctl_recv(lldpctl_conn_t *conn, const uint8_t *data, size_t length)
15+
memcpy(conn->input_buffer + conn->input_buffer_len, data, length);
16+
conn->input_buffer_len += length;
17+
18+
- /* Is it a notification? */
19+
- check_for_notification(conn);
20+
+ /* Read all notifications */
21+
+ while(!check_for_notification(conn));
22+
23+
RESET_ERROR(conn);
24+
25+
--
26+
2.17.1.windows.2
27+

‎src/lldpd/patch/0006-lib-fix-memory-leak.patch‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
From 833653dffb9be40110142af2a7cb4076a0dd24f5 Mon Sep 17 00:00:00 2001
2+
From: Pavel Shirshov <[email protected]>
3+
Date: Thu, 12 Dec 2019 12:48:47 -0800
4+
Subject: [PATCH 1/1] lib: fix memory leak
5+
6+
---
7+
src/lib/connection.c | 1 +
8+
1 file changed, 1 insertion(+)
9+
10+
diff --git a/src/lib/connection.c b/src/lib/connection.c
11+
index 88bbc99..aa88dad 100644
12+
--- a/src/lib/connection.c
13+
+++ b/src/lib/connection.c
14+
@@ -114,6 +114,7 @@ lldpctl_new_name(const char *ctlname, lldpctl_send_callback send, lldpctl_recv_c
15+
}
16+
if (!send && !recv) {
17+
if ((data = malloc(sizeof(struct lldpctl_conn_sync_t))) == NULL) {
18+
+ free(conn->ctlname);
19+
free(conn);
20+
return NULL;
21+
}
22+
--
23+
2.17.1.windows.2
24+

‎src/lldpd/patch/0007-lib-fix-memory-leak-when-handling-I-O.patch‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
From f6086575e63b5e089814ca116aa637d7588bfcd3 Mon Sep 17 00:00:00 2001
2+
From: Pavel Shirshov <[email protected]>
3+
Date: Thu, 12 Dec 2019 13:52:42 -0800
4+
Subject: [PATCH 1/1] lib: fix memory leak when handling I/O.
5+
6+
---
7+
src/lib/atom.c | 11 ++++++-----
8+
src/lib/atom.h | 9 ++++-----
9+
src/lib/atoms/port.c | 2 +-
10+
3 files changed, 11 insertions(+), 11 deletions(-)
11+
12+
diff --git a/src/lib/atom.c b/src/lib/atom.c
13+
index 955f434..f81d3bb 100644
14+
--- a/src/lib/atom.c
15+
+++ b/src/lib/atom.c
16+
@@ -322,10 +322,12 @@ _lldpctl_do_something(lldpctl_conn_t *conn,
17+
return SET_ERROR(conn, LLDPCTL_ERR_SERIALIZATION);
18+
conn->state = state_send;
19+
if (state_data)
20+
- conn->state_data = strdup(state_data);
21+
+ strlcpy(conn->state_data, state_data, sizeof(conn->state_data));
22+
+ else
23+
+ conn->state_data[0] = 0;
24+
}
25+
if (conn->state == state_send &&
26+
- (state_data == NULL || !strcmp(conn->state_data, state_data))) {
27+
+ (state_data == NULL || !strncmp(conn->state_data, state_data, sizeof(conn->state_data)))) {
28+
/* We need to send the currently built message */
29+
rc = lldpctl_send(conn);
30+
if (rc < 0)
31+
@@ -333,7 +335,7 @@ _lldpctl_do_something(lldpctl_conn_t *conn,
32+
conn->state = state_recv;
33+
}
34+
if (conn->state == state_recv &&
35+
- (state_data == NULL || !strcmp(conn->state_data, state_data))) {
36+
+ (state_data == NULL || !strncmp(conn->state_data, state_data, sizeof(conn->state_data)))) {
37+
/* We need to receive the answer */
38+
while ((rc = ctl_msg_recv_unserialized(&conn->input_buffer,
39+
&conn->input_buffer_len,
40+
@@ -347,8 +349,7 @@ _lldpctl_do_something(lldpctl_conn_t *conn,
41+
return SET_ERROR(conn, LLDPCTL_ERR_SERIALIZATION);
42+
/* rc == 0 */
43+
conn->state = CONN_STATE_IDLE;
44+
- free(conn->state_data);
45+
- conn->state_data = NULL;
46+
+ conn->state_data[0] = 0;
47+
return 0;
48+
} else
49+
return SET_ERROR(conn, LLDPCTL_ERR_INVALID_STATE);
50+
diff --git a/src/lib/atom.h b/src/lib/atom.h
51+
index 265c0a7..ab7037d 100644
52+
--- a/src/lib/atom.h
53+
+++ b/src/lib/atom.h
54+
@@ -55,11 +55,10 @@ struct lldpctl_conn_t {
55+
#define CONN_STATE_GET_DEFAULT_PORT_SEND 14
56+
#define CONN_STATE_GET_DEFAULT_PORT_RECV 15
57+
int state; /* Current state */
58+
- char *state_data; /* Data attached to the state. It is used to
59+
- * check that we are using the same data as a
60+
- * previous call until the state machine goes to
61+
- * CONN_STATE_IDLE. */
62+
-
63+
+ /* Data attached to the state. It is used to check that we are using the
64+
+ * same data as a previous call until the state machine goes to
65+
+ * CONN_STATE_IDLE. */
66+
+ char state_data[IFNAMSIZ + 64];
67+
/* Error handling */
68+
lldpctl_error_t error; /* Last error */
69+
70+
diff --git a/src/lib/atoms/port.c b/src/lib/atoms/port.c
71+
index 545155c..d902188 100644
72+
--- a/src/lib/atoms/port.c
73+
+++ b/src/lib/atoms/port.c
74+
@@ -329,7 +329,7 @@ _lldpctl_atom_set_atom_port(lldpctl_atom_t *atom, lldpctl_key_t key, lldpctl_ato
75+
struct lldpd_hardware *hardware = p->hardware;
76+
struct lldpd_port_set set = {};
77+
int rc;
78+
- char *canary;
79+
+ char *canary = NULL;
80+
81+
#ifdef ENABLE_DOT3
82+
struct _lldpctl_atom_dot3_power_t *dpow;
83+
--
84+
2.17.1.windows.2
85+

‎src/lldpd/patch/series‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,3 +2,7 @@
22
0001-return-error-when-port-does-not-exist.patch
33
0002-Let-linux-kernel-to-find-appropriate-nl_pid-automa.patch
44
0003-update-tx-interval-immediately.patch
5+
0004-lldpctl-put-a-lock-around-some-commands-to-avoid-rac.patch
6+
0005-Read-all-notifications-in-lldpctl_recv.patch
7+
0006-lib-fix-memory-leak.patch
8+
0007-lib-fix-memory-leak-when-handling-I-O.patch

0 commit comments

Comments
 (0)