[PATCH 1/2] treewide: use _exit() over exit()

[PATCH 1/2] treewide: use _exit() over exit()

[Paul Holzinger]

In the podman CI I noticed many seccomp denials in our logs even though
tests passed:
comm="pasta.avx2" exe="/usr/bin/pasta.avx2" sig=31 arch=c000003e
syscall=202 compat=0 ip=0x7fb3d31f69db code=0x80000000

Which is futex being called and blocked by the pasta profile. After a
few tries I managed to reproduce locally with this loop in ~20 min:
while :;
  do podman run -d --network bridge quay.io/libpod/testimage:20241011 \
	sleep 100 && \
  sleep 10 && \
  podman rm -fa -t0
done

And using a pasta version with prctl(PR_SET_DUMPABLE, 1); set I got the
following stack trace:
Stack trace of thread 1:
  #0  0x00007fc95e6de91b __lll_lock_wait_private (libc.so.6 + 0x9491b)
  #1  0x00007fc95e68d6de __run_exit_handlers (libc.so.6 + 0x436de)
  #2  0x00007fc95e68d70e exit (libc.so.6 + 0x4370e)
  #3  0x000055f31b78c50b n/a (n/a + 0x0)
  #4  0x00007fc95e68d70e exit (libc.so.6 + 0x4370e)
  #5  0x000055f31b78d5a2 n/a (n/a + 0x0)

Pasta got killed in exit(), it seems glibc is trying to use a lock when
running exit handlers even though no exit handlers are defined.

Given no exit handlers are needed we can call _exit() instead. This
skips exit handlers and does not flush stdio streams compared to exit()
which should be fine for the use here.

Based on the input from Stefano I did not change the test/doc programs
or qrap as they do not use seccomp filters.

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
---
 conf.c       | 8 ++++----
 log.h        | 4 ++--
 passt.c      | 8 ++++----
 pasta.c      | 8 ++++----
 tap.c        | 2 +-
 util.c       | 8 ++++----
 vhost_user.c | 2 +-
 7 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/conf.c b/conf.c
index df2b016..6817377 100644
--- a/conf.c
+++ b/conf.c
@@ -769,7 +769,7 @@ static void conf_ip6_local(struct ip6_ctx *ip6)
  * usage() - Print usage, exit with given status code
  * @name:	Executable name
  * @f:		Stream to print usage info to
- * @status:	Status code for exit()
+ * @status:	Status code for _exit()
  */
 static void usage(const char *name, FILE *f, int status)
 {
@@ -925,7 +925,7 @@ static void usage(const char *name, FILE *f, int status)
 		"    SPEC is as described for TCP above\n"
 		"    default: none\n");
 
-	exit(status);
+	_exit(status);
 
 pasta_opts:
 
@@ -980,7 +980,7 @@ pasta_opts:
 		"  --ns-mac-addr ADDR	Set MAC address on tap interface\n"
 		"  --no-splice		Disable inbound socket splicing\n");
 
-	exit(status);
+	_exit(status);
 }
 
 /**
@@ -1482,7 +1482,7 @@ void conf(struct ctx *c, int argc, char **argv)
 			FPRINTF(stdout,
 				c->mode == MODE_PASTA ? "pasta " : "passt ");
 			FPRINTF(stdout, VERSION_BLOB);
-			exit(EXIT_SUCCESS);
+			_exit(EXIT_SUCCESS);
 		case 15:
 			ret = snprintf(c->ip4.ifname_out,
 				       sizeof(c->ip4.ifname_out), "%s", optarg);
diff --git a/log.h b/log.h
index a30b091..22c7b9a 100644
--- a/log.h
+++ b/log.h
@@ -32,13 +32,13 @@ void logmsg_perror(int pri, const char *format, ...)
 #define die(...)							\
 	do {								\
 		err(__VA_ARGS__);					\
-		exit(EXIT_FAILURE);					\
+		_exit(EXIT_FAILURE);					\
 	} while (0)
 
 #define die_perror(...)							\
 	do {								\
 		err_perror(__VA_ARGS__);				\
-		exit(EXIT_FAILURE);					\
+		_exit(EXIT_FAILURE);					\
 	} while (0)
 
 extern int log_trace;
diff --git a/passt.c b/passt.c
index b1c8ab6..53fdd38 100644
--- a/passt.c
+++ b/passt.c
@@ -167,7 +167,7 @@ void exit_handler(int signal)
 {
 	(void)signal;
 
-	exit(EXIT_SUCCESS);
+	_exit(EXIT_SUCCESS);
 }
 
 /**
@@ -210,7 +210,7 @@ int main(int argc, char **argv)
 	sigaction(SIGQUIT, &sa, NULL);
 
 	if (argc < 1)
-		exit(EXIT_FAILURE);
+		_exit(EXIT_FAILURE);
 
 	strncpy(argv0, argv[0], PATH_MAX - 1);
 	name = basename(argv0);
@@ -226,7 +226,7 @@ int main(int argc, char **argv)
 	} else if (strstr(name, "passt")) {
 		c.mode = MODE_PASST;
 	} else {
-		exit(EXIT_FAILURE);
+		_exit(EXIT_FAILURE);
 	}
 
 	madvise(pkt_buf, TAP_BUF_BYTES, MADV_HUGEPAGE);
@@ -259,7 +259,7 @@ int main(int argc, char **argv)
 	flow_init();
 
 	if ((!c.no_udp && udp_init(&c)) || (!c.no_tcp && tcp_init(&c)))
-		exit(EXIT_FAILURE);
+		_exit(EXIT_FAILURE);
 
 	proto_update_l2_buf(c.guest_mac, c.our_tap_mac);
 
diff --git a/pasta.c b/pasta.c
index ff41c95..f15084d 100644
--- a/pasta.c
+++ b/pasta.c
@@ -73,12 +73,12 @@ void pasta_child_handler(int signal)
 	    !waitid(P_PID, pasta_child_pid, &infop, WEXITED | WNOHANG)) {
 		if (infop.si_pid == pasta_child_pid) {
 			if (infop.si_code == CLD_EXITED)
-				exit(infop.si_status);
+				_exit(infop.si_status);
 
 			/* If killed by a signal, si_status is the number.
 			 * Follow common shell convention of returning it + 128.
 			 */
