From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: passt.top; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=Wc0XAGdr; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by passt.top (Postfix) with ESMTPS id CF88C5A0271 for ; Wed, 10 Dec 2025 17:18:48 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1765383527; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=6M927g4dyWFLT/qIZlOUVBif5dsrGngX97hilcj1oCE=; b=Wc0XAGdros8J5+KJ2c3KiFhiaAXosaUsVaiRJ9nr5bUhfcXVjojP9dGfORJ3A3+UFIV9Mv mmOaABVcMcPaT4I6n3W2Q2QTwvMOkcj3MYN/E7yzbBhmQ0q2pFmq5PhhUZThMWY4AVh9Sq 4nSJlOf8G88sDgYl6YcHBAT95jKNukw= Received: from mail-wm1-f71.google.com (mail-wm1-f71.google.com [209.85.128.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-321-Q-1EG-v9N5m79rE80c1Wjg-1; Wed, 10 Dec 2025 11:18:46 -0500 X-MC-Unique: Q-1EG-v9N5m79rE80c1Wjg-1 X-Mimecast-MFC-AGG-ID: Q-1EG-v9N5m79rE80c1Wjg_1765383525 Received: by mail-wm1-f71.google.com with SMTP id 5b1f17b1804b1-4775d110fabso62277785e9.1 for ; Wed, 10 Dec 2025 08:18:45 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1765383524; x=1765988324; h=content-transfer-encoding:mime-version:organization:references :in-reply-to:message-id:subject:cc:to:from:date:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=6M927g4dyWFLT/qIZlOUVBif5dsrGngX97hilcj1oCE=; b=uhN7e2Tczk6N/pTR7lZnkK4FFq49/PDCwowOCviKYoOCB5evSQCaMHsmacXCAuCwT9 MDnEH4qjunNW2IeLChHgTHGGTqOBc7+ABvlw6GfmO/4rlZ1w6SII5Qji+WjhKXJyMj7x /yfGtVdGi/UTtgTpkd2nd7cei8W1M9+uno8PCwyWrvIDNXyoZRzgtHzyjkk1J94umfM4 THdalCkVmOT3k04ZM/7z95zj8STmoYCOHAfKKzrMOi/sIODAgX3xGDsZ0rQYfXp48EAv DLaBhpfPfL9Tk8PNzCtxFuwFaCKqTv5xSiS5dBrCOul3g0HZxuLHTCR9hh1jt0AF50P9 3TGQ== X-Gm-Message-State: AOJu0YwfzNsiAxm538FRfT67TnVOtnGJeXaRK/KuvJM86U+t5r0/JCLL oIUlDxdz4xxR5kQzQz61JbHwJWQRZmHzBdnzKTRPbOmf4EYienf5bFIORh7EDWTGnCoq+RdvAmw dBrm/w2AP4D2RZCIV1E9CHuXbHr3hsYHh2I0iRRAVwaIIAcSTD/NXDNlhARetMA== X-Gm-Gg: ASbGncuK33PE3cGwSqlowhOzXRuf3Lm38dH+IBFtR0aNNQ+OZbXcBfWwA2mCsKYwU5j BrDzoTa2KbwmuIU2alWEQ/iafctX1cxFEPc4c7CR+E9Os35e6OE7m+/EUzJppIEUdDIGmXtWqre vjatLYHHSJ7GjkNywVi7cdnH/xVk8BuAe2l4UICAr1MxPbP89VND4OCPzCDAD25/xy9BekcAhJO hzRJGUMqbUpHREzuzuu4/ZCW8LikNTWAgnyaQSrqHtitWvgB4aF2Julpb+VEy3g76/pp2TNJX3F k88vM3m0G+eM79Kf94a1mKdCJaYvFpPDQzQTEinuM7tdhLr4hREja9MJ+WylItRdSjBsEFbeARV I1bB6mXtAKO9E3rUQjfQh X-Received: by 2002:a05:600c:1c93:b0:477:1af2:f40a with SMTP id 5b1f17b1804b1-47a8384376bmr33757505e9.17.1765383524375; Wed, 10 Dec 2025 08:18:44 -0800 (PST) X-Google-Smtp-Source: AGHT+IGrEFP4FwM0iPq3KaXY50nmIgCJw/Yl4Ywj2OOK3acKkcv7r1ZIwjLt4iOq0Xc+a440b62Pog== X-Received: by 2002:a05:600c:1c93:b0:477:1af2:f40a with SMTP id 5b1f17b1804b1-47a8384376bmr33757035e9.17.1765383523681; Wed, 10 Dec 2025 08:18:43 -0800 (PST) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [2a10:fc81:a806:d6a9::1]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-47a88210ae6sm1164625e9.8.2025.12.10.08.18.43 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 10 Dec 2025 08:18:43 -0800 (PST) Date: Wed, 10 Dec 2025 17:18:41 +0100 From: Stefano Brivio To: David Gibson Subject: Re: [PATCH 1/2] treewide: Introduce passt_exit() helper Message-ID: <20251210171842.28da019a@elisabeth> In-Reply-To: <20251210051552.1157177-2-david@gibson.dropbear.id.au> References: <20251210051552.1157177-1-david@gibson.dropbear.id.au> <20251210051552.1157177-2-david@gibson.dropbear.id.au> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.49; x86_64-pc-linux-gnu) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: QmgPfBRpuEYudi50gH2AaCO-_DwESkubCgv5dnvtEeY_1765383525 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: O6L4ZVMBRN3ZVS4MVGHPHTPGEN3LEQRY X-Message-ID-Hash: O6L4ZVMBRN3ZVS4MVGHPHTPGEN3LEQRY X-MailFrom: sbrivio@redhat.com 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: 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". -- Stefano