[lxc-devel] [lxc/master] network: fixes
brauner on Github
lxc-bot at linuxcontainers.org
Fri Mar 20 13:05:17 UTC 2020
A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 364 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20200320/69b02747/attachment.bin>
-------------- next part --------------
From 19bfd55a0896bc131b0a08998d7aad9dbe526284 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Fri, 20 Mar 2020 13:04:23 +0100
Subject: [PATCH 1/2] nl: improve how we surface errors
Closes #3057.
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
src/lxc/nl.c | 143 +++++++++++++++++++++++----------------------------
1 file changed, 63 insertions(+), 80 deletions(-)
diff --git a/src/lxc/nl.c b/src/lxc/nl.c
index 4de2bb82a6..dd94c09c88 100644
--- a/src/lxc/nl.c
+++ b/src/lxc/nl.c
@@ -19,16 +19,19 @@
lxc_log_define(nl, lxc);
-extern size_t nlmsg_len(const struct nlmsg *nlmsg)
+size_t nlmsg_len(const struct nlmsg *nlmsg)
{
return nlmsg->nlmsghdr->nlmsg_len - NLMSG_HDRLEN;
}
-extern void *nlmsg_data(struct nlmsg *nlmsg)
+void *nlmsg_data(struct nlmsg *nlmsg)
{
- char *data = ((char *)nlmsg) + NLMSG_HDRLEN;
+ char *data;
+
+ data = ((char *)nlmsg) + NLMSG_HDRLEN;
if (!nlmsg_len(nlmsg))
- return NULL;
+ return ret_set_errno(NULL, EINVAL);
+
return data;
}
@@ -40,7 +43,7 @@ static int nla_put(struct nlmsg *nlmsg, int attr,
size_t tlen = NLMSG_ALIGN(nlmsg->nlmsghdr->nlmsg_len) + RTA_ALIGN(rtalen);
if (tlen > nlmsg->cap)
- return -ENOMEM;
+ return ret_errno(ENOMEM);
rta = NLMSG_TAIL(nlmsg->nlmsghdr);
rta->rta_type = attr;
@@ -48,41 +51,42 @@ static int nla_put(struct nlmsg *nlmsg, int attr,
if (data && len)
memcpy(RTA_DATA(rta), data, len);
nlmsg->nlmsghdr->nlmsg_len = tlen;
+
return 0;
}
-extern int nla_put_buffer(struct nlmsg *nlmsg, int attr,
- const void *data, size_t size)
+int nla_put_buffer(struct nlmsg *nlmsg, int attr, const void *data, size_t size)
{
return nla_put(nlmsg, attr, data, size);
}
-extern int nla_put_string(struct nlmsg *nlmsg, int attr, const char *string)
+int nla_put_string(struct nlmsg *nlmsg, int attr, const char *string)
{
return nla_put(nlmsg, attr, string, strlen(string) + 1);
}
-extern int nla_put_u32(struct nlmsg *nlmsg, int attr, int value)
+int nla_put_u32(struct nlmsg *nlmsg, int attr, int value)
{
return nla_put(nlmsg, attr, &value, sizeof(value));
}
-extern int nla_put_u16(struct nlmsg *nlmsg, int attr, unsigned short value)
+int nla_put_u16(struct nlmsg *nlmsg, int attr, unsigned short value)
{
return nla_put(nlmsg, attr, &value, 2);
}
-extern int nla_put_attr(struct nlmsg *nlmsg, int attr)
+int nla_put_attr(struct nlmsg *nlmsg, int attr)
{
return nla_put(nlmsg, attr, NULL, 0);
}
struct rtattr *nla_begin_nested(struct nlmsg *nlmsg, int attr)
{
- struct rtattr *rtattr = NLMSG_TAIL(nlmsg->nlmsghdr);
+ struct rtattr *rtattr;
+ rtattr = NLMSG_TAIL(nlmsg->nlmsghdr);
if (nla_put_attr(nlmsg, attr))
- return NULL;
+ return ret_set_errno(NULL, ENOMEM);
return rtattr;
}
@@ -92,37 +96,34 @@ void nla_end_nested(struct nlmsg *nlmsg, struct rtattr *attr)
attr->rta_len = (void *)NLMSG_TAIL(nlmsg->nlmsghdr) - (void *)attr;
}
-extern struct nlmsg *nlmsg_alloc(size_t size)
+struct nlmsg *nlmsg_alloc(size_t size)
{
- struct nlmsg *nlmsg;
+ __do_free struct nlmsg *nlmsg = NULL;
size_t len = NLMSG_HDRLEN + NLMSG_ALIGN(size);
- nlmsg = (struct nlmsg *)malloc(sizeof(struct nlmsg));
+ nlmsg = malloc(sizeof(struct nlmsg));
if (!nlmsg)
- return NULL;
+ return ret_set_errno(NULL, ENOMEM);
- nlmsg->nlmsghdr = (struct nlmsghdr *)malloc(len);
+ nlmsg->nlmsghdr = malloc(len);
if (!nlmsg->nlmsghdr)
- goto errout;
+ return ret_set_errno(NULL, ENOMEM);
memset(nlmsg->nlmsghdr, 0, len);
nlmsg->cap = len;
nlmsg->nlmsghdr->nlmsg_len = NLMSG_HDRLEN;
- return nlmsg;
-errout:
- free(nlmsg);
- return NULL;
+ return move_ptr(nlmsg);
}
-extern void *nlmsg_reserve(struct nlmsg *nlmsg, size_t len)
+void *nlmsg_reserve(struct nlmsg *nlmsg, size_t len)
{
void *buf;
size_t nlmsg_len = nlmsg->nlmsghdr->nlmsg_len;
size_t tlen = NLMSG_ALIGN(len);
if (nlmsg_len + tlen > nlmsg->cap)
- return NULL;
+ return ret_set_errno(NULL, ENOMEM);
buf = ((char *)(nlmsg->nlmsghdr)) + nlmsg_len;
nlmsg->nlmsghdr->nlmsg_len += tlen;
@@ -133,29 +134,28 @@ extern void *nlmsg_reserve(struct nlmsg *nlmsg, size_t len)
return buf;
}
-extern struct nlmsg *nlmsg_alloc_reserve(size_t size)
+struct nlmsg *nlmsg_alloc_reserve(size_t size)
{
struct nlmsg *nlmsg;
nlmsg = nlmsg_alloc(size);
if (!nlmsg)
- return NULL;
+ return ret_set_errno(NULL, ENOMEM);
/* Just set message length to cap directly. */
nlmsg->nlmsghdr->nlmsg_len = nlmsg->cap;
return nlmsg;
}
-extern void nlmsg_free(struct nlmsg *nlmsg)
+void nlmsg_free(struct nlmsg *nlmsg)
{
- if (!nlmsg)
- return;
-
- free(nlmsg->nlmsghdr);
- free(nlmsg);
+ if (nlmsg) {
+ free(nlmsg->nlmsghdr);
+ free(nlmsg);
+ }
}
-extern int __netlink_recv(struct nl_handler *handler, struct nlmsghdr *nlmsghdr)
+int __netlink_recv(struct nl_handler *handler, struct nlmsghdr *nlmsghdr)
{
int ret;
struct sockaddr_nl nladdr;
@@ -182,26 +182,24 @@ extern int __netlink_recv(struct nl_handler *handler, struct nlmsghdr *nlmsghdr)
if (errno == EINTR)
goto again;
- return -1;
+ return ret_errno(errno);
}
if (!ret)
return 0;
- if (msg.msg_flags & MSG_TRUNC && (ret == nlmsghdr->nlmsg_len)) {
- errno = EMSGSIZE;
- ret = -1;
- }
+ if (msg.msg_flags & MSG_TRUNC && (ret == nlmsghdr->nlmsg_len))
+ return ret_errno(EMSGSIZE);
return ret;
}
-extern int netlink_rcv(struct nl_handler *handler, struct nlmsg *answer)
+int netlink_rcv(struct nl_handler *handler, struct nlmsg *answer)
{
return __netlink_recv(handler, answer->nlmsghdr);
}
-extern int __netlink_send(struct nl_handler *handler, struct nlmsghdr *nlmsghdr)
+int __netlink_send(struct nl_handler *handler, struct nlmsghdr *nlmsghdr)
{
int ret;
struct sockaddr_nl nladdr;
@@ -223,7 +221,7 @@ extern int __netlink_send(struct nl_handler *handler, struct nlmsghdr *nlmsghdr)
ret = sendmsg(handler->fd, &msg, MSG_NOSIGNAL);
if (ret < 0)
- return -1;
+ return ret_errno(errno);
return ret;
}
@@ -241,21 +239,19 @@ extern int __netlink_transaction(struct nl_handler *handler,
ret = __netlink_send(handler, request);
if (ret < 0)
- return -1;
+ return ret;
ret = __netlink_recv(handler, answer);
if (ret < 0)
- return -1;
+ return ret;
- ret = 0;
if (answer->nlmsg_type == NLMSG_ERROR) {
struct nlmsgerr *err = (struct nlmsgerr *)NLMSG_DATA(answer);
- errno = -err->error;
if (err->error < 0)
- ret = -1;
+ return ret_errno(-err->error);
}
- return ret;
+ return 0;
}
extern int netlink_transaction(struct nl_handler *handler,
@@ -267,56 +263,44 @@ extern int netlink_transaction(struct nl_handler *handler,
extern int netlink_open(struct nl_handler *handler, int protocol)
{
+ __do_close int fd = -EBADF;
socklen_t socklen;
int sndbuf = 32768;
int rcvbuf = 32768;
- int err;
memset(handler, 0, sizeof(*handler));
+ handler->fd = -EBADF;
- handler->fd = socket(AF_NETLINK, SOCK_RAW | SOCK_CLOEXEC, protocol);
- if (handler->fd < 0)
- return -errno;
+ fd = socket(AF_NETLINK, SOCK_RAW | SOCK_CLOEXEC, protocol);
+ if (fd < 0)
+ return ret_errno(errno);
- if (setsockopt(handler->fd, SOL_SOCKET, SO_SNDBUF,
- &sndbuf, sizeof(sndbuf)) < 0)
- goto err_with_errno;
+ if (setsockopt(fd, SOL_SOCKET, SO_SNDBUF, &sndbuf, sizeof(sndbuf)) < 0)
+ return ret_errno(errno);
- if (setsockopt(handler->fd, SOL_SOCKET, SO_RCVBUF,
- &rcvbuf,sizeof(rcvbuf)) < 0)
- goto err_with_errno;
+ if (setsockopt(fd, SOL_SOCKET, SO_RCVBUF, &rcvbuf,sizeof(rcvbuf)) < 0)
+ return ret_errno(errno);
memset(&handler->local, 0, sizeof(handler->local));
handler->local.nl_family = AF_NETLINK;
handler->local.nl_groups = 0;
- if (bind(handler->fd, (struct sockaddr*)&handler->local,
- sizeof(handler->local)) < 0)
- goto err_with_errno;
+ if (bind(fd, (struct sockaddr*)&handler->local, sizeof(handler->local)) < 0)
+ return ret_errno(errno);
socklen = sizeof(handler->local);
- if (getsockname(handler->fd, (struct sockaddr*)&handler->local,
- &socklen) < 0)
- goto err_with_errno;
+ if (getsockname(fd, (struct sockaddr*)&handler->local, &socklen) < 0)
+ return ret_errno(errno);
- if (socklen != sizeof(handler->local)) {
- err = -EINVAL;
- goto errclose;
- }
+ if (socklen != sizeof(handler->local))
+ return ret_errno(EINVAL);
- if (handler->local.nl_family != AF_NETLINK) {
- err = -EINVAL;
- goto errclose;
- }
+ if (handler->local.nl_family != AF_NETLINK)
+ return ret_errno(EINVAL);
handler->seq = time(NULL);
-
+ handler->fd = move_fd(fd);
return 0;
-err_with_errno:
- err = -errno;
-errclose:
- close(handler->fd);
- return err;
}
extern void netlink_close(struct nl_handler *handler)
@@ -330,9 +314,8 @@ int addattr(struct nlmsghdr *n, size_t maxlen, int type, const void *data,
int len = RTA_LENGTH(alen);
struct rtattr *rta;
- errno = EMSGSIZE;
if (NLMSG_ALIGN(n->nlmsg_len) + RTA_ALIGN(len) > maxlen)
- return -1;
+ return ret_errno(EMSGSIZE);
rta = NLMSG_TAIL(n);
rta->rta_type = type;
From df706de4d39360f491f697a04f049a9170ceda4d Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Fri, 20 Mar 2020 14:04:17 +0100
Subject: [PATCH 2/2] lxc_user_nic: rework device creation
Closes #3058.
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
src/lxc/cmd/lxc_user_nic.c | 133 +++++++++++++++++--------------------
src/lxc/log.h | 14 ++--
2 files changed, 70 insertions(+), 77 deletions(-)
diff --git a/src/lxc/cmd/lxc_user_nic.c b/src/lxc/cmd/lxc_user_nic.c
index 47048456d2..5038d16e0a 100644
--- a/src/lxc/cmd/lxc_user_nic.c
+++ b/src/lxc/cmd/lxc_user_nic.c
@@ -53,6 +53,14 @@
#define usernic_error(format, ...) usernic_debug_stream(stderr, format, __VA_ARGS__)
+#define cmd_error_errno(__ret__, __errno__, format, ...) \
+ ({ \
+ typeof(__ret__) __internal_ret__ = (__ret__); \
+ errno = (__errno__); \
+ CMD_SYSERROR(format, ##__VA_ARGS__); \
+ __internal_ret__; \
+ })
+
__noreturn static void usage(bool fail)
{
fprintf(stderr, "Description:\n");
@@ -493,19 +501,19 @@ static int instantiate_veth(char *veth1, char *veth2, pid_t pid, unsigned int mt
ret = lxc_veth_create(veth1, veth2, pid, mtu);
if (ret < 0) {
- errno = -ret;
CMD_SYSERROR("Failed to create %s-%s\n", veth1, veth2);
- return -1;
+ return ret_errno(-ret);
}
- /* Changing the high byte of the mac address to 0xfe, the bridge
+ /*
+ * Changing the high byte of the mac address to 0xfe, the bridge
* interface will always keep the host's mac address and not take the
* mac address of a container.
*/
ret = setup_private_host_hw_addr(veth1);
if (ret < 0) {
- errno = -ret;
CMD_SYSERROR("Failed to change mac address of host interface %s\n", veth1);
+ return ret_errno(-ret);
}
return netdev_set_flag(veth1, IFF_UP);
@@ -769,13 +777,25 @@ static char *get_nic_if_avail(int fd, struct alloted_s *names, int pid,
return NULL;
}
- if (lxc_pwrite_nointr(fd, newline, slen, length) != slen)
+ if (lxc_pwrite_nointr(fd, newline, slen, length) != slen) {
CMD_SYSERROR("Failed to append new entry \"%s\" to database file", newline);
+ if (lxc_netdev_delete_by_name(nicname) != 0)
+ usernic_error("Error unlinking %s\n", nicname);
+
+ return NULL;
+ }
+
ret = ftruncate(fd, length + slen);
- if (ret < 0)
+ if (ret < 0) {
CMD_SYSERROR("Failed to truncate file\n");
+ if (lxc_netdev_delete_by_name(nicname) != 0)
+ usernic_error("Error unlinking %s\n", nicname);
+
+ return NULL;
+ }
+
return strdup(nicname);
}
@@ -814,113 +834,84 @@ static bool create_db_dir(char *fnam)
static char *lxc_secure_rename_in_ns(int pid, char *oldname, char *newname,
int *container_veth_ifidx)
{
- int ofd, ret;
+ __do_close int fd = -EBADF, ofd = -EBADF;
+ int fret = -1;
+ int ifindex, ret;
pid_t pid_self;
uid_t ruid, suid, euid;
char ifname[IFNAMSIZ];
- char *string_ret = NULL, *name = NULL;
- int fd = -1, ifindex = -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);
- return NULL;
- }
+ if (ofd < 0)
+ return cmd_error_errno(NULL, errno, "Failed opening network namespace path for %d", pid_self);
fd = lxc_preserve_ns(pid, "net");
- if (fd < 0) {
- usernic_error("Failed opening network namespace path for %d", pid);
- goto do_partial_cleanup;
- }
+ if (fd < 0)
+ return cmd_error_errno(NULL, errno, "Failed opening network namespace path for %d", pid);
ret = getresuid(&ruid, &euid, &suid);
- if (ret < 0) {
- CMD_SYSERROR("Failed to retrieve real, effective, and saved user IDs\n");
- goto do_partial_cleanup;
- }
+ if (ret < 0)
+ return cmd_error_errno(NULL, errno, "Failed to retrieve real, effective, and saved user IDs\n");
ret = setns(fd, CLONE_NEWNET);
- close(fd);
- fd = -1;
- if (ret < 0) {
- CMD_SYSERROR("Failed to setns() to the network namespace of the container with PID %d\n",
- pid);
- goto do_partial_cleanup;
- }
+ if (ret < 0)
+ return cmd_error_errno(NULL, errno, "Failed to setns() to the network namespace of the container with PID %d\n", pid);
ret = setresuid(ruid, ruid, 0);
if (ret < 0) {
- 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.
+ 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.
*/
- goto do_full_cleanup;
+ goto out_setns;
}
/* Check if old interface exists. */
ifindex = if_nametoindex(oldname);
if (!ifindex) {
CMD_SYSERROR("Failed to get netdev index\n");
- goto do_full_cleanup;
+ goto out_setresuid;
}
- /* When the IFLA_IFNAME attribute is passed something like "<prefix>%d"
+ /*
+ * When the IFLA_IFNAME attribute is passed something like "<prefix>%d"
* netlink will replace the format specifier with an appropriate index.
* So we pass "eth%d".
*/
- if (newname)
- name = newname;
- else
- name = "eth%d";
-
- ret = lxc_netdev_rename_by_name(oldname, name);
- name = NULL;
+ ret = lxc_netdev_rename_by_name(oldname, newname ? newname : "eth%d");
if (ret < 0) {
- usernic_error("Error %d renaming netdev %s to %s in container\n",
- ret, oldname, newname ? newname : "eth%d");
- goto do_full_cleanup;
+ CMD_SYSERROR("Error %d renaming netdev %s to %s in container\n", ret, oldname, newname ? newname : "eth%d");
+ goto out_setresuid;
}
/* Retrieve new name for interface. */
if (!if_indextoname(ifindex, ifname)) {
CMD_SYSERROR("Failed to get new netdev name\n");
- goto do_full_cleanup;
+ goto out_setresuid;
}
- /* Allocation failure for strdup() is checked below. */
- name = strdup(ifname);
- string_ret = name;
- *container_veth_ifidx = ifindex;
+ fret = 0;
-do_full_cleanup:
+out_setresuid:
ret = setresuid(ruid, euid, suid);
- if (ret < 0) {
- 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;
- }
+ if (ret < 0)
+ return cmd_error_errno(NULL, errno, "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);
+out_setns:
ret = setns(ofd, CLONE_NEWNET);
- if (ret < 0) {
- CMD_SYSERROR("Failed to setns() to original network namespace of PID %d\n", ofd);
- string_ret = NULL;
- }
-
-do_partial_cleanup:
- if (fd >= 0)
- close(fd);
-
- if (!string_ret && name)
- free(name);
+ if (ret < 0)
+ return cmd_error_errno(NULL, errno, "Failed to setns() to original network namespace of PID %d\n", ofd);
- close(ofd);
+ if (fret < 0)
+ return NULL;
- return string_ret;
+ *container_veth_ifidx = ifindex;
+ return strdup(ifname);
}
/* If the caller (real uid, not effective uid) may read the /proc/[pid]/ns/net,
diff --git a/src/lxc/log.h b/src/lxc/log.h
index ec10f53bc5..2b914fdea3 100644
--- a/src/lxc/log.h
+++ b/src/lxc/log.h
@@ -456,13 +456,15 @@ __lxc_unused static inline void LXC_##LEVEL(struct lxc_log_locinfo* locinfo, \
#endif
#if HAVE_M_FORMAT
-#define CMD_SYSERROR(format, ...) \
- fprintf(stderr, "%m - " format, ##__VA_ARGS__)
+#define CMD_SYSERROR(format, ...) \
+ fprintf(stderr, "%m - %s: %d: %s: " format "\n", __FILE__, __LINE__, \
+ __func__, ##__VA_ARGS__);
#else
-#define CMD_SYSERROR(format, ...) \
- do { \
- lxc_log_strerror_r; \
- fprintf(stderr, "%s - " format, ptr, ##__VA_ARGS__); \
+#define CMD_SYSERROR(format, ...) \
+ do { \
+ lxc_log_strerror_r; \
+ fprintf(stderr, "%s - %s: %d: %s: " format "\n", __FILE__, \
+ __LINE__, __func__, ##__VA_ARGS__);
} while (0)
#endif
More information about the lxc-devel
mailing list