From 067b8335e7add56e64f4b8397fbf775afcfec43d Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Sat, 11 Oct 2025 17:40:48 -0700 Subject: [PATCH] libct/nsenter: add and use bailx We use bail to report fatal errors, and bail always append %m (aka strerror(errno)). In case an error condition did not set errno, the log message will end up with ": Success" or an error from a stale errno value. Either case is confusing for users. Introduce bailx which is the same as bail except it does not append %m, and use it where appropriate. The naming follows libc's err(3) and errx(3). PS we still use bail in a few cases after read or write, even if that read/write did not return an error, because the code does not distinguish between short read/write and error (-1). This will be addressed by the next commit. Signed-off-by: Kir Kolyshkin --- libcontainer/nsenter/log.h | 21 ++++++++++------- libcontainer/nsenter/nsexec.c | 44 +++++++++++++++++++---------------- 2 files changed, 37 insertions(+), 28 deletions(-) diff --git a/libcontainer/nsenter/log.h b/libcontainer/nsenter/log.h index 3e18de687..264e32667 100644 --- a/libcontainer/nsenter/log.h +++ b/libcontainer/nsenter/log.h @@ -26,15 +26,20 @@ bool log_enabled_for(int level); void write_log(int level, const char *format, ...) __attribute__((format(printf, 2, 3))); extern int logfd; -#define bail(fmt, ...) \ - do { \ - if (logfd < 0) \ - fprintf(stderr, "FATAL: " fmt ": %m\n", \ - ##__VA_ARGS__); \ - else \ - write_log(FATAL, fmt ": %m", ##__VA_ARGS__); \ - exit(1); \ + +/* bailx logs a message to logfd (or stderr, if logfd is not available) + * and terminates the program. + */ +#define bailx(fmt, ...) \ + do { \ + if (logfd < 0) \ + fprintf(stderr, "FATAL: " fmt "\n", ##__VA_ARGS__); \ + else \ + write_log(FATAL, fmt, ##__VA_ARGS__); \ + exit(1); \ } while(0) +/* bail is the same as bailx, except it also adds ": %m" (errno). */ +#define bail(fmt, ...) bailx(fmt ": %m", ##__VA_ARGS__) #endif /* NSENTER_LOG_H */ diff --git a/libcontainer/nsenter/nsexec.c b/libcontainer/nsenter/nsexec.c index ad5f42ffe..c923c9743 100644 --- a/libcontainer/nsenter/nsexec.c +++ b/libcontainer/nsenter/nsexec.c @@ -208,7 +208,7 @@ static int try_mapping_tool(const char *app, int pid, char *map, size_t map_len) * or programming issue. */ if (!app) - bail("mapping tool not present"); + bailx("mapping tool not present"); child = fork(); if (child < 0) @@ -274,7 +274,7 @@ static void update_uidmap(const char *path, int pid, char *map, size_t map_len) bail("failed to update /proc/%d/uid_map", pid); write_log(DEBUG, "update /proc/%d/uid_map got -EPERM (trying %s)", pid, path); if (try_mapping_tool(path, pid, map, map_len)) - bail("failed to use newuid map on %d", pid); + bailx("failed to use newuid map on %d", pid); } } @@ -289,7 +289,7 @@ static void update_gidmap(const char *path, int pid, char *map, size_t map_len) bail("failed to update /proc/%d/gid_map", pid); write_log(DEBUG, "update /proc/%d/gid_map got -EPERM (trying %s)", pid, path); if (try_mapping_tool(path, pid, map, map_len)) - bail("failed to use newgid map on %d", pid); + bailx("failed to use newgid map on %d", pid); } } @@ -340,14 +340,16 @@ static void nl_parse(int fd, struct nlconfig_t *config) /* Retrieve the netlink header. */ len = read(fd, &hdr, NLMSG_HDRLEN); + if (len < 0) + bail("failed to read netlink header"); if (len != NLMSG_HDRLEN) - bail("invalid netlink header length %zu", len); + bailx("invalid netlink header length %zu", len); if (hdr.nlmsg_type == NLMSG_ERROR) - bail("failed to read netlink message"); + bailx("failed to read netlink message"); if (hdr.nlmsg_type != INIT_MSG) - bail("unexpected msg type %d", hdr.nlmsg_type); + bailx("unexpected msg type %d", hdr.nlmsg_type); /* Retrieve data. */ size = NLMSG_PAYLOAD(&hdr, 0); @@ -356,8 +358,10 @@ static void nl_parse(int fd, struct nlconfig_t *config) bail("failed to allocate %zu bytes of memory for nl_payload", size); len = read(fd, data, size); + if (len < 0) + bail("failed to read netlink payload"); if (len != size) - bail("failed to read netlink payload, %zu != %zu", len, size); + bailx("failed to read netlink payload, %zu != %zu", len, size); /* Parse the netlink payload. */ config->data = data; @@ -456,7 +460,7 @@ static int nstype(char *name) * without corresponding handling could result in broken behaviour) and * the rest of runc doesn't allow unknown namespace types anyway. */ - bail("unknown namespace type %s", name); + bailx("unknown namespace type %s", name); } static nsset_t __open_namespaces(char *nsspec, struct namespace_t **ns_list, size_t *ns_len) @@ -469,7 +473,7 @@ static nsset_t __open_namespaces(char *nsspec, struct namespace_t **ns_list, siz namespace = strtok_r(nsspec, ",", &saveptr); if (!namespace || !strlen(namespace) || !strlen(nsspec)) - bail("ns paths are empty"); + bailx("ns paths are empty"); do { int fd; @@ -485,7 +489,7 @@ static nsset_t __open_namespaces(char *nsspec, struct namespace_t **ns_list, siz /* Split 'ns:path'. */ path = strstr(namespace, ":"); if (!path) - bail("failed to parse %s", namespace); + bailx("failed to parse %s", namespace); *path++ = '\0'; fd = open(path, O_RDONLY); @@ -530,7 +534,7 @@ static nsset_t __join_namespaces(nsset_t allow, struct namespace_t *ns_list, siz /* Skip permission errors. */ if (saved_errno == EPERM) continue; - bail("failed to setns into %s namespace", ns->type); + bailx("failed to setns into %s namespace: %s", ns->type, strerror(saved_errno)); } joined |= type; @@ -597,7 +601,7 @@ static void __close_namespaces(nsset_t to_join, nsset_t joined, struct namespace /* Make sure we joined the namespaces we planned to. */ if (failed_to_join) - bail("failed to join {%s} namespaces: %s", nsset_to_str(failed_to_join), strerror(EPERM)); + bailx("failed to join {%s} namespaces: %s", nsset_to_str(failed_to_join), strerror(EPERM)); free(ns_list); } @@ -936,7 +940,7 @@ void nsexec(void) stage1_complete = true; break; default: - bail("unexpected sync value: %u", s); + bailx("unexpected sync value: %u", s); } } write_log(DEBUG, "<- stage-1 synchronisation loop"); @@ -967,7 +971,7 @@ void nsexec(void) stage2_complete = true; break; default: - bail("unexpected sync value: %u", s); + bailx("unexpected sync value: %u", s); } } write_log(DEBUG, "<- stage-2 synchronisation loop"); @@ -1058,7 +1062,7 @@ void nsexec(void) if (read(syncfd, &s, sizeof(s)) != sizeof(s)) bail("failed to sync with parent: read(SYNC_USERMAP_ACK)"); if (s != SYNC_USERMAP_ACK) - bail("failed to sync with parent: SYNC_USERMAP_ACK: got %u", s); + bailx("failed to sync with parent: SYNC_USERMAP_ACK: got %u", s); /* Revert temporary re-dumpable setting. */ if (config.namespaces) { @@ -1094,7 +1098,7 @@ void nsexec(void) if (read(syncfd, &s, sizeof(s)) != sizeof(s)) bail("failed to sync with parent: read(SYNC_TIMEOFFSETS_ACK)"); if (s != SYNC_TIMEOFFSETS_ACK) - bail("failed to sync with parent: SYNC_TIMEOFFSETS_ACK: got %u", s); + bailx("failed to sync with parent: SYNC_TIMEOFFSETS_ACK: got %u", s); } /* @@ -1130,7 +1134,7 @@ void nsexec(void) } if (s != SYNC_RECVPID_ACK) { sane_kill(stage2_pid, SIGKILL); - bail("failed to sync with parent: SYNC_RECVPID_ACK: got %u", s); + bailx("failed to sync with parent: SYNC_RECVPID_ACK: got %u", s); } write_log(DEBUG, "signal completion to stage-0"); @@ -1177,7 +1181,7 @@ void nsexec(void) if (read(syncfd, &s, sizeof(s)) != sizeof(s)) bail("failed to sync with parent: read(SYNC_GRANDCHILD)"); if (s != SYNC_GRANDCHILD) - bail("failed to sync with parent: SYNC_GRANDCHILD: got %u", s); + bailx("failed to sync with parent: SYNC_GRANDCHILD: got %u", s); if (setsid() < 0) bail("setsid failed"); @@ -1212,9 +1216,9 @@ void nsexec(void) } break; default: - bail("unexpected jump value"); + bailx("unexpected jump value"); } /* Should never be reached. */ - bail("should never be reached"); + bailx("should never be reached"); }