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