[lxc-devel] [lxc/master] bugfixes
brauner on Github
lxc-bot at linuxcontainers.org
Tue Mar 10 20:48:58 UTC 2020
A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 364 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20200310/6299af65/attachment.bin>
-------------- next part --------------
From b41ec4d2ceae5ad62b58456f0bae1254fb819306 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Tue, 10 Mar 2020 17:52:35 +0100
Subject: [PATCH 1/3] share_ns: improve error handling
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
src/tests/Makefile.am | 4 ++-
src/tests/share_ns.c | 53 ++++++++++++++++++++++++++++------------
src/tests/state_server.c | 4 +--
3 files changed, 42 insertions(+), 19 deletions(-)
diff --git a/src/tests/Makefile.am b/src/tests/Makefile.am
index 2a2ba163c4..d05021a054 100644
--- a/src/tests/Makefile.am
+++ b/src/tests/Makefile.am
@@ -41,7 +41,9 @@ lxc_test_shortlived_SOURCES = shortlived.c
lxc_test_shutdowntest_SOURCES = shutdowntest.c
lxc_test_snapshot_SOURCES = snapshot.c
lxc_test_startone_SOURCES = startone.c
-lxc_test_state_server_SOURCES = state_server.c lxctest.h
+lxc_test_state_server_SOURCES = state_server.c \
+ lxctest.h \
+ ../lxc/compiler.h
lxc_test_utils_SOURCES = lxc-test-utils.c lxctest.h
AM_CFLAGS=-DLXCROOTFSMOUNT=\"$(LXCROOTFSMOUNT)\" \
diff --git a/src/tests/share_ns.c b/src/tests/share_ns.c
index ea812d2cd5..305f5cca4d 100644
--- a/src/tests/share_ns.c
+++ b/src/tests/share_ns.c
@@ -16,6 +16,7 @@
* 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
*/
+#define _GNU_SOURCE
#include <alloca.h>
#include <errno.h>
#include <pthread.h>
@@ -32,22 +33,24 @@
#include "lxctest.h"
#include "../lxc/compiler.h"
+#define TEST_DEFAULT_BUF_SIZE 256
+
struct thread_args {
int thread_id;
bool success;
pid_t init_pid;
- char inherited_ipc_ns[4096];
- char inherited_net_ns[4096];
+ char inherited_ipc_ns[TEST_DEFAULT_BUF_SIZE];
+ char inherited_net_ns[TEST_DEFAULT_BUF_SIZE];
};
-__noreturn void *ns_sharing_wrapper(void *data)
+__noreturn static void *ns_sharing_wrapper(void *data)
{
int init_pid;
ssize_t ret;
char name[100];
char owning_ns_init_pid[100];
- char proc_ns_path[256];
- char ns_buf[256];
+ char proc_ns_path[TEST_DEFAULT_BUF_SIZE];
+ char ns_buf[TEST_DEFAULT_BUF_SIZE];
struct lxc_container *c;
struct thread_args *args = data;
@@ -75,6 +78,8 @@ __noreturn void *ns_sharing_wrapper(void *data)
goto out;
}
+ c->clear_config(c);
+
if (!c->load_config(c, NULL)) {
lxc_error("Failed to load config for container \"%s\"\n", name);
goto out;
@@ -169,6 +174,8 @@ __noreturn void *ns_sharing_wrapper(void *data)
if (!c->destroy(c))
lxc_error("Failed to destroy container \"%s\"\n", name);
+ lxc_container_put(c);
+
out_pthread_exit:
pthread_exit(NULL);
}
@@ -176,13 +183,13 @@ __noreturn void *ns_sharing_wrapper(void *data)
int main(int argc, char *argv[])
{
struct thread_args *args = NULL;
+ pthread_t *threads = NULL;
size_t nthreads = 10;
int i, init_pid, j;
- char proc_ns_path[4096];
- char ipc_ns_buf[4096];
- char net_ns_buf[4096];
+ char proc_ns_path[TEST_DEFAULT_BUF_SIZE];
+ char ipc_ns_buf[TEST_DEFAULT_BUF_SIZE];
+ char net_ns_buf[TEST_DEFAULT_BUF_SIZE];
pthread_attr_t attr;
- pthread_t threads[10];
struct lxc_container *c;
int ret = EXIT_FAILURE;
@@ -196,17 +203,17 @@ int main(int argc, char *argv[])
if (c->is_defined(c)) {
lxc_error("%s\n", "Container \"owning-ns\" is defined");
- goto on_error_put;
+ goto on_error_stop;
}
if (!c->createl(c, "busybox", NULL, NULL, 0, NULL)) {
lxc_error("%s\n", "Failed to create busybox container \"owning-ns\"");
- goto on_error_put;
+ goto on_error_stop;
}
if (!c->is_defined(c)) {
lxc_error("%s\n", "Container \"owning-ns\" is not defined");
- goto on_error_put;
+ goto on_error_stop;
}
c->clear_config(c);
@@ -269,15 +276,26 @@ int main(int argc, char *argv[])
goto on_error_stop;
}
+ threads = malloc(sizeof(pthread_t) * nthreads);
+ if (!threads) {
+ lxc_error("%s\n", "Failed to allocate memory");
+ goto on_error_stop;
+ }
+
for (j = 0; j < 10; j++) {
+ bool had_error = false;
+
lxc_debug("Starting namespace sharing test iteration %d\n", j);
for (i = 0; i < nthreads; i++) {
+ memset(&args[i], 0, sizeof(struct thread_args));
+ memset(&threads[i], 0, sizeof(pthread_t));
+
args[i].thread_id = i;
args[i].success = false;
args[i].init_pid = init_pid;
- memcpy(args[i].inherited_ipc_ns, ipc_ns_buf, sizeof(args[i].inherited_ipc_ns));
- memcpy(args[i].inherited_net_ns, net_ns_buf, sizeof(args[i].inherited_net_ns));
+ snprintf(args[i].inherited_ipc_ns, sizeof(args[i].inherited_ipc_ns), "%s", ipc_ns_buf);
+ snprintf(args[i].inherited_net_ns, sizeof(args[i].inherited_net_ns), "%s", net_ns_buf);
ret = pthread_create(&threads[i], &attr, ns_sharing_wrapper, (void *)&args[i]);
if (ret != 0)
@@ -291,15 +309,19 @@ int main(int argc, char *argv[])
if (!args[i].success) {
lxc_error("ns sharing thread %d failed\n", args[i].thread_id);
- goto on_error_stop;
+ had_error = true;
}
}
+
+ if (had_error)
+ goto on_error_stop;
}
ret = EXIT_SUCCESS;
on_error_stop:
free(args);
+ free(threads);
pthread_attr_destroy(&attr);
if (c->is_running(c) && !c->stop(c))
@@ -308,7 +330,6 @@ int main(int argc, char *argv[])
if (!c->destroy(c))
lxc_error("%s\n", "Failed to destroy container \"owning-ns\"");
-on_error_put:
lxc_container_put(c);
if (ret == EXIT_SUCCESS)
lxc_debug("%s\n", "All state namespace sharing tests passed");
diff --git a/src/tests/state_server.c b/src/tests/state_server.c
index bb64a87cba..3002888ed4 100644
--- a/src/tests/state_server.c
+++ b/src/tests/state_server.c
@@ -30,6 +30,7 @@
#include "lxc/lxccontainer.h"
#include "lxctest.h"
+#include "../lxc/compiler.h"
struct thread_args {
int thread_id;
@@ -38,7 +39,7 @@ struct thread_args {
struct lxc_container *c;
};
-static void *state_wrapper(void *data)
+__noreturn static void *state_wrapper(void *data)
{
struct thread_args *args = data;
@@ -50,7 +51,6 @@ static void *state_wrapper(void *data)
args->thread_id, args->timeout, args->success ? "SUCCESS" : "FAILED");
pthread_exit(NULL);
- return NULL;
}
int main(int argc, char *argv[])
From 565eb353e0b1a6b1cdbd882bb39eed9acb084dd4 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Tue, 10 Mar 2020 21:35:25 +0100
Subject: [PATCH 2/3] commands: switch to pid_t to send around pid
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
src/lxc/commands.c | 10 ++++------
src/lxc/macro.h | 4 ++--
2 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/src/lxc/commands.c b/src/lxc/commands.c
index 76b592a195..4ba8229874 100644
--- a/src/lxc/commands.c
+++ b/src/lxc/commands.c
@@ -329,13 +329,13 @@ int lxc_try_cmd(const char *name, const char *lxcpath)
pid_t lxc_cmd_get_init_pid(const char *name, const char *lxcpath)
{
int ret, stopped;
- intmax_t pid = -1;
+ pid_t pid = -1;
struct lxc_cmd_rr cmd = {
.req = {
.cmd = LXC_CMD_GET_INIT_PID
},
.rsp = {
- .data = INTMAX_TO_PTR(pid)
+ .data = PID_TO_PTR(pid)
}
};
@@ -343,7 +343,7 @@ pid_t lxc_cmd_get_init_pid(const char *name, const char *lxcpath)
if (ret < 0)
return -1;
- pid = PTR_TO_INTMAX(cmd.rsp.data);
+ pid = PTR_TO_PID(cmd.rsp.data);
if (pid < 0)
return -1;
@@ -357,10 +357,8 @@ static int lxc_cmd_get_init_pid_callback(int fd, struct lxc_cmd_req *req,
struct lxc_handler *handler,
struct lxc_epoll_descr *descr)
{
- intmax_t pid = handler->pid;
-
struct lxc_cmd_rsp rsp = {
- .data = INTMAX_TO_PTR(pid)
+ .data = PID_TO_PTR(handler->pid)
};
return lxc_cmd_rsp_send(fd, &rsp);
diff --git a/src/lxc/macro.h b/src/lxc/macro.h
index 68bd6ca844..612fb11ea4 100644
--- a/src/lxc/macro.h
+++ b/src/lxc/macro.h
@@ -414,8 +414,8 @@ enum {
#define PTR_TO_INT(p) ((int)((intptr_t)(p)))
#define INT_TO_PTR(u) ((void *)((intptr_t)(u)))
-#define PTR_TO_INTMAX(p) ((intmax_t)((intptr_t)(p)))
-#define INTMAX_TO_PTR(u) ((void *)((intptr_t)(u)))
+#define PTR_TO_PID(p) ((pid_t)((intptr_t)(p)))
+#define PID_TO_PTR(u) ((void *)((intptr_t)(u)))
#define PTR_TO_UINT64(p) ((uint64_t)((intptr_t)(p)))
From 39e2a438af3dafb5214245dd570447693957108e Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Tue, 10 Mar 2020 21:46:25 +0100
Subject: [PATCH 3/3] commands: improve state client cleanup
Improves: ebbca8529732 ("commands_utils: fix socket leak when adding state client")
Cc: Matthias Hardt <matthias.hardt at gmail.com>
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
src/lxc/commands.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/src/lxc/commands.c b/src/lxc/commands.c
index 4ba8229874..cf3b1ed223 100644
--- a/src/lxc/commands.c
+++ b/src/lxc/commands.c
@@ -873,10 +873,6 @@ static int lxc_cmd_add_state_client_callback(__owns int fd, struct lxc_cmd_req *
if (ret < 0)
goto reap_client_fd;
- /* close fd if state is already achieved to avoid leakage */
- if (rsp.ret != MAX_STATE)
- close(fd);
-
return 0;
reap_client_fd:
@@ -1344,12 +1340,20 @@ static void lxc_cmd_fd_cleanup(int fd, struct lxc_handler *handler,
close(client->clientfd);
free(cur->elem);
free(cur);
- /* No need to walk the whole list. If we found the state
+ /*
+ * No need to walk the whole list. If we found the state
* client fd there can't be a second one.
*/
break;
}
- break;
+
+ /*
+ * We didn't add the state client to the list. Either because
+ * we failed to allocate memory (unlikely) or because the state
+ * was already reached by the time we were ready to add it. So
+ * fallthrough and clean it up.
+ */
+ __fallthrough;
default:
close(fd);
}
More information about the lxc-devel
mailing list