[lxc-devel] [lxc/master] cmd: Clean up function return values
tcharding on Github
lxc-bot at linuxcontainers.org
Mon Aug 20 06:13:34 UTC 2018
A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 942 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20180820/85964f35/attachment.bin>
-------------- next part --------------
From ea04d16d00fb57db20a5e749f5fb5b4894c681cb Mon Sep 17 00:00:00 2001
From: 2xsec <dh48.jeong at samsung.com>
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 <dh48.jeong at samsung.com>
---
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" <me at tobin.cc>
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 <me at tobin.cc>
---
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" <me at tobin.cc>
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 <me at tobin.cc>
---
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" <me at tobin.cc>
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 <me at tobin.cc>
---
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) {
More information about the lxc-devel
mailing list