-			exit(infop.si_status + 128);
+			_exit(infop.si_status + 128);
 
 			/* Nothing to do, detached PID namespace going away */
 		}
@@ -499,7 +499,7 @@ void pasta_netns_quit_inotify_handler(struct ctx *c, int inotify_fd)
 		return;
 
 	info("Namespace %s is gone, exiting", c->netns_base);
-	exit(EXIT_SUCCESS);
+	_exit(EXIT_SUCCESS);
 }
 
 /**
@@ -525,7 +525,7 @@ void pasta_netns_quit_timer_handler(struct ctx *c, union epoll_ref ref)
 			return;
 
 		info("Namespace %s is gone, exiting", c->netns_base);
-		exit(EXIT_SUCCESS);
+		_exit(EXIT_SUCCESS);
 	}
 
 	close(fd);
diff --git a/tap.c b/tap.c
index 772648f..8c92d23 100644
--- a/tap.c
+++ b/tap.c
@@ -1002,7 +1002,7 @@ void tap_sock_reset(struct ctx *c)
 	info("Client connection closed%s", c->one_off ? ", exiting" : "");
 
 	if (c->one_off)
-		exit(EXIT_SUCCESS);
+		_exit(EXIT_SUCCESS);
 
 	/* Close the connected socket, wait for a new connection */
 	epoll_del(c, c->fd_tap);
diff --git a/util.c b/util.c
index 800c6b5..4d51e04 100644
--- a/util.c
+++ b/util.c
@@ -405,7 +405,7 @@ void pidfile_write(int fd, pid_t pid)
 
 	if (write(fd, pid_buf, n) < 0) {
 		perror("PID file write");
-		exit(EXIT_FAILURE);
+		_exit(EXIT_FAILURE);
 	}
 
 	close(fd);
@@ -441,12 +441,12 @@ int __daemon(int pidfile_fd, int devnull_fd)
 
 	if (pid == -1) {
 		perror("fork");
-		exit(EXIT_FAILURE);
+		_exit(EXIT_FAILURE);
 	}
 
 	if (pid) {
 		pidfile_write(pidfile_fd, pid);
-		exit(EXIT_SUCCESS);
+		_exit(EXIT_SUCCESS);
 	}
 
 	if (setsid()				< 0 ||
@@ -454,7 +454,7 @@ int __daemon(int pidfile_fd, int devnull_fd)
 	    dup2(devnull_fd, STDOUT_FILENO)	< 0 ||
 	    dup2(devnull_fd, STDERR_FILENO)	< 0 ||
 	    close(devnull_fd))
-		exit(EXIT_FAILURE);
+		_exit(EXIT_FAILURE);
 
 	return 0;
 }
diff --git a/vhost_user.c b/vhost_user.c
index 9e38cfd..159f0b3 100644
--- a/vhost_user.c
+++ b/vhost_user.c
@@ -60,7 +60,7 @@ void vu_print_capabilities(void)
 	info("{");
 	info("  \"type\": \"net\"");
 	info("}");
