On Wed, Dec 11, 2024 at 01:46:49AM +0100, Stefano Brivio wrote:

With glibc commit 25a5eb4010df ("string: strerror, strsignal cannot use buffer after dlmopen (bug 32026)"), strerror() now needs, at least on x86, the getrandom() and brk() system calls, in order to fill in the locale-translated error message. But getrandom() and brk() are not allowed by our seccomp profiles.

This became visible on Fedora Rawhide with the "podman login and logout" Podman tests, defined at test/e2e/login_logout_test.go in the Podman source tree, where pasta would terminate upon printing error descriptions (at least the ones related to the SO_ERROR queue for spliced connections).

Avoid dynamic memory allocation by calling strerrordesc_np() instead, which is a GNU function returning a static, untranslated version of the error description. If it's not available, keep calling strerror(), which at that point should be simple enough as to be usable (at least, that's currently the case for musl).

Reported-by: Paul Holzinger Link: https://github.com/containers/podman/issues/24804 Analysed-by: Paul Holzinger Signed-off-by: Stefano Brivio

Reviewed-by: David Gibson

--- v2: Name the wrapper strerror_() instead of __strerror()

conf.c | 10 +++++----- icmp.c | 4 ++-- log.c | 2 +- netlink.c | 2 +- pasta.c | 22 +++++++++++----------- tcp.c | 22 +++++++++++----------- tcp_splice.c | 16 ++++++++-------- udp.c | 4 ++-- udp_flow.c | 8 ++++---- util.c | 6 +++--- util.h | 32 ++++++++++++++++++++++++++++++++ 11 files changed, 80 insertions(+), 48 deletions(-)

diff --git a/conf.c b/conf.c index eaa7d99..43daeca 100644 --- a/conf.c +++ b/conf.c @@ -365,7 +365,7 @@ mode_conflict: die("Port forwarding mode '%s' conflicts with previous mode", optarg); bind_fail: die("Failed to bind port %u (%s) for option '-%c %s', exiting", - i, strerror(-ret), optname, optarg); + i, strerror_(-ret), optname, optarg); bind_all_fail: die("Failed to bind any port for '-%c %s', exiting", optname, optarg); } @@ -655,7 +655,7 @@ static unsigned int conf_ip4(unsigned int ifi, struct ip4_ctx *ip4) &ip4->guest_gw); if (rc < 0) { debug("Couldn't discover IPv4 gateway address: %s", - strerror(-rc)); + strerror_(-rc)); return 0; } } @@ -665,7 +665,7 @@ static unsigned int conf_ip4(unsigned int ifi, struct ip4_ctx *ip4) &ip4->addr, &ip4->prefix_len, NULL); if (rc < 0) { debug("Couldn't discover IPv4 address: %s", - strerror(-rc)); + strerror_(-rc)); return 0; } } @@ -729,7 +729,7 @@ static unsigned int conf_ip6(unsigned int ifi, struct ip6_ctx *ip6) rc = nl_route_get_def(nl_sock, ifi, AF_INET6, &ip6->guest_gw); if (rc < 0) { debug("Couldn't discover IPv6 gateway address: %s", - strerror(-rc)); + strerror_(-rc)); return 0; } } @@ -738,7 +738,7 @@ static unsigned int conf_ip6(unsigned int ifi, struct ip6_ctx *ip6) IN6_IS_ADDR_UNSPECIFIED(&ip6->addr) ? &ip6->addr : NULL, &prefix_len, &ip6->our_tap_ll); if (rc < 0) { - debug("Couldn't discover IPv6 address: %s", strerror(-rc)); + debug("Couldn't discover IPv6 address: %s", strerror_(-rc)); return 0; }

diff --git a/icmp.c b/icmp.c index f514dbc..143e93b 100644 --- a/icmp.c +++ b/icmp.c @@ -85,7 +85,7 @@ void icmp_sock_handler(const struct ctx *c, union epoll_ref ref)

n = recvfrom(ref.fd, buf, sizeof(buf), 0, &sr.sa, &sl); if (n < 0) { - flow_err(pingf, "recvfrom() error: %s", strerror(errno)); + flow_err(pingf, "recvfrom() error: %s", strerror_(errno)); return; }

