Re: [PATCH 1/2] treewide: Introduce passt_exit() helper

On Wed, Dec 10, 2025 at 05:18:41PM +0100, Stefano Brivio wrote:

On Wed, 10 Dec 2025 16:15:51 +1100 David Gibson wrote:

...

In d0006fa78 ("treewide: use _exit() over exit()"), we replaced use of the normal exit(3) with direct calls to _exit(2). That was because glibc exit(3) made some unexpected futex() calls, which hit our seccomp profile.

We've since had some bugs due to missing the extra cleanup that exit(3) implemented, for which we've added explicit cleanup calls. Specifically, we have fflush() calls in some places to avoid leaving incomplete messages on stdout/stderr, and in other places fsync_pcap_and_log() to avoid leaving incomplete log or pcap files.

It's easy to forget these when adding new error paths, so instead, implement our own passt_exit() wrapper to perform vital cleanup then call _exit(2). This also provides an obvious place to add any additional cleanups we discover we need in future.

Signed-off-by: David Gibson --- conf.c | 11 ++++------- log.h | 9 +++++++-- passt.c | 5 ++--- pasta.c | 10 ++++------ tap.c | 6 ++---- util.c | 24 ++++++++++++++++++------ util.h | 1 - vhost_user.c | 4 ++-- 8 files changed, 39 insertions(+), 31 deletions(-)

diff --git a/conf.c b/conf.c index fdc19e81..24b44419 100644 --- a/conf.c +++ b/conf.c @@ -826,7 +826,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(2) */ static void usage(const char *name, FILE *f, int status) { @@ -997,8 +997,7 @@ static void usage(const char *name, FILE *f, int status) " SPEC is as described for TCP above\n" " default: none\n");

- (void)fflush(f); - _exit(status); + passt_exit(status);

pasta_opts:

@@ -1052,8 +1051,7 @@ pasta_opts: " --ns-mac-addr ADDR Set MAC address on tap interface\n" " --no-splice Disable inbound socket splicing\n");

- (void)fflush(f); - _exit(status); + passt_exit(status); }

/** @@ -1621,8 +1619,7 @@ void conf(struct ctx *c, int argc, char **argv) FPRINTF(stdout, c->mode == MODE_PASTA ? "pasta " : "passt "); FPRINTF(stdout, VERSION_BLOB); - (void)fflush(stdout); - _exit(EXIT_SUCCESS); + passt_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 c8473c0d..b7b20672 100644 --- a/log.h +++ b/log.h @@ -9,6 +9,11 @@ #include #include

+/* This would make more sense in util.h, but because we use it in die(), that + * would cause awkward circular reference problems. + */ +void passt_exit(int status) __attribute__((noreturn)); + #define LOGFILE_SIZE_DEFAULT (1024 * 1024UL) #define LOGFILE_CUT_RATIO 30 /* When full, cut ~30% size */ #define LOGFILE_SIZE_MIN (5UL * MAX(BUFSIZ, PAGE_SIZE)) @@ -32,13 +37,13 @@ void logmsg_perror(int pri, const char *format, ...) #define die(...) \ do { \ err(__VA_ARGS__); \ - _exit(EXIT_FAILURE); \ + passt_exit(EXIT_FAILURE); \ } while (0)

#define die_perror(...) \ do { \ err_perror(__VA_ARGS__); \ - _exit(EXIT_FAILURE); \ + passt_exit(EXIT_FAILURE); \ } while (0)

extern int log_file; diff --git a/passt.c b/passt.c index 5ed88d07..cf38822f 100644 --- a/passt.c +++ b/passt.c @@ -177,8 +177,7 @@ static void exit_handler(int signal) { (void)signal;

- fsync_pcap_and_log(); - _exit(EXIT_SUCCESS); + passt_exit(EXIT_SUCCESS); }

/** @@ -399,7 +398,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); + passt_exit(EXIT_FAILURE);

proto_update_l2_buf(c.guest_mac);

diff --git a/pasta.c b/pasta.c index 674b5542..5c693de1 100644 --- a/pasta.c +++ b/pasta.c @@ -70,15 +70,13 @@ void pasta_child_handler(int signal) if (pasta_child_pid && !waitid(P_PID, pasta_child_pid, &infop, WEXITED | WNOHANG)) { if (infop.si_pid == pasta_child_pid) { - fsync_pcap_and_log(); - if (infop.si_code == CLD_EXITED) - _exit(infop.si_status); + passt_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); + passt_exit(infop.si_status + 128);

/* Nothing to do, detached PID namespace going away */ } @@ -511,7 +509,7 @@ void pasta_netns_quit_inotify_handler(struct ctx *c, int inotify_fd)

if (!strncmp(ev->name, c->netns_base, sizeof(c->netns_base))) { info("Namespace %s is gone, exiting", c->netns_base); - _exit(EXIT_SUCCESS); + passt_exit(EXIT_SUCCESS); } } } @@ -539,7 +537,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); + passt_exit(EXIT_SUCCESS); }

close(fd); diff --git a/tap.c b/tap.c index e3ea61cb..9d1344b2 100644 --- a/tap.c +++ b/tap.c @@ -1149,10 +1149,8 @@ void tap_sock_reset(struct ctx *c) { info("Client connection closed%s", c->one_off ? ", exiting" : "");

- if (c->one_off) { - fsync_pcap_and_log(); - _exit(EXIT_SUCCESS); - } + if (c->one_off) + passt_exit(EXIT_SUCCESS);

/* Close the connected socket, wait for a new connection */ epoll_del(c->epollfd, c->fd_tap); diff --git a/util.c b/util.c index bfeb6191..e266c396 100644 --- a/util.c +++ b/util.c @@ -555,7 +555,7 @@ void pidfile_write(int fd, pid_t pid)

if (write(fd, pid_buf, n) < 0) { perror("PID file write"); - _exit(EXIT_FAILURE); + passt_exit(EXIT_FAILURE); }

close(fd); @@ -592,12 +592,12 @@ int __daemon(int pidfile_fd, int devnull_fd)

if (pid == -1) { perror("fork"); - _exit(EXIT_FAILURE); + passt_exit(EXIT_FAILURE); }

if (pid) { pidfile_write(pidfile_fd, pid); - _exit(EXIT_SUCCESS); + passt_exit(EXIT_SUCCESS); }

if (setsid() < 0 || @@ -605,7 +605,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); + passt_exit(EXIT_FAILURE);

return 0; } @@ -1225,17 +1225,29 @@ void abort_with_msg(const char *fmt, ...) }

/** - * fsync_pcap_and_log() - Flush pcap and log files as needed + * passt_exit() - Perform vital cleanup and exit + * + * We don't use exit(3) because on some C library versions it can do unexpected + * things that hit our seccomp profile (e.g. futex() calls). This is a bespoke + * wrapper around _exit(2) performing just the cleanup that we need. * * #syscalls fsync */ -void fsync_pcap_and_log(void) +void passt_exit(int status) { + /* Make sure we don't leave the pcap file truncated */ if (pcap_fd != -1 && fsync(pcap_fd)) warn_perror("Failed to flush pcap file, it might be truncated");

+ /* Make sure we don't leave an incomplete log */ if (log_file != -1) (void)fsync(log_file); + + /* Make sure we dont leave any messages incomplete */

Nit just if you respin: "don't".

Oops, that's mildly embarrassing. I will respin based on your comments for 2/2. -- 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