-	exit(EXIT_SUCCESS);
+	_exit(EXIT_SUCCESS);
 }
 
 /**
-- 
@@ -60,7 +60,7 @@ void vu_print_capabilities(void)
 	info("{");
 	info("  \"type\": \"net\"");
 	info("}");
-	exit(EXIT_SUCCESS);
+	_exit(EXIT_SUCCESS);
 }
 
 /**
-- 
2.48.1

[PATCH 2/2] passt-repair: use _exit() over return

[Paul Holzinger]

When returning from main it does the same as calling exit() which is not
good as glibc might try to call futex() which will be blocked by
seccomp. See the prevoius commit "treewide: use _exit() over exit()" for
a more detailed explanation.

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
---
 passt-repair.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/passt-repair.c b/passt-repair.c
index dd8578f..6f79423 100644
--- a/passt-repair.c
+++ b/passt-repair.c
@@ -71,7 +71,7 @@ int main(int argc, char **argv)
 	if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) ||
 	    prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog)) {
 		fprintf(stderr, "Failed to apply seccomp filter");
-		return 1;
+		_exit(1);
 	}
 
 	iov = (struct iovec){ &cmd, sizeof(cmd) };
@@ -80,42 +80,42 @@ int main(int argc, char **argv)
 
 	if (argc != 2) {
 		fprintf(stderr, "Usage: %s PATH\n", argv[0]);
-		return 2;
+		_exit(2);
 	}
 
 	ret = snprintf(a.sun_path, sizeof(a.sun_path), "%s", argv[1]);
 	if (ret <= 0 || ret >= (int)sizeof(a.sun_path)) {
 		fprintf(stderr, "Invalid socket path: %s\n", argv[1]);
-		return 2;
+		_exit(2);
 	}
 
 	if ((s = socket(AF_UNIX, SOCK_STREAM, 0)) < 0) {
 		perror("Failed to create AF_UNIX socket");
-		return 1;
+		_exit(1);
 	}
 
 	if (connect(s, (struct sockaddr *)&a, sizeof(a))) {
 		fprintf(stderr, "Failed to connect to %s: %s\n", argv[1],
 			strerror(errno));
-		return 1;
+		_exit(1);
 	}
 
 loop:
 	ret = recvmsg(s, &msg, 0);
 	if (ret < 0) {
 		perror("Failed to receive message");
-		return 1;
+		_exit(1);
 	}
 
 	if (!ret)	/* Done */
-		return 0;
+		_exit(0);
 
 	if (!cmsg ||
 	    cmsg->cmsg_len < CMSG_LEN(sizeof(int)) ||
 	    cmsg->cmsg_len > CMSG_LEN(sizeof(int) * SCM_MAX_FD) ||
 	    cmsg->cmsg_type != SCM_RIGHTS) {
 		fprintf(stderr, "No/bad ancillary data from peer\n");
-		return 1;
+		_exit(1);
 	}
 
 	n = cmsg->cmsg_len / CMSG_LEN(sizeof(int));
@@ -124,7 +124,7 @@ loop:
 	if (cmd != TCP_REPAIR_ON && cmd != TCP_REPAIR_OFF &&
 	    cmd != TCP_REPAIR_OFF_NO_WP) {
 		fprintf(stderr, "Unsupported command 0x%04x\n", cmd);
-		return 1;
+		_exit(1);
 	}
 
 	for (i = 0; i < n; i++) {
@@ -134,7 +134,7 @@ loop:
 			fprintf(stderr,
 				"Setting TCP_REPAIR to %i on socket %i: %s", o,
 				fds[i], strerror(errno));
-			return 1;
+			_exit(1);
 		}
 
 		/* Close _our_ copy */
@@ -144,11 +144,11 @@ loop:
 		if (send(s, &cmd, sizeof(cmd), 0) < 0) {
 			fprintf(stderr, "Reply to command %i: %s\n",
 				o, strerror(errno));
-			return 1;
+			_exit(1);
 		}
 	}
 
 	goto loop;
 
-	return 0;
+	_exit(0);
 }
-- 
@@ -71,7 +71,7 @@ int main(int argc, char **argv)
 	if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) ||
 	    prctl(PR_SET_SECCOMP, SECCOMP_MODE_FILTER, &prog)) {
 		fprintf(stderr, "Failed to apply seccomp filter");
-		return 1;
+		_exit(1);
 	}
 
 	iov = (struct iovec){ &cmd, sizeof(cmd) };
@@ -80,42 +80,42 @@ int main(int argc, char **argv)
 
 	if (argc != 2) {
 		fprintf(stderr, "Usage: %s PATH\n", argv[0]);
-		return 2;
+		_exit(2);
 	}
 
 	ret = snprintf(a.sun_path, sizeof(a.sun_path), "%s", argv[1]);
 	if (ret <= 0 || ret >= (int)sizeof(a.sun_path)) {
 		fprintf(stderr, "Invalid socket path: %s\n", argv[1]);
-		return 2;
+		_exit(2);
 	}
 
 	if ((s = socket(AF_UNIX, SOCK_STREAM, 0)) < 0) {
 		perror("Failed to create AF_UNIX socket");
-		return 1;
+		_exit(1);
 	}
 
 	if (connect(s, (struct sockaddr *)&a, sizeof(a))) {
 		fprintf(stderr, "Failed to connect to %s: %s\n", argv[1],
 			strerror(errno));
-		return 1;
+		_exit(1);
 	}
 
 loop:
 	ret = recvmsg(s, &msg, 0);
 	if (ret < 0) {
 		perror("Failed to receive message");
-		return 1;
+		_exit(1);
 	}
 
 	if (!ret)	/* Done */