@@ -301,7 +301,7 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t af, pif_sockaddr(c, &sa, &sl, PIF_HOST, &tgt->eaddr, 0); if (sendto(pingf->sock, pkt, l4len, MSG_NOSIGNAL, &sa.sa, sl) < 0) { flow_dbg(pingf, "failed to relay request to socket: %s", - strerror(errno)); + strerror_(errno)); } else { flow_dbg(pingf, "echo request to socket, ID: %"PRIu16", seq: %"PRIu16, diff --git a/log.c b/log.c index 239c8ce..95e4576 100644 --- a/log.c +++ b/log.c @@ -322,7 +322,7 @@ void logmsg_perror(int pri, const char *format, ...) vlogmsg(false, false, pri, format, ap); va_end(ap);

- logmsg(true, true, pri, ": %s", strerror(errno_copy)); + logmsg(true, true, pri, ": %s", strerror_(errno_copy)); }

/** diff --git a/netlink.c b/netlink.c index 4aba2a3..0407692 100644 --- a/netlink.c +++ b/netlink.c @@ -320,7 +320,7 @@ unsigned int nl_get_ext_if(int s, sa_family_t af) }

if (status < 0) - warn("netlink: RTM_GETROUTE failed: %s", strerror(-status)); + warn("netlink: RTM_GETROUTE failed: %s", strerror_(-status));

if (defifi) { if (ndef > 1) { diff --git a/pasta.c b/pasta.c index 96dacc3..ff41c95 100644 --- a/pasta.c +++ b/pasta.c @@ -296,7 +296,7 @@ void pasta_ns_conf(struct ctx *c) rc = nl_link_set_flags(nl_sock_ns, 1 /* lo */, IFF_UP, IFF_UP); if (rc < 0) die("Couldn't bring up loopback interface in namespace: %s", - strerror(-rc)); + strerror_(-rc));

/* Get or set MAC in target namespace */ if (MAC_IS_ZERO(c->guest_mac)) @@ -305,7 +305,7 @@ void pasta_ns_conf(struct ctx *c) rc = nl_link_set_mac(nl_sock_ns, c->pasta_ifi, c->guest_mac); if (rc < 0) die("Couldn't set MAC address in namespace: %s", - strerror(-rc)); + strerror_(-rc));

if (c->pasta_conf_ns) { unsigned int flags = IFF_UP; @@ -332,7 +332,7 @@ void pasta_ns_conf(struct ctx *c)

if (rc < 0) { die("Couldn't set IPv4 address(es) in namespace: %s", - strerror(-rc)); + strerror_(-rc)); }

if (c->ip4.no_copy_routes) { @@ -346,7 +346,7 @@ void pasta_ns_conf(struct ctx *c)

if (rc < 0) { die("Couldn't set IPv4 route(s) in guest: %s", - strerror(-rc)); + strerror_(-rc)); } }

@@ -355,13 +355,13 @@ void pasta_ns_conf(struct ctx *c) &c->ip6.addr_ll_seen); if (rc < 0) { warn("Can't get LL address from namespace: %s", - strerror(-rc)); + strerror_(-rc)); }

rc = nl_addr_set_ll_nodad(nl_sock_ns, c->pasta_ifi); if (rc < 0) { warn("Can't set nodad for LL in namespace: %s", - strerror(-rc)); + strerror_(-rc)); }

/* We dodged DAD: re-enable neighbour solicitations */ @@ -382,7 +382,7 @@ void pasta_ns_conf(struct ctx *c)

if (rc < 0) { die("Couldn't set IPv6 address(es) in namespace: %s", - strerror(-rc)); + strerror_(-rc)); }

if (c->ip6.no_copy_routes) { @@ -397,7 +397,7 @@ void pasta_ns_conf(struct ctx *c)

if (rc < 0) { die("Couldn't set IPv6 route(s) in guest: %s", - strerror(-rc)); + strerror_(-rc)); } } } @@ -446,18 +446,18 @@ void pasta_netns_quit_init(const struct ctx *c) return;

if ((dir_fd = open(c->netns_dir, O_CLOEXEC | O_RDONLY)) < 0) - die("netns dir open: %s, exiting", strerror(errno)); + die("netns dir open: %s, exiting", strerror_(errno));

