public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
From: Stefano Brivio <sbrivio@redhat.com>
To: David Gibson <david@gibson.dropbear.id.au>
Cc: passt-dev@passt.top, Paul Holzinger <pholzing@redhat.com>
Subject: Re: [PATCH] treewide: Dodge dynamic memory allocation in strerror() from glibc > 2.40
Date: Wed, 11 Dec 2024 01:46:41 +0100	[thread overview]
Message-ID: <20241211014641.27e438ca@elisabeth> (raw)
In-Reply-To: <Z1jcRr6zq-ZfvTK3@zatzit>

On Wed, 11 Dec 2024 11:26:46 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Wed, Dec 11, 2024 at 12:29:09AM +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 <pholzing@redhat.com>
> > Link: https://github.com/containers/podman/issues/24804
> > Analysed-by: Paul Holzinger <pholzing@redhat.com>
> > Signed-off-by: Stefano Brivio <sbrivio@redhat.com>  
> 
> In the short term, this seems like a reasonable fix, except for one
> detail.  Generally '_' prefixed names are reserved for "the system",
> meaning libc in practice.  So I don't think __strerror() is a good
> choice for our interposed function name.  If we can keep the interface
> the same and use macro shannigans to interpose it would be nice if we
> can keep it as 'strerror()' through most of the code.  I think this is
> possible - see the clang tidy workarounds at the bottom of util.h for
> example.

It definitely is, it's even simpler than those as we wouldn't even need
local variables here, but... it's not strerror(), so I don't think we
should call it strerror().

> Otherwise I guess 'strerror_()' or 'passt_strerror()', ugly
> as those are.

Picked strerror_(), passt_strerror() is way too long.

> Longer term, it's not awesome to be ignoring the locale.  Could we use
> the XSI compliant strerror_r() version to get translated messages
> somewhat portably and without allocation?

I gave that a thought, but the XSI compliant version of strerror_r()
needs a user-supplied buffer as well, which would make the callers look
horrible. We could pass in a static buffer otherwise, but then,
confusingly, our own version/wrapper of strerror_r() wouldn't be
thread-safe.

I wonder if ignoring the locale is really that bad. After all, we print
all the messages in English, without localisation. Printing the error
description in other languages is arguably inconsistent.

-- 
Stefano


> 
> > ---
> >  conf.c       | 10 +++++-----
> >  icmp.c       |  4 ++--
> >  log.c        |  2 +-
> >  netlink.c    |  2 +-
> >  pasta.c      | 22 +++++++++++-----------
> >  tcp.c        | 22 +++++++++++-----------
> >  tcp_splice.c | 16 ++++++++--------
> >  udp.c        |  5 +++--
> >  udp_flow.c   |  8 ++++----
> >  util.c       |  6 +++---
> >  util.h       | 32 ++++++++++++++++++++++++++++++++
> >  11 files changed, 81 insertions(+), 48 deletions(-)
> > 
> > diff --git a/conf.c b/conf.c
> > index eaa7d99..6d03111 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..bb0e30b 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..c0eae4f 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..983f45b 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..d3fb36c 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..640b25a 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..d35bfd5 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..0154a1a 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,8 @@ 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..627cc15 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..386bf76 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..b766dc1 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
> >   *  
> 


  reply	other threads:[~2024-12-11  0:46 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-10 23:29 [PATCH] treewide: Dodge dynamic memory allocation in strerror() from glibc > 2.40 Stefano Brivio
2024-12-11  0:26 ` David Gibson
2024-12-11  0:46   ` Stefano Brivio [this message]
2024-12-11  3:55     ` David Gibson
2024-12-11 10:36       ` Paul Holzinger
2024-12-11 12:02         ` Stefano Brivio

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20241211014641.27e438ca@elisabeth \
    --to=sbrivio@redhat.com \
    --cc=david@gibson.dropbear.id.au \
    --cc=passt-dev@passt.top \
    --cc=pholzing@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://passt.top/passt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).