From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=gibson.dropbear.id.au Authentication-Results: passt.top; dkim=pass (2048-bit key; secure) header.d=gibson.dropbear.id.au header.i=@gibson.dropbear.id.au header.a=rsa-sha256 header.s=202512 header.b=l87efdLe; dkim-atps=neutral Received: from mail.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id AF4CD5A0626 for ; Thu, 11 Dec 2025 00:45:50 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202512; t=1765410348; bh=UaK8OfHGoQnM2db/PsNuIaOSjNnhE3T/OfqiliEeiic=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=l87efdLe5xV2wINP7KH81LpdoHHKIMBF+bVoS/DIrBkhiiFzg2Hi4oveKufz+/ZIv JXlq+8IJ2EpZRuNfJhpS3ef8XUsrCeyP2U6tCnYHLwWCZyVnHVbCtFIjb91wYqxkka sHvA9qBxIuVAakuZaN6dyPdoer7ndQ8kQqcPa7GaxstGcCbupyECvyzv4xW3GAjKIF TOlb/67WNnxnQYszqU2lNTp4doq6cpso3HQq/mwoD3y75pdI/4AmDqafR+NBGz7vh8 YRvwF5R+8ob9T1EusMCGLW36HPs6ctVy+PGWPMDliXnU8Qd83/H9Ytvho0STqDILrh ckcWh1XTS20DA== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4dRXR02hwpz4wHj; Thu, 11 Dec 2025 10:45:48 +1100 (AEDT) Date: Thu, 11 Dec 2025 10:32:50 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH 1/2] treewide: Introduce passt_exit() helper Message-ID: References: <20251210051552.1157177-1-david@gibson.dropbear.id.au> <20251210051552.1157177-2-david@gibson.dropbear.id.au> <20251210171842.28da019a@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="a+W4KR4NGN7rWeCB" Content-Disposition: inline In-Reply-To: <20251210171842.28da019a@elisabeth> Message-ID-Hash: 4D2PK27IAKUROGGYOE62VWVMXYH47IH5 X-Message-ID-Hash: 4D2PK27IAKUROGGYOE62VWVMXYH47IH5 X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --a+W4KR4NGN7rWeCB Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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: >=20 > > In d0006fa78 ("treewide: use _exit() over exit()"), we replaced use of > > the normal exit(3) with direct calls to _exit(2). That was because gli= bc > > exit(3) made some unexpected futex() calls, which hit our seccomp profi= le. > >=20 > > We've since had some bugs due to missing the extra cleanup that exit(3) > > implemented, for which we've added explicit cleanup calls. Specificall= y, > > we have fflush() calls in some places to avoid leaving incomplete messa= ges > > on stdout/stderr, and in other places fsync_pcap_and_log() to avoid > > leaving incomplete log or pcap files. > >=20 > > It's easy to forget these when adding new error paths, so instead, > > implement our own passt_exit() wrapper to perform vital cleanup then ca= ll > > _exit(2). This also provides an obvious place to add any additional > > cleanups we discover we need in future. > >=20 > > 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(-) > >=20 > > 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 st= atus) > > " SPEC is as described for TCP above\n" > > " default: none\n"); > > =20 > > - (void)fflush(f); > > - _exit(status); > > + passt_exit(status); > > =20 > > pasta_opts: > > =20 > > @@ -1052,8 +1051,7 @@ pasta_opts: > > " --ns-mac-addr ADDR Set MAC address on tap interface\n" > > " --no-splice Disable inbound socket splicing\n"); > > =20 > > - (void)fflush(f); > > - _exit(status); > > + passt_exit(status); > > } > > =20 > > /** > > @@ -1621,8 +1619,7 @@ void conf(struct ctx *c, int argc, char **argv) > > FPRINTF(stdout, > > c->mode =3D=3D MODE_PASTA ? "pasta " : "passt "); > > FPRINTF(stdout, VERSION_BLOB); > > - (void)fflush(stdout); > > - _exit(EXIT_SUCCESS); > > + passt_exit(EXIT_SUCCESS); > > case 15: > > ret =3D 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 > > =20 > > +/* 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) > > =20 > > #define die_perror(...) \ > > do { \ > > err_perror(__VA_ARGS__); \ > > - _exit(EXIT_FAILURE); \ > > + passt_exit(EXIT_FAILURE); \ > > } while (0) > > =20 > > 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; > > =20 > > - fsync_pcap_and_log(); > > - _exit(EXIT_SUCCESS); > > + passt_exit(EXIT_SUCCESS); > > } > > =20 > > /** > > @@ -399,7 +398,7 @@ int main(int argc, char **argv) > > flow_init(); > > =20 > > if ((!c.no_udp && udp_init(&c)) || (!c.no_tcp && tcp_init(&c))) > > - _exit(EXIT_FAILURE); > > + passt_exit(EXIT_FAILURE); > > =20 > > proto_update_l2_buf(c.guest_mac); > > =20 > > 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 =3D=3D pasta_child_pid) { > > - fsync_pcap_and_log(); > > - > > if (infop.si_code =3D=3D CLD_EXITED) > > - _exit(infop.si_status); > > + passt_exit(infop.si_status); > > =20 > > /* 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); > > =20 > > /* Nothing to do, detached PID namespace going away */ > > } > > @@ -511,7 +509,7 @@ void pasta_netns_quit_inotify_handler(struct ctx *c= , int inotify_fd) > > =20 > > 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; > > =20 > > info("Namespace %s is gone, exiting", c->netns_base); > > - _exit(EXIT_SUCCESS); > > + passt_exit(EXIT_SUCCESS); > > } > > =20 > > 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" : ""); > > =20 > > - if (c->one_off) { > > - fsync_pcap_and_log(); > > - _exit(EXIT_SUCCESS); > > - } > > + if (c->one_off) > > + passt_exit(EXIT_SUCCESS); > > =20 > > /* 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) > > =20 > > if (write(fd, pid_buf, n) < 0) { > > perror("PID file write"); > > - _exit(EXIT_FAILURE); > > + passt_exit(EXIT_FAILURE); > > } > > =20 > > close(fd); > > @@ -592,12 +592,12 @@ int __daemon(int pidfile_fd, int devnull_fd) > > =20 > > if (pid =3D=3D -1) { > > perror("fork"); > > - _exit(EXIT_FAILURE); > > + passt_exit(EXIT_FAILURE); > > } > > =20 > > if (pid) { > > pidfile_write(pidfile_fd, pid); > > - _exit(EXIT_SUCCESS); > > + passt_exit(EXIT_SUCCESS); > > } > > =20 > > 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); > > =20 > > return 0; > > } > > @@ -1225,17 +1225,29 @@ void abort_with_msg(const char *fmt, ...) > > } > > =20 > > /** > > - * 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 u= nexpected > > + * 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 !=3D -1 && fsync(pcap_fd)) > > warn_perror("Failed to flush pcap file, it might be truncated"); > > =20 > > + /* Make sure we don't leave an incomplete log */ > > if (log_file !=3D -1) > > (void)fsync(log_file); > > + > > + /* Make sure we dont leave any messages incomplete */ >=20 > Nit just if you respin: "don't". Oops, that's mildly embarrassing. I will respin based on your comments for 2/2. --=20 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 --a+W4KR4NGN7rWeCB Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmk6AxIACgkQzQJF27ox 2GdtTQ//X8VQBjSTfsxBIeAnoZhGj+oQvvdQNsWHYCT9u6AxERcGSGG8hg+tkeAj ipdMnJnzPaoQX8vEJCnvulYGeNgwWutVLcBJbagURydQR03V0H3a1YbbmK+YR6EK wHcwj3MvfJ8rlFYwszhAVpSkiqp+x8NlpGhGno/yZZl8P7DWF1bFA4x8mbFFcnWG ZJma+C6dlUSsdp0Ikp87WikRwtC55K9EGjUWTRGjhfFMS1T/mRzj8RIw/JmWpU49 RYVjHat/clhDztxmSQtbEFgBAXP/V/2d7rVByk0iEqnoKiAUMeHI67PToh20fMnH ZQKZt9lHKQZkCG6J1U8MXR2gkZNZaNBSWNVQl2dtUybA/qNv6bxlTDPPBRqpDQQV UkI7iFDjHLN+GwVTkCGF/qab1PZVqV2Zph2XkkUC2VAbcdr1LO6uvR+UdBccf6Y9 paHhjTw0Pvzy9SGKKNfMvYavt4fD5aCtxFsQe6nnsulnlXX5071Nsi60Hf9s0GvK /7lLwCAsCkSYdC7h+GvdLcHdivyLLI68xCLpoOlKhUskQcFYHDaWgqKkMPWw7mnu ubqJELQG9rbVwiRapVWsHYutQkWUzYheLl52MZz1hWDZ7yltmKAtgs6FQ6wMXk6e I+YkmwabipZItpT1dxIkby5dqjvpI4S7qXp8Hiug9k3EhOzPRHw= =/exO -----END PGP SIGNATURE----- --a+W4KR4NGN7rWeCB--