if (fstatfs(dir_fd, &s) || s.f_type == DEVPTS_SUPER_MAGIC || s.f_type == PROC_SUPER_MAGIC || s.f_type == SYSFS_MAGIC) try_inotify = false;

if (try_inotify && (fd = inotify_init1(flags)) < 0) - warn("inotify_init1(): %s, use a timer", strerror(errno)); + warn("inotify_init1(): %s, use a timer", strerror_(errno));

if (fd >= 0 && inotify_add_watch(fd, c->netns_dir, IN_DELETE) < 0) { warn("inotify_add_watch(): %s, use a timer", - strerror(errno)); + strerror_(errno)); close(fd); fd = -1; } diff --git a/tcp.c b/tcp.c index 1872ccb..ec433f7 100644 --- a/tcp.c +++ b/tcp.c @@ -516,7 +516,7 @@ static void tcp_timer_ctl(const struct ctx *c, struct tcp_tap_conn *conn) fd = timerfd_create(CLOCK_MONOTONIC, 0); if (fd == -1 || fd > FD_REF_MAX) { flow_dbg(conn, "failed to get timer: %s", - strerror(errno)); + strerror_(errno)); if (fd > -1) close(fd); conn->timer = -1; @@ -526,7 +526,7 @@ static void tcp_timer_ctl(const struct ctx *c, struct tcp_tap_conn *conn)

if (epoll_ctl(c->epollfd, EPOLL_CTL_ADD, conn->timer, &ev)) { flow_dbg(conn, "failed to add timer: %s", - strerror(errno)); + strerror_(errno)); close(conn->timer); conn->timer = -1; return; @@ -551,7 +551,7 @@ static void tcp_timer_ctl(const struct ctx *c, struct tcp_tap_conn *conn) (unsigned long long)it.it_value.tv_nsec / 1000 / 1000);

if (timerfd_settime(conn->timer, 0, &it, NULL)) - flow_err(conn, "failed to set timer: %s", strerror(errno)); + flow_err(conn, "failed to set timer: %s", strerror_(errno)); }

/** @@ -1307,7 +1307,7 @@ int tcp_conn_sock(const struct ctx *c, sa_family_t af) return s;

err("TCP: Unable to open socket for new connection: %s", - strerror(-s)); + strerror_(-s)); return -1; }

@@ -1360,7 +1360,7 @@ static void tcp_bind_outbound(const struct ctx *c, flow_dbg(conn, "Can't bind TCP outbound socket to %s:%hu: %s", inany_ntop(&tgt->oaddr, sstr, sizeof(sstr)), - tgt->oport, strerror(errno)); + tgt->oport, strerror_(errno)); } }

@@ -1371,7 +1371,7 @@ static void tcp_bind_outbound(const struct ctx *c, strlen(c->ip4.ifname_out))) { flow_dbg(conn, "Can't bind IPv4 TCP socket to" " interface %s: %s", c->ip4.ifname_out, - strerror(errno)); + strerror_(errno)); } } } else if (bind_sa.sa_family == AF_INET6) { @@ -1381,7 +1381,7 @@ static void tcp_bind_outbound(const struct ctx *c, strlen(c->ip6.ifname_out))) { flow_dbg(conn, "Can't bind IPv6 TCP socket to" " interface %s: %s", c->ip6.ifname_out, - strerror(errno)); + strerror_(errno)); } } } @@ -2113,7 +2113,7 @@ void tcp_timer_handler(const struct ctx *c, union epoll_ref ref) * and we just set the timer to a new point in the future: discard it. */ if (timerfd_gettime(conn->timer, &check_armed)) - flow_err(conn, "failed to read timer: %s", strerror(errno)); + flow_err(conn, "failed to read timer: %s", strerror_(errno));

if (check_armed.it_value.tv_sec || check_armed.it_value.tv_nsec) return; @@ -2154,7 +2154,7 @@ void tcp_timer_handler(const struct ctx *c, union epoll_ref ref) */ if (timerfd_settime(conn->timer, 0, &new, &old)) flow_err(conn, "failed to set timer: %s", - strerror(errno)); + strerror_(errno));

