The following pull request was submitted through Github. It can be accessed and reviewed at: https://github.com/lxc/lxc/pull/2551
This e-mail was sent by the LXC bot, direct replies will not reach the author unless they happen to be subscribed to this list. === Description (from pull-request) === In response to issue #2471 this PR cleans up function return values for code error paths resulting from failed stdlib function calls. - Return `-EINVAL` instead of -1 for invalid command line options. - Return `-EINVAL` instead of -1 for invalid function args. - Save and return `-errno` for failed stdlib function calls. For stdlib functions that return a single error code just return the error code directly, comment if not obvious. (This PR only does `src/lxc/cmd/`, if this is correct I can do the rest of `src/lxc`. Assuming that @2xsec is not working on this anymore). thanks, Tobin. Signed-off-by: Tobin C. Harding
From ea04d16d00fb57db20a5e749f5fb5b4894c681cb Mon Sep 17 00:00:00 2001 From: 2xsec <[email protected]> Date: Sun, 19 Aug 2018 12:45:54 +0900 Subject: [PATCH 1/4] cmd: lxc-user-nic: change log macro & cleanups Signed-off-by: 2xsec <[email protected]> --- src/lxc/Makefile.am | 1 + src/lxc/cmd/lxc_user_nic.c | 255 ++++++++++++++++++------------------- 2 files changed, 126 insertions(+), 130 deletions(-) diff --git a/src/lxc/Makefile.am b/src/lxc/Makefile.am index 14db7cb47..56a45b4ac 100644 --- a/src/lxc/Makefile.am +++ b/src/lxc/Makefile.am @@ -322,6 +322,7 @@ if ENABLE_COMMANDS init_lxc_SOURCES = cmd/lxc_init.c lxc_monitord_SOURCES = cmd/lxc_monitord.c lxc_user_nic_SOURCES = cmd/lxc_user_nic.c \ + log.c log.h \ namespace.c namespace.h \ network.c network.h \ parse.c parse.h diff --git a/src/lxc/cmd/lxc_user_nic.c b/src/lxc/cmd/lxc_user_nic.c index 09b158245..153940b86 100644 --- a/src/lxc/cmd/lxc_user_nic.c +++ b/src/lxc/cmd/lxc_user_nic.c @@ -46,6 +46,7 @@ #include <sys/types.h> #include "config.h" +#include "log.h" #include "namespace.h" #include "network.h" #include "parse.h" @@ -72,9 +73,9 @@ static void usage(char *me, bool fail) fprintf(stderr, "{nicname} is the name to use inside the container\n"); if (fail) - exit(EXIT_FAILURE); + _exit(EXIT_FAILURE); - exit(EXIT_SUCCESS); + _exit(EXIT_SUCCESS); } static int open_and_lock(char *path) @@ -84,8 +85,7 @@ static int open_and_lock(char *path) fd = open(path, O_RDWR | O_CREAT, S_IWUSR | S_IRUSR); if (fd < 0) { - usernic_error("Failed to open \"%s\": %s\n", path, - strerror(errno)); + CMD_SYSERROR("Failed to open \"%s\"\n", path); return -1; } @@ -96,8 +96,7 @@ static int open_and_lock(char *path) ret = fcntl(fd, F_SETLKW, &lk); if (ret < 0) { - usernic_error("Failed to lock \"%s\": %s\n", path, - strerror(errno)); + CMD_SYSERROR("Failed to lock \"%s\"\n", path); close(fd); return -1; } @@ -127,7 +126,7 @@ static char *get_username(void) if (ret == 0) usernic_error("%s", "Could not find matched password record\n"); - usernic_error("Failed to get username: %s(%u)\n", strerror(errno), getuid()); + CMD_SYSERROR("Failed to get username: %u\n", getuid()); free(buf); return NULL; } @@ -164,35 +163,29 @@ static char **get_groupnames(void) ngroups = getgroups(0, NULL); if (ngroups < 0) { - usernic_error("Failed to get number of groups the user " - "belongs to: %s\n", strerror(errno)); + CMD_SYSERROR("Failed to get number of groups the user belongs to\n"); return NULL; - } - if (ngroups == 0) + } else if (ngroups == 0) { return NULL; + } group_ids = malloc(sizeof(gid_t) * ngroups); if (!group_ids) { - usernic_error("Failed to allocate memory while getting groups " - "the user belongs to: %s\n", - strerror(errno)); + CMD_SYSERROR("Failed to allocate memory while getting groups the user belongs to\n"); return NULL; } ret = getgroups(ngroups, group_ids); if (ret < 0) { free(group_ids); - usernic_error("Failed to get process groups: %s\n", - strerror(errno)); + CMD_SYSERROR("Failed to get process groups\n"); return NULL; } groupnames = malloc(sizeof(char *) * (ngroups + 1)); if (!groupnames) { free(group_ids); - usernic_error("Failed to allocate memory while getting group " - "names: %s\n", - strerror(errno)); + CMD_SYSERROR("Failed to allocate memory while getting group names\n"); return NULL; } @@ -206,9 +199,7 @@ static char **get_groupnames(void) if (!buf) { free(group_ids); free_groupnames(groupnames); - usernic_error("Failed to allocate memory while getting group " - "names: %s\n", - strerror(errno)); + CMD_SYSERROR("Failed to allocate memory while getting group names\n"); return NULL; } @@ -218,8 +209,7 @@ static char **get_groupnames(void) if (ret == 0) usernic_error("%s", "Could not find matched group record\n"); - usernic_error("Failed to get group name: %s(%u)\n", - strerror(errno), group_ids[i]); + CMD_SYSERROR("Failed to get group name: %u\n", group_ids[i]); free(buf); free(group_ids); free_groupnames(groupnames); @@ -228,8 +218,7 @@ static char **get_groupnames(void) groupnames[i] = strdup(grent.gr_name); if (!groupnames[i]) { - usernic_error("Failed to copy group name \"%s\"", - grent.gr_name); + usernic_error("Failed to copy group name \"%s\"", grent.gr_name); free(buf); free(group_ids); free_groupnames(groupnames); @@ -250,6 +239,7 @@ static bool name_is_in_groupnames(char *name, char **groupnames) return true; groupnames++; } + return false; } @@ -272,8 +262,7 @@ static struct alloted_s *append_alloted(struct alloted_s **head, char *name, al = malloc(sizeof(struct alloted_s)); if (!al) { - usernic_error("Failed to allocate memory: %s\n", - strerror(errno)); + CMD_SYSERROR("Failed to allocate memory\n"); return NULL; } @@ -331,15 +320,13 @@ static int get_alloted(char *me, char *intype, char *link, char name[100], type[100], br[100]; char **groups; FILE *fin; - int count = 0; size_t len = 0; char *line = NULL; fin = fopen(LXC_USERNIC_CONF, "r"); if (!fin) { - usernic_error("Failed to open \"%s\": %s\n", LXC_USERNIC_CONF, - strerror(errno)); + CMD_SYSERROR("Failed to open \"%s\"\n", LXC_USERNIC_CONF); return -1; } @@ -381,6 +368,7 @@ static int get_alloted(char *me, char *intype, char *link, count = 0; break; } + count += n; } @@ -396,6 +384,7 @@ static char *get_eol(char *s, char *e) { while ((s < e) && *s && (*s != '\n')) s++; + return s; } @@ -403,6 +392,7 @@ static char *get_eow(char *s, char *e) { while ((s < e) && *s && !isblank(*s) && (*s != '\n')) s++; + return s; } @@ -505,8 +495,8 @@ static int instantiate_veth(char *veth1, char *veth2) ret = lxc_veth_create(veth1, veth2); if (ret < 0) { - usernic_error("Failed to create %s-%s : %s.\n", veth1, veth2, - strerror(-ret)); + errno = -ret; + CMD_SYSERROR("Failed to create %s-%s\n", veth1, veth2); return -1; } @@ -515,9 +505,10 @@ static int instantiate_veth(char *veth1, char *veth2) * mac address of a container. */ ret = setup_private_host_hw_addr(veth1); - if (ret < 0) - usernic_error("Failed to change mac address of host interface " - "%s : %s\n", veth1, strerror(-ret)); + if (ret < 0) { + errno = -ret; + CMD_SYSERROR("Failed to change mac address of host interface %s\n", veth1); + } return netdev_set_flag(veth1, IFF_UP); } @@ -529,6 +520,7 @@ static int get_mtu(char *name) idx = if_nametoindex(name); if (idx < 0) return -1; + return netdev_get_mtu(idx); } @@ -548,6 +540,7 @@ static int create_nic(char *nic, char *br, int pid, char **cnic) usernic_error("%s\n", "Could not create nic name"); return -1; } + /* create the nics */ ret = instantiate_veth(veth1buf, veth2buf); if (ret < 0) { @@ -562,14 +555,14 @@ static int create_nic(char *nic, char *br, int pid, char **cnic) ret = lxc_netdev_set_mtu(veth1buf, mtu); if (ret < 0) { usernic_error("Failed to set mtu to %d on %s\n", - mtu, veth1buf); + mtu, veth1buf); goto out_del; } ret = lxc_netdev_set_mtu(veth2buf, mtu); if (ret < 0) { usernic_error("Failed to set mtu to %d on %s\n", - mtu, veth2buf); + mtu, veth2buf); goto out_del; } } @@ -586,7 +579,7 @@ static int create_nic(char *nic, char *br, int pid, char **cnic) ret = lxc_netdev_move_by_name(veth2buf, pid, NULL); if (ret < 0) { usernic_error("Error moving %s to network namespace of %d\n", - veth2buf, pid); + veth2buf, pid); goto out_del; } @@ -621,7 +614,7 @@ static bool cull_entries(int fd, char *name, char *net_type, char *net_link, ret = fstat(fd, &sb); if (ret < 0) { - usernic_error("Failed to fstat: %s\n", strerror(errno)); + CMD_SYSERROR("Failed to fstat\n"); return false; } @@ -630,8 +623,7 @@ static bool cull_entries(int fd, char *name, char *net_type, char *net_link, buf = lxc_strmmap(NULL, sb.st_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0); if (buf == MAP_FAILED) { - usernic_error("Failed to establish shared memory mapping: %s\n", - strerror(errno)); + CMD_SYSERROR("Failed to establish shared memory mapping\n"); return false; } @@ -664,6 +656,7 @@ static bool cull_entries(int fd, char *name, char *net_type, char *net_link, } buf_start = buf; + for (i = 0; i < n; i++) { if (!entry_lines[i].keep) continue; @@ -673,13 +666,13 @@ static bool cull_entries(int fd, char *name, char *net_type, char *net_link, *buf_start = '\n'; buf_start++; } + free(entry_lines); ret = ftruncate(fd, buf_start - buf); lxc_strmunmap(buf, sb.st_size); if (ret < 0) - usernic_error("Failed to set new file size: %s\n", - strerror(errno)); + CMD_SYSERROR("Failed to set new file size\n"); return true; } @@ -695,6 +688,7 @@ static int count_entries(char *buf, off_t len, char *name, char *net_type, char &owner, &(bool){true}, &(bool){true}))) { if (owner) count++; + buf = get_eol(buf, buf_end) + 1; if (buf >= buf_end) break; @@ -726,7 +720,7 @@ static char *get_nic_if_avail(int fd, struct alloted_s *names, int pid, ret = fstat(fd, &sb); if (ret < 0) { - usernic_error("Failed to fstat: %s\n", strerror(errno)); + CMD_SYSERROR("Failed to fstat\n"); return NULL; } @@ -734,12 +728,12 @@ static char *get_nic_if_avail(int fd, struct alloted_s *names, int pid, buf = lxc_strmmap(NULL, sb.st_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0); if (buf == MAP_FAILED) { - usernic_error("Failed to establish shared memory " - "mapping: %s\n", strerror(errno)); + CMD_SYSERROR("Failed to establish shared memory mapping\n"); return NULL; } owner = NULL; + for (n = names; n != NULL; n = n->next) { count = count_entries(buf, sb.st_size, n->name, intype, br); if (count >= n->allowed) @@ -790,7 +784,7 @@ static char *get_nic_if_avail(int fd, struct alloted_s *names, int pid, newline = malloc(slen + 1); if (!newline) { free(newline); - usernic_error("Failed allocate memory: %s\n", strerror(errno)); + CMD_SYSERROR("Failed allocate memory\n"); return NULL; } @@ -798,6 +792,7 @@ static char *get_nic_if_avail(int fd, struct alloted_s *names, int pid, if (ret < 0 || (size_t)ret >= (slen + 1)) { if (lxc_netdev_delete_by_name(nicname) != 0) usernic_error("Error unlinking %s\n", nicname); + free(newline); return NULL; } @@ -807,15 +802,16 @@ static char *get_nic_if_avail(int fd, struct alloted_s *names, int pid, */ ret = ftruncate(fd, sb.st_size + slen); if (ret < 0) - usernic_error("Failed to truncate file: %s\n", strerror(errno)); + CMD_SYSERROR("Failed to truncate file\n"); buf = lxc_strmmap(NULL, sb.st_size + slen, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0); if (buf == MAP_FAILED) { - usernic_error("Failed to establish shared memory mapping: %s\n", - strerror(errno)); + CMD_SYSERROR("Failed to establish shared memory mapping\n"); + if (lxc_netdev_delete_by_name(nicname) != 0) usernic_error("Error unlinking %s\n", nicname); + free(newline); return NULL; } @@ -845,6 +841,7 @@ static bool create_db_dir(char *fnam) again: while (*p && *p != '/') p++; + if (!*p) return true; @@ -852,11 +849,11 @@ static bool create_db_dir(char *fnam) ret = mkdir(fnam, 0755); if (ret < 0 && errno != EEXIST) { - usernic_error("Failed to create %s: %s\n", fnam, - strerror(errno)); + CMD_SYSERROR("Failed to create %s\n", fnam); *p = '/'; return false; } + *(p++) = '/'; goto again; @@ -873,6 +870,7 @@ static char *lxc_secure_rename_in_ns(int pid, char *oldname, char *newname, int fd = -1, ifindex = -1, ofd = -1; pid_self = lxc_raw_getpid(); + ofd = lxc_preserve_ns(pid_self, "net"); if (ofd < 0) { usernic_error("Failed opening network namespace path for %d", pid_self); @@ -887,9 +885,7 @@ static char *lxc_secure_rename_in_ns(int pid, char *oldname, char *newname, ret = getresuid(&ruid, &euid, &suid); if (ret < 0) { - usernic_error("Failed to retrieve real, effective, and saved " - "user IDs: %s\n", - strerror(errno)); + CMD_SYSERROR("Failed to retrieve real, effective, and saved user IDs\n"); goto do_partial_cleanup; } @@ -897,18 +893,16 @@ static char *lxc_secure_rename_in_ns(int pid, char *oldname, char *newname, close(fd); fd = -1; if (ret < 0) { - usernic_error("Failed to setns() to the network namespace of " - "the container with PID %d: %s\n", - pid, strerror(errno)); + CMD_SYSERROR("Failed to setns() to the network namespace of " + "the container with PID %d\n", pid); goto do_partial_cleanup; } ret = setresuid(ruid, ruid, 0); if (ret < 0) { - usernic_error("Failed to drop privilege by setting effective " - "user id and real user id to %d, and saved user " - "ID to 0: %s\n", - ruid, strerror(errno)); + CMD_SYSERROR("Failed to drop privilege by setting effective " + "user id and real user id to %d, and saved user " + "ID to 0\n", ruid); /* It's ok to jump to do_full_cleanup here since setresuid() * will succeed when trying to set real, effective, and saved to * values they currently have. @@ -919,7 +913,7 @@ static char *lxc_secure_rename_in_ns(int pid, char *oldname, char *newname, /* Check if old interface exists. */ ifindex = if_nametoindex(oldname); if (!ifindex) { - usernic_error("Failed to get netdev index: %s\n", strerror(errno)); + CMD_SYSERROR("Failed to get netdev index\n"); goto do_full_cleanup; } @@ -936,13 +930,13 @@ static char *lxc_secure_rename_in_ns(int pid, char *oldname, char *newname, name = NULL; if (ret < 0) { usernic_error("Error %d renaming netdev %s to %s in container\n", - ret, oldname, newname ? newname : "eth%d"); + ret, oldname, newname ? newname : "eth%d"); goto do_full_cleanup; } /* Retrieve new name for interface. */ if (!if_indextoname(ifindex, ifname)) { - usernic_error("Failed to get new netdev name: %s\n", strerror(errno)); + CMD_SYSERROR("Failed to get new netdev name\n"); goto do_full_cleanup; } @@ -954,18 +948,17 @@ static char *lxc_secure_rename_in_ns(int pid, char *oldname, char *newname, do_full_cleanup: ret = setresuid(ruid, euid, suid); if (ret < 0) { - usernic_error("Failed to restore privilege by setting " - "effective user id to %d, real user id to %d, " - "and saved user ID to %d: %s\n", ruid, euid, suid, - strerror(errno)); + CMD_SYSERROR("Failed to restore privilege by setting " + "effective user id to %d, real user id to %d, " + "and saved user ID to %d\n", ruid, euid, suid); string_ret = NULL; } ret = setns(ofd, CLONE_NEWNET); if (ret < 0) { - usernic_error("Failed to setns() to original network namespace " - "of PID %d: %s\n", ofd, strerror(errno)); + CMD_SYSERROR("Failed to setns() to original network namespace " + "of PID %d\n", ofd); string_ret = NULL; } @@ -994,18 +987,15 @@ static bool may_access_netns(int pid) ret = getresuid(&ruid, &euid, &suid); if (ret < 0) { - usernic_error("Failed to retrieve real, effective, and saved " - "user IDs: %s\n", - strerror(errno)); + CMD_SYSERROR("Failed to retrieve real, effective, and saved user IDs\n"); return false; } ret = setresuid(ruid, ruid, euid); if (ret < 0) { - usernic_error("Failed to drop privilege by setting effective " - "user id and real user id to %d, and saved user " - "ID to %d: %s\n", - ruid, euid, strerror(errno)); + CMD_SYSERROR("Failed to drop privilege by setting effective " + "user id and real user id to %d, and saved user " + "ID to %d\n", ruid, euid); return false; } @@ -1017,14 +1007,13 @@ static bool may_access_netns(int pid) may_access = true; if (ret < 0) { may_access = false; - usernic_error("Uid %d may not access %s: %s\n", (int)ruid, s, strerror(errno)); + CMD_SYSERROR("Uid %d may not access %s\n", (int)ruid, s); } ret = setresuid(ruid, euid, suid); if (ret < 0) { - usernic_error("Failed to restore user id to %d, real user id " - "to %d, and saved user ID to %d: %s\n", - ruid, euid, suid, strerror(errno)); + CMD_SYSERROR("Failed to restore user id to %d, real user id " + "to %d, and saved user ID to %d\n", ruid, euid, suid); may_access = false; } @@ -1053,6 +1042,7 @@ static bool is_privileged_over_netns(int netns_fd) int ofd = -1; pid_self = lxc_raw_getpid(); + ofd = lxc_preserve_ns(pid_self, "net"); if (ofd < 0) { usernic_error("Failed opening network namespace path for %d", pid_self); @@ -1061,25 +1051,21 @@ static bool is_privileged_over_netns(int netns_fd) ret = getresuid(&ruid, &euid, &suid); if (ret < 0) { - usernic_error("Failed to retrieve real, effective, and saved " - "user IDs: %s\n", - strerror(errno)); + CMD_SYSERROR("Failed to retrieve real, effective, and saved user IDs\n"); goto do_partial_cleanup; } ret = setns(netns_fd, CLONE_NEWNET); if (ret < 0) { - usernic_error("Failed to setns() to network namespace %s\n", - strerror(errno)); + CMD_SYSERROR("Failed to setns() to network namespace\n"); goto do_partial_cleanup; } ret = setresuid(ruid, ruid, 0); if (ret < 0) { - usernic_error("Failed to drop privilege by setting effective " - "user id and real user id to %d, and saved user " - "ID to 0: %s\n", - ruid, strerror(errno)); + CMD_SYSERROR("Failed to drop privilege by setting effective " + "user id and real user id to %d, and saved user " + "ID to 0\n", ruid); /* It's ok to jump to do_full_cleanup here since setresuid() * will succeed when trying to set real, effective, and saved to * values they currently have. @@ -1099,24 +1085,20 @@ static bool is_privileged_over_netns(int netns_fd) do_full_cleanup: ret = setresuid(ruid, euid, suid); if (ret < 0) { - usernic_error("Failed to restore privilege by setting " - "effective user id to %d, real user id to %d, " - "and saved user ID to %d: %s\n", ruid, euid, suid, - strerror(errno)); - + CMD_SYSERROR("Failed to restore privilege by setting " + "effective user id to %d, real user id to %d, " + "and saved user ID to %d\n", ruid, euid, suid); bret = false; } ret = setns(ofd, CLONE_NEWNET); if (ret < 0) { - usernic_error("Failed to setns() to original network namespace " - "of PID %d: %s\n", ofd, strerror(errno)); - + CMD_SYSERROR("Failed to setns() to original network namespace " + "of PID %d\n", ofd); bret = false; } do_partial_cleanup: - close(ofd); return bret; } @@ -1132,10 +1114,11 @@ int main(int argc, char *argv[]) if (argc < 7 || argc > 8) { usage(argv[0], true); - exit(EXIT_FAILURE); + _exit(EXIT_FAILURE); } memset(&args, 0, sizeof(struct user_nic_args)); + args.cmd = argv[1]; args.lxc_path = argv[2]; args.lxc_name = argv[3]; @@ -1151,33 +1134,33 @@ int main(int argc, char *argv[]) request = LXC_USERNIC_DELETE; } else { usage(argv[0], true); - exit(EXIT_FAILURE); + _exit(EXIT_FAILURE); } /* Set a sane env, because we are setuid-root. */ ret = clearenv(); if (ret) { usernic_error("%s", "Failed to clear environment\n"); - exit(EXIT_FAILURE); + _exit(EXIT_FAILURE); } ret = setenv("PATH", "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", 1); if (ret < 0) { usernic_error("%s", "Failed to set PATH, exiting\n"); - exit(EXIT_FAILURE); + _exit(EXIT_FAILURE); } me = get_username(); if (!me) { usernic_error("%s", "Failed to get username\n"); - exit(EXIT_FAILURE); + _exit(EXIT_FAILURE); } if (request == LXC_USERNIC_CREATE) { ret = lxc_safe_int(args.pid, &pid); if (ret < 0) { usernic_error("Could not read pid: %s\n", args.pid); - exit(EXIT_FAILURE); + _exit(EXIT_FAILURE); } } else if (request == LXC_USERNIC_DELETE) { char opath[LXC_PROC_PID_FD_LEN]; @@ -1191,60 +1174,66 @@ int main(int argc, char *argv[]) netns_fd = open(args.pid, O_PATH | O_CLOEXEC); if (netns_fd < 0) { usernic_error("Failed to open \"%s\"\n", args.pid); - exit(EXIT_FAILURE); + _exit(EXIT_FAILURE); } if (!fhas_fs_type(netns_fd, NSFS_MAGIC)) { usernic_error("Path \"%s\" does not refer to a network namespace path\n", args.pid); close(netns_fd); - exit(EXIT_FAILURE); + _exit(EXIT_FAILURE); } ret = snprintf(opath, sizeof(opath), "/proc/self/fd/%d", netns_fd); if (ret < 0 || (size_t)ret >= sizeof(opath)) { close(netns_fd); - exit(EXIT_FAILURE); + _exit(EXIT_FAILURE); } /* Now get an fd that we can use in setns() calls. */ ret = open(opath, O_RDONLY | O_CLOEXEC); if (ret < 0) { - usernic_error("Failed to open \"%s\": %s\n", args.pid, strerror(errno)); + CMD_SYSERROR("Failed to open \"%s\"\n", args.pid); close(netns_fd); - exit(EXIT_FAILURE); + _exit(EXIT_FAILURE); } + close(netns_fd); netns_fd = ret; } if (!create_db_dir(LXC_USERNIC_DB)) { usernic_error("%s", "Failed to create directory for db file\n"); + if (netns_fd >= 0) close(netns_fd); - exit(EXIT_FAILURE); + + _exit(EXIT_FAILURE); } fd = open_and_lock(LXC_USERNIC_DB); if (fd < 0) { usernic_error("Failed to lock %s\n", LXC_USERNIC_DB); + if (netns_fd >= 0) close(netns_fd); - exit(EXIT_FAILURE); + + _exit(EXIT_FAILURE); } if (request == LXC_USERNIC_CREATE) { if (!may_access_netns(pid)) { usernic_error("User %s may not modify netns for pid %d\n", me, pid); - exit(EXIT_FAILURE); + _exit(EXIT_FAILURE); } } else if (request == LXC_USERNIC_DELETE) { bool has_priv; + has_priv = is_privileged_over_netns(netns_fd); close(netns_fd); if (!has_priv) { usernic_error("%s", "Process is not privileged over " - "network namespace\n"); - exit(EXIT_FAILURE); + "network namespace\n"); + _exit(EXIT_FAILURE); } } @@ -1258,10 +1247,10 @@ int main(int argc, char *argv[]) if (!is_ovs_bridge(args.link)) { usernic_error("%s", "Deletion of non ovs type network " - "devices not implemented\n"); + "devices not implemented\n"); close(fd); free_alloted(&alloted); - exit(EXIT_FAILURE); + _exit(EXIT_FAILURE); } /* Check whether the network device we are supposed to delete @@ -1278,29 +1267,31 @@ int main(int argc, char *argv[]) if (!found_nicname) { usernic_error("Caller is not allowed to delete network " - "device \"%s\"\n", args.veth_name); - exit(EXIT_FAILURE); + "device \"%s\"\n", args.veth_name); + _exit(EXIT_FAILURE); } ret = lxc_ovs_delete_port(args.link, args.veth_name); if (ret < 0) { usernic_error("Failed to remove port \"%s\" from " - "openvswitch bridge \"%s\"", - args.veth_name, args.link); - exit(EXIT_FAILURE); + "openvswitch bridge \"%s\"", + args.veth_name, args.link); + _exit(EXIT_FAILURE); } - exit(EXIT_SUCCESS); + _exit(EXIT_SUCCESS); } + if (n > 0) nicname = get_nic_if_avail(fd, alloted, pid, args.type, args.link, n, &cnic); close(fd); free_alloted(&alloted); + if (!nicname) { usernic_error("%s", "Quota reached\n"); - exit(EXIT_FAILURE); + _exit(EXIT_FAILURE); } /* Now rename the link. */ @@ -1308,19 +1299,21 @@ int main(int argc, char *argv[]) &container_veth_ifidx); if (!newname) { usernic_error("%s", "Failed to rename the link\n"); + ret = lxc_netdev_delete_by_name(cnic); if (ret < 0) usernic_error("Failed to delete \"%s\"\n", cnic); + free(nicname); - exit(EXIT_FAILURE); + _exit(EXIT_FAILURE); } host_veth_ifidx = if_nametoindex(nicname); if (!host_veth_ifidx) { free(newname); free(nicname); - usernic_error("Failed to get netdev index: %s\n", strerror(errno)); - exit(EXIT_FAILURE); + CMD_SYSERROR("Failed to get netdev index\n"); + _exit(EXIT_FAILURE); } /* Write names of veth pairs and their ifindeces to stout: @@ -1330,5 +1323,7 @@ int main(int argc, char *argv[]) host_veth_ifidx); free(newname); free(nicname); - exit(EXIT_SUCCESS); + + fflush(stdout); + _exit(EXIT_SUCCESS); } From 6e95cfd95aa552e47d00e50373d39e1dc89085c6 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" <[email protected]> Date: Mon, 20 Aug 2018 15:42:59 +1000 Subject: [PATCH 2/4] cmd: Return -EINVAL for incorrect options During args parsing we fail and return -1 for a particular combination of options. This is invalid args so we should return EINVAL. Return EINVAL instead of -1 for invalid options combination. Signed-off-by: Tobin C. Harding <[email protected]> --- src/lxc/cmd/lxc_init.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lxc/cmd/lxc_init.c b/src/lxc/cmd/lxc_init.c index 75900a9d3..6ead7eb46 100644 --- a/src/lxc/cmd/lxc_init.c +++ b/src/lxc/cmd/lxc_init.c @@ -556,7 +556,7 @@ static int arguments_parse(struct arguments *args, int argc, if (!args->name) { if (!args->quiet) fprintf(stderr, "lxc-init: missing container name, use --name option\n"); - return -1; + return -EINVAL; } return 0; From fdedb9760a59bd5186f2a0f61802042964a324e9 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" <[email protected]> Date: Mon, 20 Aug 2018 16:03:37 +1000 Subject: [PATCH 3/4] cmd: Return -EINVAL for invalid arguments Currently we are returning -1 from a function for invalid arguments, the error code EINVAL has this meaning. We should return negative EINVAL instead of -1. Return -EINVAL for invalid function arguments. Signed-off-by: Tobin C. Harding <[email protected]> --- src/lxc/cmd/lxc_usernsexec.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lxc/cmd/lxc_usernsexec.c b/src/lxc/cmd/lxc_usernsexec.c index 5ff23400d..90d203547 100644 --- a/src/lxc/cmd/lxc_usernsexec.c +++ b/src/lxc/cmd/lxc_usernsexec.c @@ -166,14 +166,14 @@ static int parse_map(char *map) struct lxc_list *tmp = NULL; if (!map) - return -1; + return -EINVAL; ret = sscanf(map, "%c:%ld:%ld:%ld", &which, &ns_id, &host_id, &range); if (ret != 4) - return -1; + return -EINVAL; if (which != 'b' && which != 'u' && which != 'g') - return -1; + return -EINVAL; for (i = 0; i < 2; i++) { if (which != types[i] && which != 'b') From 4bca4c940523533e854e01db4e2bf553f0c9e320 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" <[email protected]> Date: Mon, 20 Aug 2018 15:56:55 +1000 Subject: [PATCH 4/4] cmd: Return -errno for failed library calls Currently we ignore errno in a bunch of places, we just return -1. We can save the error value by returning negative errno. If we make a function call between the failing function call and the return we must save errno. If a library function only returns a single error code we can just return this instead of saving errno. Save errno if required; return negative errno for failed library calls. Signed-off-by: Tobin C. Harding <[email protected]> --- src/lxc/cmd/lxc_monitord.c | 11 +++++++---- src/lxc/cmd/lxc_user_nic.c | 22 +++++++++++++--------- src/lxc/cmd/lxc_usernsexec.c | 29 +++++++++++++++++------------ 3 files changed, 37 insertions(+), 25 deletions(-) diff --git a/src/lxc/cmd/lxc_monitord.c b/src/lxc/cmd/lxc_monitord.c index e6cb77338..389671bda 100644 --- a/src/lxc/cmd/lxc_monitord.c +++ b/src/lxc/cmd/lxc_monitord.c @@ -83,7 +83,7 @@ static int lxc_monitord_fifo_create(struct lxc_monitor *mon) { struct flock lk; char fifo_path[PATH_MAX]; - int ret; + int ret, save; ret = lxc_monitor_fifo_name(mon->lxcpath, fifo_path, sizeof(fifo_path), 1); if (ret < 0) @@ -91,15 +91,17 @@ static int lxc_monitord_fifo_create(struct lxc_monitor *mon) ret = mknod(fifo_path, S_IFIFO | S_IRUSR | S_IWUSR, 0); if (ret < 0 && errno != EEXIST) { + save = errno; SYSINFO("Failed to mknod monitor fifo %s", fifo_path); - return -1; + return -save; } mon->fifofd = open(fifo_path, O_RDWR); if (mon->fifofd < 0) { + save = errno; SYSERROR("Failed to open monitor fifo %s", fifo_path); unlink(fifo_path); - return -1; + return -save; } lk.l_type = F_WRLCK; @@ -108,10 +110,11 @@ static int lxc_monitord_fifo_create(struct lxc_monitor *mon) lk.l_len = 0; if (fcntl(mon->fifofd, F_SETLK, &lk) != 0) { + save = errno; /* another lxc-monitord is already running, don't start up */ SYSDEBUG("lxc-monitord already running on lxcpath %s", mon->lxcpath); close(mon->fifofd); - return -1; + return -save; } return 0; diff --git a/src/lxc/cmd/lxc_user_nic.c b/src/lxc/cmd/lxc_user_nic.c index 153940b86..e768608e1 100644 --- a/src/lxc/cmd/lxc_user_nic.c +++ b/src/lxc/cmd/lxc_user_nic.c @@ -80,13 +80,14 @@ static void usage(char *me, bool fail) static int open_and_lock(char *path) { - int fd, ret; + int fd, ret, save; struct flock lk; fd = open(path, O_RDWR | O_CREAT, S_IWUSR | S_IRUSR); if (fd < 0) { + save = errno; CMD_SYSERROR("Failed to open \"%s\"\n", path); - return -1; + return -errno; } lk.l_type = F_WRLCK; @@ -96,9 +97,10 @@ static int open_and_lock(char *path) ret = fcntl(fd, F_SETLKW, &lk); if (ret < 0) { + save = errno; CMD_SYSERROR("Failed to lock \"%s\"\n", path); close(fd); - return -1; + return -save; } return fd; @@ -311,7 +313,7 @@ static void free_alloted(struct alloted_s **head) * @group type bridge count * * Return the count entry for the calling user if there is one. Else - * return -1. + * return -EINVAL. */ static int get_alloted(char *me, char *intype, char *link, struct alloted_s **alloted) @@ -327,7 +329,7 @@ static int get_alloted(char *me, char *intype, char *link, fin = fopen(LXC_USERNIC_CONF, "r"); if (!fin) { CMD_SYSERROR("Failed to open \"%s\"\n", LXC_USERNIC_CONF); - return -1; + return -EINVAL; /* fopen only returns EINVAL */ } groups = get_groupnames(); @@ -527,18 +529,20 @@ static int get_mtu(char *name) static int create_nic(char *nic, char *br, int pid, char **cnic) { char veth1buf[IFNAMSIZ], veth2buf[IFNAMSIZ]; - int mtu, ret; + int mtu, ret, save; ret = snprintf(veth1buf, IFNAMSIZ, "%s", nic); if (ret < 0 || ret >= IFNAMSIZ) { + save = errno; usernic_error("%s", "Could not create nic name\n"); - return -1; + return -save; } ret = snprintf(veth2buf, IFNAMSIZ, "%sp", veth1buf); if (ret < 0 || ret >= IFNAMSIZ) { + save = errno; usernic_error("%s\n", "Could not create nic name"); - return -1; + return -save; } /* create the nics */ @@ -586,7 +590,7 @@ static int create_nic(char *nic, char *br, int pid, char **cnic) *cnic = strdup(veth2buf); if (!*cnic) { usernic_error("Failed to copy string \"%s\"\n", veth2buf); - return -1; + return -ENOMEM; /* strdup only returns ENOMEM */ } return 0; diff --git a/src/lxc/cmd/lxc_usernsexec.c b/src/lxc/cmd/lxc_usernsexec.c index 90d203547..65a020405 100644 --- a/src/lxc/cmd/lxc_usernsexec.c +++ b/src/lxc/cmd/lxc_usernsexec.c @@ -109,39 +109,44 @@ static void opentty(const char *tty, int which) static int do_child(void *vargv) { - int ret; + int ret, save; char **argv = (char **)vargv; /* Assume we want to become root */ ret = setgid(0); if (ret < 0) { + save = errno; CMD_SYSERROR("Failed to set gid to"); - return -1; + return -save; } ret = setuid(0); if (ret < 0) { + save = errno; CMD_SYSERROR("Failed to set uid to 0"); - return -1; + return -save; } ret = setgroups(0, NULL); if (ret < 0) { + save = errno; CMD_SYSERROR("Failed to clear supplementary groups"); - return -1; + return -save; } ret = unshare(CLONE_NEWNS); if (ret < 0) { + save = errno; CMD_SYSERROR("Failed to unshare mount namespace"); - return -1; + return -save; } if (detect_shared_rootfs()) { ret = mount(NULL, "/", NULL, MS_SLAVE | MS_REC, NULL); if (ret < 0) { + save = errno; CMD_SYSINFO("Failed to make \"/\" rslave"); - return -1; + return -save; } } @@ -181,7 +186,7 @@ static int parse_map(char *map) newmap = malloc(sizeof(*newmap)); if (!newmap) - return -1; + return -ENOMEM; newmap->hostid = host_id; newmap->nsid = ns_id; @@ -195,7 +200,7 @@ static int parse_map(char *map) tmp = malloc(sizeof(*tmp)); if (!tmp) { free(newmap); - return -1; + return -ENOMEM; } tmp->elem = newmap; @@ -224,7 +229,7 @@ static int read_default_map(char *fnam, int which, char *username) fin = fopen(fnam, "r"); if (!fin) - return -1; + return -errno; while (getline(&line, &sz, fin) != -1) { if (sz <= strlen(username) || @@ -244,7 +249,7 @@ static int read_default_map(char *fnam, int which, char *username) if (!newmap) { fclose(fin); free(line); - return -1; + return -ENOMEM; } newmap->hostid = atol(p1 + 1); @@ -257,7 +262,7 @@ static int read_default_map(char *fnam, int which, char *username) fclose(fin); free(line); free(newmap); - return -1; + return -ENOMEM; } tmp->elem = newmap; @@ -285,7 +290,7 @@ static int find_default_map(void) buf = malloc(bufsize); if (!buf) - return -1; + return -ENOMEM; ret = getpwuid_r(getuid(), &pwent, buf, bufsize, &pwentp); if (!pwentp) {
_______________________________________________ lxc-devel mailing list [email protected] http://lists.linuxcontainers.org/listinfo/lxc-devel