-		return 0;
+		_exit(0);
 
 	if (!cmsg ||
 	    cmsg->cmsg_len < CMSG_LEN(sizeof(int)) ||
 	    cmsg->cmsg_len > CMSG_LEN(sizeof(int) * SCM_MAX_FD) ||
 	    cmsg->cmsg_type != SCM_RIGHTS) {
 		fprintf(stderr, "No/bad ancillary data from peer\n");
-		return 1;
+		_exit(1);
 	}
 
 	n = cmsg->cmsg_len / CMSG_LEN(sizeof(int));
@@ -124,7 +124,7 @@ loop:
 	if (cmd != TCP_REPAIR_ON && cmd != TCP_REPAIR_OFF &&
 	    cmd != TCP_REPAIR_OFF_NO_WP) {
 		fprintf(stderr, "Unsupported command 0x%04x\n", cmd);
-		return 1;
+		_exit(1);
 	}
 
 	for (i = 0; i < n; i++) {
@@ -134,7 +134,7 @@ loop:
 			fprintf(stderr,
 				"Setting TCP_REPAIR to %i on socket %i: %s", o,
 				fds[i], strerror(errno));
-			return 1;
+			_exit(1);
 		}
 
 		/* Close _our_ copy */
@@ -144,11 +144,11 @@ loop:
 		if (send(s, &cmd, sizeof(cmd), 0) < 0) {
 			fprintf(stderr, "Reply to command %i: %s\n",
 				o, strerror(errno));
-			return 1;
+			_exit(1);
 		}
 	}
 
 	goto loop;
 
-	return 0;
+	_exit(0);
 }
-- 
2.48.1

Re: [PATCH 2/2] passt-repair: use _exit() over return

[Stefano Brivio]

On Wed,  5 Feb 2025 14:00:42 +0100
Paul Holzinger <pholzing@redhat.com> wrote:

> When returning from main it does the same as calling exit() which is not
> good as glibc might try to call futex() which will be blocked by
> seccomp. See the prevoius commit "treewide: use _exit() over exit()" for
> a more detailed explanation.
> 
> Signed-off-by: Paul Holzinger <pholzing@redhat.com>

Applied.

-- 
Stefano

Re: [PATCH 1/2] treewide: use _exit() over exit()

[Stefano Brivio]

On Wed,  5 Feb 2025 14:00:41 +0100
Paul Holzinger <pholzing@redhat.com> wrote:

> In the podman CI I noticed many seccomp denials in our logs even though
> tests passed:
> comm="pasta.avx2" exe="/usr/bin/pasta.avx2" sig=31 arch=c000003e
> syscall=202 compat=0 ip=0x7fb3d31f69db code=0x80000000
> 
> Which is futex being called and blocked by the pasta profile. After a
> few tries I managed to reproduce locally with this loop in ~20 min:
> while :;
>   do podman run -d --network bridge quay.io/libpod/testimage:20241011 \
> 	sleep 100 && \
>   sleep 10 && \
>   podman rm -fa -t0
> done
> 
> And using a pasta version with prctl(PR_SET_DUMPABLE, 1); set I got the
> following stack trace:
> Stack trace of thread 1:
>   #0  0x00007fc95e6de91b __lll_lock_wait_private (libc.so.6 + 0x9491b)
>   #1  0x00007fc95e68d6de __run_exit_handlers (libc.so.6 + 0x436de)
>   #2  0x00007fc95e68d70e exit (libc.so.6 + 0x4370e)
>   #3  0x000055f31b78c50b n/a (n/a + 0x0)
>   #4  0x00007fc95e68d70e exit (libc.so.6 + 0x4370e)
>   #5  0x000055f31b78d5a2 n/a (n/a + 0x0)
> 
> Pasta got killed in exit(), it seems glibc is trying to use a lock when
> running exit handlers even though no exit handlers are defined.
> 
> Given no exit handlers are needed we can call _exit() instead. This
> skips exit handlers and does not flush stdio streams compared to exit()
> which should be fine for the use here.
> 
> Based on the input from Stefano I did not change the test/doc programs
> or qrap as they do not use seccomp filters.
> 
> Signed-off-by: Paul Holzinger <pholzing@redhat.com>

Applied.

-- 
Stefano