[lxc-devel] [lxc/master] log: add logging tools for commands; lxc-usernsexec: cleanup and bugfixes
brauner on Github
lxc-bot at linuxcontainers.org
Thu Aug 16 11:05:12 UTC 2018
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/20180816/111c51b4/attachment.bin>
-------------- next part --------------
From bb9623e70582bc734244acd91edee9d0b43fd6be Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Thu, 16 Aug 2018 13:01:54 +0200
Subject: [PATCH 1/3] log: add CMD_SYSERROR()
Add a thread-safe and uniform way to retrieve errno values in programs that are
shipped as part of LXC but are not expected to have access to the logging
system.
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
src/lxc/log.h | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/lxc/log.h b/src/lxc/log.h
index 734fbe177..d75d87d43 100644
--- a/src/lxc/log.h
+++ b/src/lxc/log.h
@@ -429,6 +429,12 @@ ATTR_UNUSED static inline void LXC_##LEVEL(struct lxc_log_locinfo* locinfo, \
ERROR("%s - " format, ptr, ##__VA_ARGS__); \
} while (0)
+#define CMD_SYSERROR(format, ...) \
+ do { \
+ lxc_log_strerror_r; \
+ fprintf(stderr, "%s - " format, ptr, ##__VA_ARGS__); \
+ } while (0)
+
extern int lxc_log_fd;
extern int lxc_log_syslog(int facility);
From 20029ee35ab294365b320a4adf047804dd367b5c Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Thu, 16 Aug 2018 13:03:21 +0200
Subject: [PATCH 2/3] log: add CMD_SYSINFO()
Add a thread-safe and uniform way to retrieve errno values in programs that are
shipped as part of LXC but are not expected to have access to the logging
system.
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
src/lxc/log.h | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/lxc/log.h b/src/lxc/log.h
index d75d87d43..4654fd918 100644
--- a/src/lxc/log.h
+++ b/src/lxc/log.h
@@ -435,6 +435,12 @@ ATTR_UNUSED static inline void LXC_##LEVEL(struct lxc_log_locinfo* locinfo, \
fprintf(stderr, "%s - " format, ptr, ##__VA_ARGS__); \
} while (0)
+#define CMD_SYSINFO(format, ...) \
+ do { \
+ lxc_log_strerror_r; \
+ printf("%s - " format, ptr, ##__VA_ARGS__); \
+ } while (0)
+
extern int lxc_log_fd;
extern int lxc_log_syslog(int facility);
From b48245387d2e9fa24539be4567047a52fdac701e Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Thu, 16 Aug 2018 13:03:36 +0200
Subject: [PATCH 3/3] lxc-usernsexec: cleanup and bugfixes
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
src/lxc/Makefile.am | 7 +-
src/lxc/cmd/lxc_usernsexec.c | 277 +++++++++++++++++++++--------------
2 files changed, 171 insertions(+), 113 deletions(-)
diff --git a/src/lxc/Makefile.am b/src/lxc/Makefile.am
index d5b6e8180..14db7cb47 100644
--- a/src/lxc/Makefile.am
+++ b/src/lxc/Makefile.am
@@ -325,7 +325,12 @@ lxc_user_nic_SOURCES = cmd/lxc_user_nic.c \
namespace.c namespace.h \
network.c network.h \
parse.c parse.h
-lxc_usernsexec_SOURCES = cmd/lxc_usernsexec.c
+lxc_usernsexec_SOURCES = cmd/lxc_usernsexec.c \
+ conf.c conf.h \
+ list.h \
+ log.c log.h \
+ namespace.c namespace.h \
+ utils.c utils.h
endif
diff --git a/src/lxc/cmd/lxc_usernsexec.c b/src/lxc/cmd/lxc_usernsexec.c
index 227b79212..63cffbddb 100644
--- a/src/lxc/cmd/lxc_usernsexec.c
+++ b/src/lxc/cmd/lxc_usernsexec.c
@@ -42,6 +42,8 @@
#include <grp.h>
#include "conf.h"
+#include "list.h"
+#include "log.h"
#include "namespace.h"
#include "utils.h"
@@ -77,21 +79,22 @@ static void usage(const char *name)
static void opentty(const char *tty, int which)
{
- int fd, flags;
+ int fd, flags, ret;
if (tty[0] == '\0')
return;
fd = open(tty, O_RDWR | O_NONBLOCK);
if (fd < 0) {
- printf("WARN: could not reopen tty: %s\n", strerror(errno));
+ CMD_SYSERROR("Failed to open tty");
return;
}
flags = fcntl(fd, F_GETFL);
flags &= ~O_NONBLOCK;
- if (fcntl(fd, F_SETFL, flags) < 0) {
- printf("WARN: could not set fd flags: %s\n", strerror(errno));
+ ret = fcntl(fd, F_SETFL, flags);
+ if (ret < 0) {
+ CMD_SYSERROR("Failed to remove O_NONBLOCK from file descriptor %d", fd);
close(fd);
return;
}
@@ -106,51 +109,60 @@ static void opentty(const char *tty, int which)
static int do_child(void *vargv)
{
+ int ret;
char **argv = (char **)vargv;
/* Assume we want to become root */
- if (setgid(0) < 0) {
- perror("setgid");
+ ret = setgid(0);
+ if (ret < 0) {
+ CMD_SYSERROR("Failed to set gid to");
return -1;
}
- if (setuid(0) < 0) {
- perror("setuid");
+
+ ret = setuid(0);
+ if (ret < 0) {
+ CMD_SYSERROR("Failed to set uid to 0");
return -1;
}
- if (setgroups(0, NULL) < 0) {
- perror("setgroups");
+
+ ret = setgroups(0, NULL);
+ if (ret < 0) {
+ CMD_SYSERROR("Failed to clear supplementary groups");
return -1;
}
- if (unshare(CLONE_NEWNS) < 0) {
- perror("unshare CLONE_NEWNS");
+
+ ret = unshare(CLONE_NEWNS);
+ if (ret < 0) {
+ CMD_SYSERROR("Failed to unshare mount namespace");
return -1;
}
+
if (detect_shared_rootfs()) {
- if (mount(NULL, "/", NULL, MS_SLAVE|MS_REC, NULL)) {
- printf("Failed to make / rslave\n");
+ ret = mount(NULL, "/", NULL, MS_SLAVE | MS_REC, NULL);
+ if (ret < 0) {
+ printf("Failed to make \"/\" rslave\n");
return -1;
}
}
+
execvp(argv[0], argv);
- perror("execvpe");
+ CMD_SYSERROR("Failed to execute \"%s\"", argv[0]);
return -1;
}
static struct lxc_list active_map;
/*
- * given a string like "b:0:100000:10", map both uids and gids
- * 0-10 to 100000 to 100010
+ * Given a string like "b:0:100000:10", map both uids and gids 0-10 to 100000
+ * to 100010
*/
static int parse_map(char *map)
{
+ long host_id, i, ns_id, range, ret;
+ char which;
struct id_map *newmap;
- struct lxc_list *tmp = NULL;
- int ret;
- int i;
char types[2] = {'u', 'g'};
- char which;
- long host_id, ns_id, range;
+ struct lxc_list *tmp = NULL;
if (!map)
return -1;
@@ -193,45 +205,49 @@ static int parse_map(char *map)
}
/*
- * This is called if the user did not pass any uid ranges in
- * through -m flags. It's called once to get the default uid
- * map, and once for the default gid map.
- * Go through /etc/subuids and /etc/subgids to find this user's
- * allowed map. We only use the first one for each of uid and
- * gid, because otherwise we're not sure which entries the user
- * wanted.
+ * This is called if the user did not pass any uid ranges in through -m flags.
+ * It's called once to get the default uid map, and once for the default gid
+ * map.
+ * Go through /etc/subuids and /etc/subgids to find this user's allowed map. We
+ * only use the first one for each of uid and gid, because otherwise we're not
+ * sure which entries the user wanted.
*/
static int read_default_map(char *fnam, int which, char *username)
{
+ char *p1, *p2;
FILE *fin;
- char *line = NULL;
- size_t sz = 0;
struct id_map *newmap;
+ size_t sz = 0;
+ char *line = NULL;
struct lxc_list *tmp = NULL;
- char *p1, *p2;
fin = fopen(fnam, "r");
if (!fin)
return -1;
+
while (getline(&line, &sz, fin) != -1) {
if (sz <= strlen(username) ||
strncmp(line, username, strlen(username)) != 0 ||
line[strlen(username)] != ':')
continue;
+
p1 = strchr(line, ':');
if (!p1)
continue;
- p2 = strchr(p1+1, ':');
+
+ p2 = strchr(p1 + 1, ':');
if (!p2)
continue;
+
newmap = malloc(sizeof(*newmap));
- if (!newmap) {
+ if (!newmap) {
fclose(fin);
free(line);
return -1;
}
- newmap->hostid = atol(p1+1);
- newmap->range = atol(p2+1);
+
+ newmap->hostid = atol(p1 + 1);
+ newmap->range = atol(p2 + 1);
newmap->nsid = 0;
newmap->idtype = which;
@@ -250,16 +266,17 @@ static int read_default_map(char *fnam, int which, char *username)
free(line);
fclose(fin);
+
return 0;
}
static int find_default_map(void)
{
+ size_t bufsize;
+ char *buf;
struct passwd pwent;
+ int ret = -1;
struct passwd *pwentp = NULL;
- char *buf;
- size_t bufsize;
- int ret;
bufsize = sysconf(_SC_GETPW_R_SIZE_MAX);
if (bufsize == -1)
@@ -272,60 +289,58 @@ static int find_default_map(void)
ret = getpwuid_r(getuid(), &pwent, buf, bufsize, &pwentp);
if (!pwentp) {
if (ret == 0)
- printf("WARN: could not find matched password record\n");
+ CMD_SYSERROR("Failed to find matched password record");
- printf("Failed to get password record - %u\n", getuid());
- free(buf);
- return -1;
+ CMD_SYSERROR("Failed to get password record for uid %d", getuid());
+ ret = -1;
+ goto out;
}
- if (read_default_map(subuidfile, ID_TYPE_UID, pwent.pw_name) < 0) {
- free(buf);
- return -1;
- }
+ ret = read_default_map(subuidfile, ID_TYPE_UID, pwent.pw_name);
+ if (ret < 0)
+ goto out;
- if (read_default_map(subgidfile, ID_TYPE_GID, pwent.pw_name) < 0) {
- free(buf);
- return -1;
- }
+ ret = read_default_map(subgidfile, ID_TYPE_GID, pwent.pw_name);
+ if (ret < 0)
+ goto out;
+
+ ret = 0;
+out:
free(buf);
- return 0;
+
+ return ret;
}
int main(int argc, char *argv[])
{
- int c;
+ int c, pid, ret, status;
+ char buf[1];
+ int pipe_fds1[2], /* child tells parent it has unshared */
+ pipe_fds2[2]; /* parent tells child it is mapped and may proceed */
unsigned long flags = CLONE_NEWUSER | CLONE_NEWNS;
- char ttyname0[256], ttyname1[256], ttyname2[256];
- int status;
- int ret;
- int pid;
+ char ttyname0[256] = {0}, ttyname1[256] = {0}, ttyname2[256] = {0};
char *default_args[] = {"/bin/sh", NULL};
- char buf[1];
- int pipe_fds1[2], /* child tells parent it has unshared */
- pipe_fds2[2]; /* parent tells child it is mapped and may proceed */
lxc_log_fd = STDERR_FILENO;
- memset(ttyname0, '\0', sizeof(ttyname0));
- memset(ttyname1, '\0', sizeof(ttyname1));
- memset(ttyname2, '\0', sizeof(ttyname2));
- if (isatty(0)) {
+ if (isatty(STDIN_FILENO)) {
ret = readlink("/proc/self/fd/0", ttyname0, sizeof(ttyname0));
if (ret < 0) {
- perror("unable to open stdin.");
- exit(EXIT_FAILURE);
+ CMD_SYSERROR("Failed to open stdin");
+ _exit(EXIT_FAILURE);
}
+
ret = readlink("/proc/self/fd/1", ttyname1, sizeof(ttyname1));
if (ret < 0) {
- printf("Warning: unable to open stdout, continuing.\n");
- memset(ttyname1, '\0', sizeof(ttyname1));
+ CMD_SYSINFO("Failed to open stdout. Continuing");
+ ttyname1[0] = '\0';
}
+
ret = readlink("/proc/self/fd/2", ttyname2, sizeof(ttyname2));
if (ret < 0) {
- printf("Warning: unable to open stderr, continuing.\n");
- memset(ttyname2, '\0', sizeof(ttyname2));
+ CMD_SYSINFO("Failed to open stderr. Continuing");
+ ttyname2[0] = '\0';
}
}
@@ -334,24 +349,26 @@ int main(int argc, char *argv[])
while ((c = getopt(argc, argv, "m:h")) != EOF) {
switch (c) {
case 'm':
- if (parse_map(optarg)) {
+ ret = parse_map(optarg);
+ if (ret < 0) {
usage(argv[0]);
- exit(EXIT_FAILURE);
+ _exit(EXIT_FAILURE);
}
break;
case 'h':
usage(argv[0]);
- exit(EXIT_SUCCESS);
+ _exit(EXIT_SUCCESS);
default:
usage(argv[0]);
- exit(EXIT_FAILURE);
+ _exit(EXIT_FAILURE);
}
};
if (lxc_list_empty(&active_map)) {
- if (find_default_map()) {
- fprintf(stderr, "You have no allocated subuids or subgids\n");
- exit(EXIT_FAILURE);
+ ret = find_default_map();
+ if (ret < 0) {
+ CMD_SYSERROR("Failed to find subuid or subgid allocation");
+ _exit(EXIT_FAILURE);
}
}
@@ -360,68 +377,104 @@ int main(int argc, char *argv[])
if (argc < 1)
argv = default_args;
- if (pipe2(pipe_fds1, O_CLOEXEC) < 0 || pipe2(pipe_fds2, O_CLOEXEC) < 0) {
- perror("pipe");
- exit(EXIT_FAILURE);
+ ret = pipe2(pipe_fds1, O_CLOEXEC);
+ if (ret < 0) {
+ CMD_SYSERROR("Failed to open new pipe");
+ _exit(EXIT_FAILURE);
}
+
+ ret = pipe2(pipe_fds2, O_CLOEXEC);
+ if (ret < 0) {
+ CMD_SYSERROR("Failed to open new pipe");
+ close(pipe_fds1[0]);
+ close(pipe_fds1[1]);
+ _exit(EXIT_FAILURE);
+ }
+
pid = fork();
- if (pid == 0) { /* Child. */
+ if (pid < 0) {
close(pipe_fds1[0]);
+ close(pipe_fds1[1]);
+ close(pipe_fds2[0]);
close(pipe_fds2[1]);
- opentty(ttyname0, 0);
- opentty(ttyname1, 1);
- opentty(ttyname2, 2);
+ _exit(EXIT_FAILURE);
+ }
+
+ if (pid == 0) {
+ close(pipe_fds1[0]);
+ close(pipe_fds2[1]);
+
+ opentty(ttyname0, STDIN_FILENO);
+ opentty(ttyname1, STDOUT_FILENO);
+ opentty(ttyname2, STDERR_FILENO);
ret = unshare(flags);
if (ret < 0) {
- fprintf(stderr,
- "Failed to unshare mount and user namespace\n");
- return 1;
+ CMD_SYSERROR("Failed to unshare mount and user namespace");
+ close(pipe_fds1[1]);
+ close(pipe_fds2[0]);
+ _exit(EXIT_FAILURE);
}
+
buf[0] = '1';
- if (lxc_write_nointr(pipe_fds1[1], buf, 1) < 1) {
- perror("write pipe");
- exit(EXIT_FAILURE);
- }
- if (lxc_read_nointr(pipe_fds2[0], buf, 1) < 1) {
- perror("read pipe");
- exit(EXIT_FAILURE);
+ ret = lxc_write_nointr(pipe_fds1[1], buf, 1);
+ if (ret != 1) {
+ CMD_SYSERROR("Failed to write to pipe file descriptor %d",
+ pipe_fds1[1]);
+ close(pipe_fds1[1]);
+ close(pipe_fds2[0]);
+ _exit(EXIT_FAILURE);
}
- if (buf[0] != '1') {
- fprintf(stderr, "parent had an error, child exiting\n");
- exit(EXIT_FAILURE);
+
+ ret = lxc_read_nointr(pipe_fds2[0], buf, 1);
+ if (ret != 1) {
+ CMD_SYSERROR("Failed to read from pipe file descriptor %d",
+ pipe_fds2[0]);
+ close(pipe_fds1[1]);
+ close(pipe_fds2[0]);
+ _exit(EXIT_FAILURE);
}
close(pipe_fds1[1]);
close(pipe_fds2[0]);
- return do_child((void *)argv);
+
+ if (buf[0] != '1') {
+ fprintf(stderr, "Received unexpected value from parent process\n");
+ _exit(EXIT_FAILURE);
+ }
+
+ ret = do_child((void *)argv);
+ if (ret < 0)
+ _exit(EXIT_FAILURE);
+
+ _exit(EXIT_SUCCESS);
}
close(pipe_fds1[1]);
close(pipe_fds2[0]);
+
ret = lxc_read_nointr(pipe_fds1[0], buf, 1);
- if (ret < 0) {
- perror("read pipe");
- exit(EXIT_FAILURE);
- } else if (ret == 0) {
- fprintf(stderr, "Failed to read from pipe\n");
- exit(EXIT_FAILURE);
+ if (ret <= 0) {
+ CMD_SYSERROR("Failed to read from pipe file descriptor %d", pipe_fds1[0]);
}
buf[0] = '1';
- if (lxc_map_ids(&active_map, pid))
- fprintf(stderr, "error mapping child\n");
+ ret = lxc_map_ids(&active_map, pid);
+ if (ret < 0)
+ fprintf(stderr, "Failed to write id mapping for child process\n");
- if (lxc_write_nointr(pipe_fds2[1], buf, 1) < 0) {
- perror("write to pipe");
- exit(EXIT_FAILURE);
+ ret = lxc_write_nointr(pipe_fds2[1], buf, 1);
+ if (ret < 0) {
+ CMD_SYSERROR("Failed to write to pipe file descriptor %d", pipe_fds2[1]);
+ _exit(EXIT_FAILURE);
}
+
ret = waitpid(pid, &status, __WALL);
if (ret < 0) {
- printf("waitpid() returns %d, errno %d\n", ret, errno);
- exit(EXIT_FAILURE);
+ CMD_SYSERROR("Failed to wait on child process");
+ _exit(EXIT_FAILURE);
}
- exit(WEXITSTATUS(status));
+ _exit(WEXITSTATUS(status));
}
More information about the lxc-devel
mailing list