if (old.it_value.tv_sec == ACT_TIMEOUT) { flow_dbg(conn, "activity timeout"); @@ -2422,13 +2422,13 @@ static void tcp_sock_refill_init(const struct ctx *c) int rc = tcp_sock_refill_pool(c, init_sock_pool4, AF_INET); if (rc < 0) warn("TCP: Error refilling IPv4 host socket pool: %s", - strerror(-rc)); + strerror_(-rc)); } if (c->ifi6) { int rc = tcp_sock_refill_pool(c, init_sock_pool6, AF_INET6); if (rc < 0) warn("TCP: Error refilling IPv6 host socket pool: %s", - strerror(-rc)); + strerror_(-rc)); } }

diff --git a/tcp_splice.c b/tcp_splice.c index 93f8bce..3a0f868 100644 --- a/tcp_splice.c +++ b/tcp_splice.c @@ -160,7 +160,7 @@ static int tcp_splice_epoll_ctl(const struct ctx *c, if (epoll_ctl(c->epollfd, m, conn->s[0], &ev[0]) || epoll_ctl(c->epollfd, m, conn->s[1], &ev[1])) { int ret = -errno; - flow_err(conn, "ERROR on epoll_ctl(): %s", strerror(errno)); + flow_err(conn, "ERROR on epoll_ctl(): %s", strerror_(errno)); return ret; }

@@ -314,7 +314,7 @@ static int tcp_splice_connect_finish(const struct ctx *c, if (conn->pipe[sidei][0] < 0) { if (pipe2(conn->pipe[sidei], O_NONBLOCK | O_CLOEXEC)) { flow_err(conn, "cannot create %d->%d pipe: %s", - sidei, !sidei, strerror(errno)); + sidei, !sidei, strerror_(errno)); conn_flag(c, conn, CLOSING); return -EIO; } @@ -370,7 +370,7 @@ static int tcp_splice_connect(const struct ctx *c, struct tcp_splice_conn *conn) if (connect(conn->s[1], &sa.sa, sl)) { if (errno != EINPROGRESS) { flow_trace(conn, "Couldn't connect socket for splice: %s", - strerror(errno)); + strerror_(errno)); return -errno; }

@@ -469,10 +469,10 @@ void tcp_splice_sock_handler(struct ctx *c, union epoll_ref ref, rc = getsockopt(ref.fd, SOL_SOCKET, SO_ERROR, &err, &sl); if (rc) flow_err(conn, "Error retrieving SO_ERROR: %s", - strerror(errno)); + strerror_(errno)); else flow_trace(conn, "Error event on socket: %s", - strerror(err)); + strerror_(err));

goto close; } @@ -551,7 +551,7 @@ eintr: &lowat, sizeof(lowat))) { flow_trace(conn, "Setting SO_RCVLOWAT %i: %s", - lowat, strerror(errno)); + lowat, strerror_(errno)); } else { conn_flag(c, conn, lowat_set_flag); conn_flag(c, conn, lowat_act_flag); @@ -696,13 +696,13 @@ static int tcp_sock_refill_ns(void *arg) int rc = tcp_sock_refill_pool(c, ns_sock_pool4, AF_INET); if (rc < 0) warn("TCP: Error refilling IPv4 ns socket pool: %s", - strerror(-rc)); + strerror_(-rc)); } if (c->ifi6) { int rc = tcp_sock_refill_pool(c, ns_sock_pool6, AF_INET6); if (rc < 0) warn("TCP: Error refilling IPv6 ns socket pool: %s", - strerror(-rc)); + strerror_(-rc)); }

return 0; diff --git a/udp.c b/udp.c index c89f031..923cc38 100644 --- a/udp.c +++ b/udp.c @@ -453,7 +453,7 @@ static int udp_sock_recverr(int s)

/* TODO: When possible propagate and otherwise handle errors */ debug("%s error on UDP socket %i: %s", - str_ee_origin(ee), s, strerror(ee->ee_errno)); + str_ee_origin(ee), s, strerror_(ee->ee_errno));

return 1; } @@ -492,7 +492,7 @@ int udp_sock_errs(const struct ctx *c, int s, uint32_t events) }

