The following pull request was submitted through Github. It can be accessed and reviewed at: https://github.com/lxc/lxc/pull/3594
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) === Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com>
From c4ef8f4c1103c87144e5dabe051d23b3619179d7 Mon Sep 17 00:00:00 2001 From: Christian Brauner <christian.brau...@ubuntu.com> Date: Tue, 8 Dec 2020 11:53:54 +0100 Subject: [PATCH 1/3] tree-wide: use call_cleaner(netns_freeifaddrs) Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com> --- src/include/netns_ifaddrs.h | 3 +- src/lxc/confile.c | 12 +++---- src/lxc/lxccontainer.c | 72 +++++++++++++++++-------------------- 3 files changed, 38 insertions(+), 49 deletions(-) diff --git a/src/include/netns_ifaddrs.h b/src/include/netns_ifaddrs.h index 1b8703ee7d..d3f1d6eef8 100644 --- a/src/include/netns_ifaddrs.h +++ b/src/include/netns_ifaddrs.h @@ -13,7 +13,7 @@ extern "C" { #include <sys/socket.h> #include "compiler.h" -#include "netns_ifaddrs.h" +#include "memory_utils.h" struct netns_ifaddrs { struct netns_ifaddrs *ifa_next; @@ -52,6 +52,7 @@ struct netns_ifaddrs { #define __ifa_dstaddr ifa_ifu.ifu_dstaddr __hidden extern void netns_freeifaddrs(struct netns_ifaddrs *); +define_cleanup_function(struct netns_ifaddrs *, netns_freeifaddrs); __hidden extern int netns_getifaddrs(struct netns_ifaddrs **ifap, __s32 netns_id, bool *netnsid_aware); diff --git a/src/lxc/confile.c b/src/lxc/confile.c index 4f7621a900..6f5bf3909b 100644 --- a/src/lxc/confile.c +++ b/src/lxc/confile.c @@ -377,17 +377,16 @@ static int set_config_net_flags(const char *key, const char *value, static int create_matched_ifnames(const char *value, struct lxc_conf *lxc_conf, struct lxc_netdev *netdev) { - struct netns_ifaddrs *ifaddr, *ifa; + call_cleaner(netns_freeifaddrs) struct netns_ifaddrs *ifaddr = NULL; + struct netns_ifaddrs *ifa; int n; int ret = 0; const char *type_key = "lxc.net.type"; const char *link_key = "lxc.net.link"; const char *tmpvalue = "phys"; - if (netns_getifaddrs(&ifaddr, -1, &(bool){false}) < 0) { - SYSERROR("Failed to get network interfaces"); - return -1; - } + if (netns_getifaddrs(&ifaddr, -1, &(bool){false}) < 0) + return log_error_errno(-1, errno, "Failed to get network interfaces"); for (ifa = ifaddr, n = 0; ifa != NULL; ifa = ifa->ifa_next, n++) { if (!ifa->ifa_addr) @@ -413,9 +412,6 @@ static int create_matched_ifnames(const char *value, struct lxc_conf *lxc_conf, } } - netns_freeifaddrs(ifaddr); - ifaddr = NULL; - return ret; } diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c index 96aa372e1d..da18be8aa1 100644 --- a/src/lxc/lxccontainer.c +++ b/src/lxc/lxccontainer.c @@ -2340,20 +2340,21 @@ static char **do_lxcapi_get_interfaces(struct lxc_container *c) char **interfaces = NULL; char interface[IFNAMSIZ]; - if (pipe2(pipefd, O_CLOEXEC) < 0) - return NULL; + if (pipe2(pipefd, O_CLOEXEC)) + return log_error_errno(NULL, errno, "Failed to create pipe"); pid = fork(); if (pid < 0) { - SYSERROR("Failed to fork task to get interfaces information"); close(pipefd[0]); close(pipefd[1]); - return NULL; + return log_error_errno(NULL, errno, "Failed to fork task to get interfaces information"); } - if (pid == 0) { /* child */ - int ret = 1, nbytes; - struct netns_ifaddrs *interfaceArray = NULL, *tempIfAddr = NULL; + if (pid == 0) { + call_cleaner(netns_freeifaddrs) struct netns_ifaddrs *ifaddrs = NULL; + struct netns_ifaddrs *ifa = NULL; + int ret = 1; + int nbytes; /* close the read-end of the pipe */ close(pipefd[0]); @@ -2364,15 +2365,15 @@ static char **do_lxcapi_get_interfaces(struct lxc_container *c) } /* Grab the list of interfaces */ - if (netns_getifaddrs(&interfaceArray, -1, &(bool){false})) { + if (netns_getifaddrs(&ifaddrs, -1, &(bool){false})) { SYSERROR("Failed to get interfaces list"); goto out; } /* Iterate through the interfaces */ - for (tempIfAddr = interfaceArray; tempIfAddr != NULL; - tempIfAddr = tempIfAddr->ifa_next) { - nbytes = lxc_write_nointr(pipefd[1], tempIfAddr->ifa_name, IFNAMSIZ); + for (ifa = ifaddrs; ifa != NULL; + ifa = ifa->ifa_next) { + nbytes = lxc_write_nointr(pipefd[1], ifa->ifa_name, IFNAMSIZ); if (nbytes < 0) goto out; @@ -2382,9 +2383,6 @@ static char **do_lxcapi_get_interfaces(struct lxc_container *c) ret = 0; out: - if (interfaceArray) - netns_freeifaddrs(interfaceArray); - /* close the write-end of the pipe, thus sending EOF to the reader */ close(pipefd[1]); _exit(ret); @@ -2405,7 +2403,7 @@ static char **do_lxcapi_get_interfaces(struct lxc_container *c) count++; } - if (wait_for_pid(pid) != 0) { + if (wait_for_pid(pid)) { for (i = 0; i < count; i++) free(interfaces[i]); @@ -2436,10 +2434,8 @@ static char **do_lxcapi_get_ips(struct lxc_container *c, const char *interface, char **addresses = NULL; ret = pipe2(pipefd, O_CLOEXEC); - if (ret < 0) { - SYSERROR("Failed to create pipe"); - return NULL; - } + if (ret < 0) + return log_error_errno(NULL, errno, "Failed to create pipe"); pid = fork(); if (pid < 0) { @@ -2450,11 +2446,12 @@ static char **do_lxcapi_get_ips(struct lxc_container *c, const char *interface, } if (pid == 0) { + call_cleaner(netns_freeifaddrs) struct netns_ifaddrs *ifaddrs = NULL; + struct netns_ifaddrs *ifa = NULL; ssize_t nbytes; char addressOutputBuffer[INET6_ADDRSTRLEN]; char *address_ptr = NULL; - void *tempAddrPtr = NULL; - struct netns_ifaddrs *interfaceArray = NULL, *tempIfAddr = NULL; + void *address_ptr_tmp = NULL; /* close the read-end of the pipe */ close(pipefd[0]); @@ -2465,52 +2462,50 @@ static char **do_lxcapi_get_ips(struct lxc_container *c, const char *interface, } /* Grab the list of interfaces */ - if (netns_getifaddrs(&interfaceArray, -1, &(bool){false})) { + if (netns_getifaddrs(&ifaddrs, -1, &(bool){false})) { SYSERROR("Failed to get interfaces list"); goto out; } /* Iterate through the interfaces */ - for (tempIfAddr = interfaceArray; tempIfAddr; - tempIfAddr = tempIfAddr->ifa_next) { - if (tempIfAddr->ifa_addr == NULL) + for (ifa = ifaddrs; ifa; ifa = ifa->ifa_next) { + if (ifa->ifa_addr == NULL) continue; #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wcast-align" - if (tempIfAddr->ifa_addr->sa_family == AF_INET) { + if (ifa->ifa_addr->sa_family == AF_INET) { if (family && strcmp(family, "inet")) continue; - tempAddrPtr = &((struct sockaddr_in *)tempIfAddr->ifa_addr)->sin_addr; + address_ptr_tmp = &((struct sockaddr_in *)ifa->ifa_addr)->sin_addr; } else { if (family && strcmp(family, "inet6")) continue; - if (((struct sockaddr_in6 *)tempIfAddr->ifa_addr)->sin6_scope_id != scope) + if (((struct sockaddr_in6 *)ifa->ifa_addr)->sin6_scope_id != scope) continue; - tempAddrPtr = &((struct sockaddr_in6 *)tempIfAddr->ifa_addr)->sin6_addr; + address_ptr_tmp = &((struct sockaddr_in6 *)ifa->ifa_addr)->sin6_addr; } #pragma GCC diagnostic pop - if (interface && strcmp(interface, tempIfAddr->ifa_name)) + if (interface && strcmp(interface, ifa->ifa_name)) continue; - else if (!interface && strcmp("lo", tempIfAddr->ifa_name) == 0) + else if (!interface && strcmp("lo", ifa->ifa_name) == 0) continue; - address_ptr = (char *)inet_ntop(tempIfAddr->ifa_addr->sa_family, - tempAddrPtr, addressOutputBuffer, - sizeof(addressOutputBuffer)); + address_ptr = (char *)inet_ntop(ifa->ifa_addr->sa_family, address_ptr_tmp, + addressOutputBuffer, + sizeof(addressOutputBuffer)); if (!address_ptr) continue; nbytes = lxc_write_nointr(pipefd[1], address_ptr, INET6_ADDRSTRLEN); if (nbytes != INET6_ADDRSTRLEN) { - SYSERROR("Failed to send ipv6 address \"%s\"", - address_ptr); + SYSERROR("Failed to send ipv6 address \"%s\"", address_ptr); goto out; } @@ -2520,9 +2515,6 @@ static char **do_lxcapi_get_ips(struct lxc_container *c, const char *interface, ret = 0; out: - if (interfaceArray) - netns_freeifaddrs(interfaceArray); - /* close the write-end of the pipe, thus sending EOF to the reader */ close(pipefd[1]); _exit(ret); @@ -2540,7 +2532,7 @@ static char **do_lxcapi_get_ips(struct lxc_container *c, const char *interface, count++; } - if (wait_for_pid(pid) != 0) { + if (wait_for_pid(pid)) { for (i = 0; i < count; i++) free(addresses[i]); From 059a1ec30bf9a04c296b173b72c143a720176af2 Mon Sep 17 00:00:00 2001 From: Christian Brauner <christian.brau...@ubuntu.com> Date: Tue, 8 Dec 2020 12:05:47 +0100 Subject: [PATCH 2/3] confile: clean up network configuration parsing Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com> --- src/lxc/confile.c | 273 ++++++++++++++++++++-------------------------- src/lxc/utils.c | 4 +- 2 files changed, 120 insertions(+), 157 deletions(-) diff --git a/src/lxc/confile.c b/src/lxc/confile.c index 6f5bf3909b..90b52f7a41 100644 --- a/src/lxc/confile.c +++ b/src/lxc/confile.c @@ -323,7 +323,7 @@ static int set_config_net_type(const char *key, const char *value, return clr_config_net_type(key, lxc_conf, data); if (!netdev) - return -1; + return ret_errno(EINVAL); if (strcmp(value, "veth") == 0) { netdev->type = LXC_NET_VETH; @@ -351,8 +351,7 @@ static int set_config_net_type(const char *key, const char *value, } else if (strcmp(value, "none") == 0) { netdev->type = LXC_NET_NONE; } else { - ERROR("Invalid network type %s", value); - return -1; + return log_error(-1, "Invalid network type %s", value); } return 0; @@ -367,7 +366,7 @@ static int set_config_net_flags(const char *key, const char *value, return clr_config_net_flags(key, lxc_conf, data); if (!netdev) - return -1; + return ret_errno(EINVAL); netdev->flags |= IFF_UP; @@ -425,7 +424,7 @@ static int set_config_net_link(const char *key, const char *value, return clr_config_net_link(key, lxc_conf, data); if (!netdev) - return -1; + return ret_errno(EINVAL); if (value[strlen(value) - 1] == '+' && netdev->type == LXC_NET_PHYS) ret = create_matched_ifnames(value, lxc_conf, netdev); @@ -446,11 +445,11 @@ static int set_config_net_l2proxy(const char *key, const char *value, return clr_config_net_l2proxy(key, lxc_conf, data); if (!netdev) - return ret_set_errno(-1, EINVAL); + return ret_errno(EINVAL); ret = lxc_safe_uint(value, &val); if (ret < 0) - return ret_set_errno(-1, -ret); + return ret_errno(-ret); switch (val) { case 0: @@ -473,7 +472,7 @@ static int set_config_net_name(const char *key, const char *value, return clr_config_net_name(key, lxc_conf, data); if (!netdev) - return -1; + return ret_errno(EINVAL); return network_ifname(netdev->name, value, sizeof(netdev->name)); } @@ -488,7 +487,7 @@ static int set_config_net_veth_mode(const char *key, const char *value, return clr_config_net_veth_mode(key, lxc_conf, data); if (!netdev) - return -1; + return ret_errno(EINVAL); return lxc_veth_mode_to_flag(&netdev->priv.veth_attr.mode, value); } @@ -502,9 +501,10 @@ static int set_config_net_veth_pair(const char *key, const char *value, return clr_config_net_veth_pair(key, lxc_conf, data); if (!netdev) - return -1; + return ret_errno(EINVAL); - return network_ifname(netdev->priv.veth_attr.pair, value, sizeof(netdev->priv.veth_attr.pair)); + return network_ifname(netdev->priv.veth_attr.pair, value, + sizeof(netdev->priv.veth_attr.pair)); } static int set_config_net_veth_vlan_id(const char *key, const char *value, @@ -579,7 +579,7 @@ static int set_config_net_macvlan_mode(const char *key, const char *value, return clr_config_net_macvlan_mode(key, lxc_conf, data); if (!netdev) - return -1; + return ret_errno(EINVAL); return lxc_macvlan_mode_to_flag(&netdev->priv.macvlan_attr.mode, value); } @@ -593,12 +593,12 @@ static int set_config_net_ipvlan_mode(const char *key, const char *value, return clr_config_net_ipvlan_mode(key, lxc_conf, data); if (!netdev) - return ret_set_errno(-1, EINVAL); + return ret_errno(EINVAL); - if (netdev->type != LXC_NET_IPVLAN) { - SYSERROR("Invalid ipvlan mode \"%s\", can only be used with ipvlan network", value); - return ret_set_errno(-1, EINVAL); - } + if (netdev->type != LXC_NET_IPVLAN) + return log_error_errno(-EINVAL, + EINVAL, "Invalid ipvlan mode \"%s\", can only be used with ipvlan network", + value); return lxc_ipvlan_mode_to_flag(&netdev->priv.ipvlan_attr.mode, value); } @@ -612,12 +612,12 @@ static int set_config_net_ipvlan_isolation(const char *key, const char *value, return clr_config_net_ipvlan_isolation(key, lxc_conf, data); if (!netdev) - return ret_set_errno(-1, EINVAL); + return ret_errno(EINVAL); - if (netdev->type != LXC_NET_IPVLAN) { - SYSERROR("Invalid ipvlan isolation \"%s\", can only be used with ipvlan network", value); - return ret_set_errno(-1, EINVAL); - } + if (netdev->type != LXC_NET_IPVLAN) + return log_error_errno(-EINVAL, + EINVAL, "Invalid ipvlan isolation \"%s\", can only be used with ipvlan network", + value); return lxc_ipvlan_isolation_to_flag(&netdev->priv.ipvlan_attr.isolation, value); } @@ -632,11 +632,11 @@ static int set_config_net_hwaddr(const char *key, const char *value, return clr_config_net_hwaddr(key, lxc_conf, data); if (!netdev) - return -1; + return ret_errno(EINVAL); new_value = strdup(value); if (!new_value) - return -1; + return ret_errno(ENOMEM); rand_complete_hwaddr(new_value); @@ -661,11 +661,11 @@ static int set_config_net_vlan_id(const char *key, const char *value, return clr_config_net_vlan_id(key, lxc_conf, data); if (!netdev) - return -1; + return ret_errno(EINVAL); ret = get_u16(&netdev->priv.vlan_attr.vid, value, 0); if (ret < 0) - return -1; + return ret; return 0; } @@ -679,7 +679,7 @@ static int set_config_net_mtu(const char *key, const char *value, return clr_config_net_mtu(key, lxc_conf, data); if (!netdev) - return -1; + return ret_errno(EINVAL); return set_config_string_item(&netdev->mtu, value); } @@ -687,39 +687,34 @@ static int set_config_net_mtu(const char *key, const char *value, static int set_config_net_ipv4_address(const char *key, const char *value, struct lxc_conf *lxc_conf, void *data) { + __do_free char *addr = NULL; + __do_free struct lxc_inetdev *inetdev = NULL; + __do_free struct lxc_list *list = NULL; int ret; struct lxc_netdev *netdev = data; - struct lxc_inetdev *inetdev; - struct lxc_list *list; char *cursor, *slash; - char *addr = NULL, *bcast = NULL, *prefix = NULL; + char *bcast = NULL, *prefix = NULL; if (lxc_config_value_empty(value)) return clr_config_net_ipv4_address(key, lxc_conf, data); if (!netdev) - return -1; + return ret_errno(EINVAL); inetdev = malloc(sizeof(*inetdev)); if (!inetdev) - return -1; + return ret_errno(ENOMEM); memset(inetdev, 0, sizeof(*inetdev)); list = malloc(sizeof(*list)); - if (!list) { - free(inetdev); - return -1; - } + if (!list) + return ret_errno(ENOMEM); lxc_list_init(list); - list->elem = inetdev; addr = strdup(value); - if (!addr) { - free(inetdev); - free(list); - return -1; - } + if (!addr) + return ret_errno(ENOMEM); cursor = strstr(addr, " "); if (cursor) { @@ -734,35 +729,21 @@ static int set_config_net_ipv4_address(const char *key, const char *value, } ret = inet_pton(AF_INET, addr, &inetdev->addr); - if (!ret || ret < 0) { - SYSERROR("Invalid ipv4 address \"%s\"", value); - free(inetdev); - free(addr); - free(list); - return -1; - } + if (!ret || ret < 0) + return log_error_errno(-1, errno, "Invalid ipv4 address \"%s\"", value); if (bcast) { ret = inet_pton(AF_INET, bcast, &inetdev->bcast); - if (!ret || ret < 0) { - SYSERROR("Invalid ipv4 broadcast address \"%s\"", value); - free(inetdev); - free(list); - free(addr); - return -1; - } + if (!ret || ret < 0) + return log_error_errno(-1, errno, "Invalid ipv4 broadcast address \"%s\"", value); } /* No prefix specified, determine it from the network class. */ if (prefix) { ret = lxc_safe_uint(prefix, &inetdev->prefix); - if (ret < 0) { - free(inetdev); - free(list); - free(addr); - return -1; - } + if (ret < 0) + return ret; } else { inetdev->prefix = config_ip_prefix(&inetdev->addr); } @@ -775,8 +756,10 @@ static int set_config_net_ipv4_address(const char *key, const char *value, inetdev->bcast.s_addr |= htonl(INADDR_BROADCAST >> inetdev->prefix); } + list->elem = inetdev; lxc_list_add_tail(&netdev->ipv4, list); - free(addr); + move_ptr(inetdev); + move_ptr(list); return 0; } @@ -802,21 +785,18 @@ static int set_config_net_ipv4_gateway(const char *key, const char *value, netdev->ipv4_gateway_auto = false; netdev->ipv4_gateway_dev = true; } else { + __do_free struct in_addr *gw = NULL; int ret; - struct in_addr *gw; gw = malloc(sizeof(*gw)); if (!gw) - return -1; + return ret_errno(ENOMEM); ret = inet_pton(AF_INET, value, gw); - if (!ret || ret < 0) { - SYSERROR("Invalid ipv4 gateway address \"%s\"", value); - free(gw); - return -1; - } + if (!ret || ret < 0) + return log_error_errno(-1, errno, "Invalid ipv4 gateway address \"%s\"", value); - netdev->ipv4_gateway = gw; + netdev->ipv4_gateway = move_ptr(gw); netdev->ipv4_gateway_auto = false; } @@ -837,47 +817,47 @@ static int set_config_net_veth_ipv4_route(const char *key, const char *value, return clr_config_net_veth_ipv4_route(key, lxc_conf, data); if (!netdev) - return ret_set_errno(-1, EINVAL); + return ret_errno(EINVAL); - if (netdev->type != LXC_NET_VETH) { - SYSERROR("Invalid ipv4 route \"%s\", can only be used with veth network", value); - return ret_set_errno(-1, EINVAL); - } + if (netdev->type != LXC_NET_VETH) + return log_error_errno(-EINVAL, + EINVAL, "Invalid ipv4 route \"%s\", can only be used with veth network", + value); inetdev = malloc(sizeof(*inetdev)); if (!inetdev) - return -1; + return ret_errno(ENOMEM); memset(inetdev, 0, sizeof(*inetdev)); list = malloc(sizeof(*list)); if (!list) - return -1; + return ret_errno(ENOMEM); lxc_list_init(list); list->elem = inetdev; valdup = strdup(value); if (!valdup) - return -1; + return ret_errno(ENOMEM); slash = strchr(valdup, '/'); if (!slash) - return ret_set_errno(-1, EINVAL); + return ret_errno(EINVAL); *slash = '\0'; slash++; if (*slash == '\0') - return ret_set_errno(-1, EINVAL); + return ret_errno(EINVAL); netmask = slash; ret = lxc_safe_uint(netmask, &inetdev->prefix); if (ret < 0 || inetdev->prefix > 32) - return ret_set_errno(-1, EINVAL); + return ret_errno(EINVAL); ret = inet_pton(AF_INET, valdup, &inetdev->addr); if (!ret || ret < 0) - return ret_set_errno(-1, EINVAL); + return ret_errno(EINVAL); lxc_list_add_tail(&netdev->priv.veth_attr.ipv4_routes, list); move_ptr(inetdev); @@ -889,38 +869,33 @@ static int set_config_net_veth_ipv4_route(const char *key, const char *value, static int set_config_net_ipv6_address(const char *key, const char *value, struct lxc_conf *lxc_conf, void *data) { + __do_free char *valdup = NULL; + __do_free struct lxc_inet6dev *inet6dev = NULL; + __do_free struct lxc_list *list = NULL; int ret; struct lxc_netdev *netdev = data; - struct lxc_inet6dev *inet6dev; - struct lxc_list *list; - char *slash, *valdup, *netmask; + char *slash, *netmask; if (lxc_config_value_empty(value)) return clr_config_net_ipv6_address(key, lxc_conf, data); if (!netdev) - return -1; + return ret_errno(EINVAL); inet6dev = malloc(sizeof(*inet6dev)); if (!inet6dev) - return -1; + return ret_errno(ENOMEM); memset(inet6dev, 0, sizeof(*inet6dev)); list = malloc(sizeof(*list)); - if (!list) { - free(inet6dev); - return -1; - } + if (!list) + return ret_errno(ENOMEM); lxc_list_init(list); - list->elem = inet6dev; valdup = strdup(value); - if (!valdup) { - free(list); - free(inet6dev); - return -1; - } + if (!valdup) + return ret_errno(ENOMEM); inet6dev->prefix = 64; slash = strstr(valdup, "/"); @@ -929,25 +904,18 @@ static int set_config_net_ipv6_address(const char *key, const char *value, netmask = slash + 1; ret = lxc_safe_uint(netmask, &inet6dev->prefix); - if (ret < 0) { - free(list); - free(inet6dev); - free(valdup); - return -1; - } + if (ret < 0) + return ret; } ret = inet_pton(AF_INET6, valdup, &inet6dev->addr); - if (!ret || ret < 0) { - SYSERROR("Invalid ipv6 address \"%s\"", valdup); - free(list); - free(inet6dev); - free(valdup); - return -1; - } + if (!ret || ret < 0) + return log_error_errno(-EINVAL, EINVAL, "Invalid ipv6 address \"%s\"", valdup); + list->elem = inet6dev; lxc_list_add_tail(&netdev->ipv6, list); - free(valdup); + move_ptr(inet6dev); + move_ptr(list); return 0; } @@ -961,7 +929,7 @@ static int set_config_net_ipv6_gateway(const char *key, const char *value, return clr_config_net_ipv6_gateway(key, lxc_conf, data); if (!netdev) - return -1; + return ret_errno(EINVAL); free(netdev->ipv6_gateway); @@ -974,20 +942,18 @@ static int set_config_net_ipv6_gateway(const char *key, const char *value, netdev->ipv6_gateway_dev = true; } else { int ret; - struct in6_addr *gw; + __do_free struct in6_addr *gw = NULL; gw = malloc(sizeof(*gw)); if (!gw) - return -1; + return ret_errno(ENOMEM); ret = inet_pton(AF_INET6, value, gw); - if (!ret || ret < 0) { - SYSERROR("Invalid ipv6 gateway address \"%s\"", value); - free(gw); - return -1; - } + if (!ret || ret < 0) + return log_error_errno(-EINVAL, EINVAL, + "Invalid ipv6 gateway address \"%s\"", value); - netdev->ipv6_gateway = gw; + netdev->ipv6_gateway = move_ptr(gw); netdev->ipv6_gateway_auto = false; } @@ -1008,24 +974,23 @@ static int set_config_net_veth_ipv6_route(const char *key, const char *value, return clr_config_net_veth_ipv6_route(key, lxc_conf, data); if (!netdev) - return ret_set_errno(-1, EINVAL); + return ret_errno(EINVAL); - if (netdev->type != LXC_NET_VETH) { - SYSERROR("Invalid ipv6 route \"%s\", can only be used with veth network", value); - return ret_set_errno(-1, EINVAL); - } + if (netdev->type != LXC_NET_VETH) + return log_error_errno(-EINVAL, + EINVAL, "Invalid ipv6 route \"%s\", can only be used with veth network", + value); inet6dev = malloc(sizeof(*inet6dev)); if (!inet6dev) - return -1; + return ret_errno(ENOMEM); memset(inet6dev, 0, sizeof(*inet6dev)); list = malloc(sizeof(*list)); if (!list) - return -1; + return ret_errno(ENOMEM); lxc_list_init(list); - list->elem = inet6dev; valdup = strdup(value); if (!valdup) @@ -1033,23 +998,24 @@ static int set_config_net_veth_ipv6_route(const char *key, const char *value, slash = strchr(valdup, '/'); if (!slash) - return ret_set_errno(-1, EINVAL); + return ret_errno(EINVAL); *slash = '\0'; slash++; if (*slash == '\0') - return ret_set_errno(-1, EINVAL); + return ret_errno(EINVAL); netmask = slash; ret = lxc_safe_uint(netmask, &inet6dev->prefix); if (ret < 0 || inet6dev->prefix > 128) - return ret_set_errno(-1, EINVAL); + return ret_errno(EINVAL); ret = inet_pton(AF_INET6, valdup, &inet6dev->addr); if (!ret || ret < 0) - return ret_set_errno(-1, EINVAL); + return ret_errno(EINVAL); + list->elem = inet6dev; lxc_list_add_tail(&netdev->priv.veth_attr.ipv6_routes, list); move_ptr(inet6dev); move_ptr(list); @@ -1066,7 +1032,7 @@ static int set_config_net_script_up(const char *key, const char *value, return clr_config_net_script_up(key, lxc_conf, data); if (!netdev) - return -1; + return ret_errno(EINVAL); return set_config_string_item(&netdev->upscript, value); } @@ -1080,22 +1046,21 @@ static int set_config_net_script_down(const char *key, const char *value, return clr_config_net_script_down(key, lxc_conf, data); if (!netdev) - return -1; + return ret_errno(EINVAL); return set_config_string_item(&netdev->downscript, value); } -static int add_hook(struct lxc_conf *lxc_conf, int which, char *hook) +static int add_hook(struct lxc_conf *lxc_conf, int which, __owns char *hook) { + __do_free char *val = hook; struct lxc_list *hooklist; hooklist = malloc(sizeof(*hooklist)); - if (!hooklist) { - free(hook); - return -1; - } + if (!hooklist) + return ret_errno(ENOMEM); - hooklist->elem = hook; + hooklist->elem = move_ptr(val); lxc_list_add_tail(&lxc_conf->hooks[which], hooklist); return 0; @@ -1216,7 +1181,7 @@ static int set_config_init_gid(const char *key, const char *value, static int set_config_hooks(const char *key, const char *value, struct lxc_conf *lxc_conf, void *data) { - char *copy; + __do_free char *copy = NULL; if (lxc_config_value_empty(value)) return lxc_clear_hooks(lxc_conf, key); @@ -1228,30 +1193,28 @@ static int set_config_hooks(const char *key, const char *value, copy = strdup(value); if (!copy) - return -1; + return ret_errno(ENOMEM); if (strcmp(key + 9, "pre-start") == 0) - return add_hook(lxc_conf, LXCHOOK_PRESTART, copy); + return add_hook(lxc_conf, LXCHOOK_PRESTART, move_ptr(copy)); else if (strcmp(key + 9, "start-host") == 0) - return add_hook(lxc_conf, LXCHOOK_START_HOST, copy); + return add_hook(lxc_conf, LXCHOOK_START_HOST, move_ptr(copy)); else if (strcmp(key + 9, "pre-mount") == 0) - return add_hook(lxc_conf, LXCHOOK_PREMOUNT, copy); + return add_hook(lxc_conf, LXCHOOK_PREMOUNT, move_ptr(copy)); else if (strcmp(key + 9, "autodev") == 0) - return add_hook(lxc_conf, LXCHOOK_AUTODEV, copy); + return add_hook(lxc_conf, LXCHOOK_AUTODEV, move_ptr(copy)); else if (strcmp(key + 9, "mount") == 0) - return add_hook(lxc_conf, LXCHOOK_MOUNT, copy); + return add_hook(lxc_conf, LXCHOOK_MOUNT, move_ptr(copy)); else if (strcmp(key + 9, "start") == 0) - return add_hook(lxc_conf, LXCHOOK_START, copy); + return add_hook(lxc_conf, LXCHOOK_START, move_ptr(copy)); else if (strcmp(key + 9, "stop") == 0) - return add_hook(lxc_conf, LXCHOOK_STOP, copy); + return add_hook(lxc_conf, LXCHOOK_STOP, move_ptr(copy)); else if (strcmp(key + 9, "post-stop") == 0) - return add_hook(lxc_conf, LXCHOOK_POSTSTOP, copy); + return add_hook(lxc_conf, LXCHOOK_POSTSTOP, move_ptr(copy)); else if (strcmp(key + 9, "clone") == 0) - return add_hook(lxc_conf, LXCHOOK_CLONE, copy); + return add_hook(lxc_conf, LXCHOOK_CLONE, move_ptr(copy)); else if (strcmp(key + 9, "destroy") == 0) - return add_hook(lxc_conf, LXCHOOK_DESTROY, copy); - - free(copy); + return add_hook(lxc_conf, LXCHOOK_DESTROY, move_ptr(copy)); return -1; } diff --git a/src/lxc/utils.c b/src/lxc/utils.c index fdf437ef35..27b0fb7359 100644 --- a/src/lxc/utils.c +++ b/src/lxc/utils.c @@ -201,12 +201,12 @@ extern int get_u16(unsigned short *val, const char *arg, int base) char *ptr; if (!arg || !*arg) - return -1; + return ret_errno(EINVAL); errno = 0; res = strtoul(arg, &ptr, base); if (!ptr || ptr == arg || *ptr || res > 0xFFFF || errno != 0) - return -1; + return ret_errno(ERANGE); *val = res; From ed1454e85281cbabe6821b370decaee26b919dc3 Mon Sep 17 00:00:00 2001 From: Christian Brauner <christian.brau...@ubuntu.com> Date: Tue, 8 Dec 2020 12:19:04 +0100 Subject: [PATCH 3/3] confile: clean up hooks Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com> --- src/lxc/confile.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/lxc/confile.c b/src/lxc/confile.c index 90b52f7a41..e7ab359291 100644 --- a/src/lxc/confile.c +++ b/src/lxc/confile.c @@ -1186,10 +1186,8 @@ static int set_config_hooks(const char *key, const char *value, if (lxc_config_value_empty(value)) return lxc_clear_hooks(lxc_conf, key); - if (strcmp(key + 4, "hook") == 0) { - ERROR("lxc.hook must not have a value"); - return -1; - } + if (strcmp(key + 4, "hook") == 0) + return log_error_errno(-EINVAL, EINVAL, "lxc.hook must not have a value"); copy = strdup(value); if (!copy) @@ -1232,11 +1230,9 @@ static int set_config_hooks_version(const char *key, const char *value, if (ret < 0) return -1; - if (tmp > 1) { - ERROR("Invalid hook version specified. Currently only 0 " - "(legacy) and 1 are supported"); - return -1; - } + if (tmp > 1) + return log_error_errno(-EINVAL, + EINVAL, "Invalid hook version specified. Currently only 0 (legacy) and 1 are supported"); lxc_conf->hooks_version = tmp;
_______________________________________________ lxc-devel mailing list lxc-devel@lists.linuxcontainers.org http://lists.linuxcontainers.org/listinfo/lxc-devel