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

[Stefano Brivio <sbrivio@redhat.com>]

On Wed, 10 Dec 2025 16:15:51 +1100
David Gibson <david@gibson.dropbear.id.au> 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 <david@gibson.dropbear.id.au>
> ---
>  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 <stdbool.h>
>  #include <syslog.h>
>  
> +/* 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".

-- 
Stefano