if (err) { - debug("Unqueued error on UDP socket %i: %s", s, strerror(err)); + debug("Unqueued error on UDP socket %i: %s", s, strerror_(err)); n_err++; }

diff --git a/udp_flow.c b/udp_flow.c index c8fdb5f..343caae 100644 --- a/udp_flow.c +++ b/udp_flow.c @@ -95,7 +95,7 @@ static flow_sidx_t udp_flow_new(const struct ctx *c, union flow *flow, if (uflow->s[INISIDE] < 0) { flow_err(uflow, "Couldn't duplicate listening socket: %s", - strerror(errno)); + strerror_(errno)); goto cancel; } } @@ -115,14 +115,14 @@ static flow_sidx_t udp_flow_new(const struct ctx *c, union flow *flow, if (uflow->s[TGTSIDE] < 0) { flow_dbg(uflow, "Couldn't open socket for spliced flow: %s", - strerror(errno)); + strerror_(errno)); goto cancel; }

if (flowside_connect(c, uflow->s[TGTSIDE], tgtpif, tgt) < 0) { flow_dbg(uflow, "Couldn't connect flow socket: %s", - strerror(errno)); + strerror_(errno)); goto cancel; }

@@ -144,7 +144,7 @@ static flow_sidx_t udp_flow_new(const struct ctx *c, union flow *flow, } else if (errno != EAGAIN) { flow_err(uflow, "Unexpected error discarding datagrams: %s", - strerror(errno)); + strerror_(errno)); } }

diff --git a/util.c b/util.c index 55cae3f..11973c4 100644 --- a/util.c +++ b/util.c @@ -90,7 +90,7 @@ int sock_l4_sa(const struct ctx *c, enum epoll_type type,

ret = -errno; if (fd < 0) { - warn("L4 socket: %s", strerror(-ret)); + warn("L4 socket: %s", strerror_(-ret)); return ret; }

@@ -162,7 +162,7 @@ int sock_l4_sa(const struct ctx *c, enum epoll_type type,

if (type == EPOLL_TYPE_TCP_LISTEN && listen(fd, 128) < 0) { ret = -errno; - warn("TCP socket listen: %s", strerror(-ret)); + warn("TCP socket listen: %s", strerror_(-ret)); close(fd); return ret; } @@ -171,7 +171,7 @@ int sock_l4_sa(const struct ctx *c, enum epoll_type type, ev.data.u64 = ref.u64; if (epoll_ctl(c->epollfd, EPOLL_CTL_ADD, fd, &ev) == -1) { ret = -errno; - warn("L4 epoll_ctl: %s", strerror(-ret)); + warn("L4 epoll_ctl: %s", strerror_(-ret)); return ret; }

diff --git a/util.h b/util.h index 41bbd60..3fa1d12 100644 --- a/util.h +++ b/util.h @@ -274,6 +274,38 @@ static inline bool mod_between(unsigned x, unsigned i, unsigned j, unsigned m)

void raw_random(void *buf, size_t buflen);

+/* + * Starting from glibc 2.40.9000 and commit 25a5eb4010df ("string: strerror, + * strsignal cannot use buffer after dlmopen (bug 32026)"), strerror() needs + * getrandom(2) and brk(2) as it allocates memory for the locale-translated + * error description, but our seccomp profiles forbid both. + * + * Use the strerror_() wrapper instead, calling into strerrordesc_np() to get + * a static untranslated string. It's a GNU implementation, but also defined by + * bionic. + * + * If strerrordesc_np() is not defined (e.g. musl), call strerror(). C libraries + * not defining strerrordesc_np() are expected to provide strerror() + * implementations that are simple enough for us to call. + */ +__attribute__ ((weak)) const char *strerrordesc_np(int errnum); + +/** + * strerror_() - strerror() wrapper calling strerrordesc_np() if available + * @errnum: Error code + * + * Return: error description string + */ +static inline const char *strerror_(int errnum) +{ + if (strerrordesc_np) + return strerrordesc_np(errnum); + + return strerror(errnum); +} + +#define strerror(x) @ "Don't call strerror() directly, use strerror_() instead" + /* * Workarounds for https://github.com/llvm/llvm-project/issues/58992 *

-- David Gibson (he or they) | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you, not the other way | around. http://www.ozlabs.org/~dgibson