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=202606 header.b=o+5cdpST; dkim-atps=neutral Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 66C0E5A0262 for ; Tue, 09 Jun 2026 02:39:27 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202606; t=1780965563; bh=Gv0baTQFDNav2Q6rXdRa+4v6xmkP87eaic1nRu/nLYc=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=o+5cdpSTnTTlaD4T8OhM1LSPpfypurNVr7if84JnoPOvTdmOWEq3gNVt9kowSjDvX qlPbEdEccg+SBdXRE34vaEGyKMbx9nkjolcF9pBWr+hR3qJN8kF1nljS985PeYSdU+ KL6xRvEYSXTxAiJ4pRRn3E8Rg/tNQ5yet3HT3SSYvdZytzJB4bh9PCQ9P4I4FRCw6t t+eCczXdkeJcdU7HoDCUzgUyfnPhLo6kDsoEHNFQw6GfRWVNRI1LU5ppLJtpYfspvE XrfCWgN0J3Z0vTyINYj8U88jj0IDdXwePKmFVDRrEbpjHb3s2xq3FUbqWij5qsT/RA aeO63anmtMVRQ== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4gZ95l03P5z4wTK; Tue, 09 Jun 2026 10:39:23 +1000 (AEST) Date: Tue, 9 Jun 2026 10:06:50 +1000 From: David Gibson To: Jon Maloy Subject: Re: [PATCH v2] util, passt: Close daemon-lifetime fds on exit to avoid Coverity warning Message-ID: References: <20260607221942.447370-1-jmaloy@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="kimm7SsvLhMu71fe" Content-Disposition: inline In-Reply-To: <20260607221942.447370-1-jmaloy@redhat.com> Message-ID-Hash: 6XT4TG4J7YWQ34IFNUAYJX357RWQHQ56 X-Message-ID-Hash: 6XT4TG4J7YWQ34IFNUAYJX357RWQHQ56 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: sbrivio@redhat.com, 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: --kimm7SsvLhMu71fe Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sun, Jun 07, 2026 at 06:19:42PM -0400, Jon Maloy wrote: > conf_open_files() opens three file descriptors (fd_tap_listen, > fd_repair_listen, fd_control_listen) that are held for the entire > daemon lifetime. Because no close() call exists for them > anywhere, Coverity flags each as INCOMPLETE_DEALLOCATOR. This is > clearly a false positive, but we still want to get rid of this > warning. >=20 > We now register the execution context in global area so that > passt_exit() can use it to close these descriptors before calling > _exit(). All exit paths (signal handler, die(), die_perror()) > funnel through passt_exit(), so this covers all cases. >=20 > Signed-off-by: Jon Maloy >=20 > --- > v2: - Made struct ctx global from the beginning, on suggestion > from David Gibson. > - Drop pidfile_fd from passt_exit() cleanup, since it can be > closed directly after use. >=20 > Signed-off-by: Jon Maloy Reviewed-by: David Gibson Although one tiny nit.. > --- > passt.c | 77 +++++++++++++++++++++++++++++++-------------------------- > passt.h | 2 ++ > util.c | 7 ++++++ > 3 files changed, 51 insertions(+), 35 deletions(-) >=20 > diff --git a/passt.c b/passt.c > index b3f806b9..e75b80b1 100644 > --- a/passt.c > +++ b/passt.c > @@ -62,6 +62,13 @@ > =20 > char pkt_buf[PKT_BUF_BYTES] __attribute__ ((aligned(PAGE_SIZE))); > =20 > +struct ctx passt_ctx =3D { > + .pidfile_fd =3D -1, > + .fd_tap =3D -1, > + .pasta_netns_fd =3D -1, > + .device_state_fd =3D -1, > +}; > + > char *epoll_type_str[] =3D { > [EPOLL_TYPE_TCP] =3D "connected TCP socket", > [EPOLL_TYPE_TCP_SPLICE] =3D "connected spliced TCP socket", > @@ -322,9 +329,9 @@ static void passt_worker(void *opaque, int nfds, stru= ct epoll_event *events) > */ > int main(int argc, char **argv) > { > + struct ctx *c =3D &passt_ctx; =2E. this line belongs one below according to our reverse christmas tree co= nvention. > struct epoll_event events[NUM_EPOLL_EVENTS]; > int nfds, devnull_fd =3D -1; > - struct ctx c =3D { 0 }; > struct rlimit limit; > struct timespec now; > struct sigaction sa; > @@ -336,18 +343,15 @@ int main(int argc, char **argv) > =20 > isolate_initial(argc, argv); > =20 > - c.pasta_netns_fd =3D c.fd_tap =3D c.pidfile_fd =3D -1; > - c.device_state_fd =3D -1; > - > sigemptyset(&sa.sa_mask); > sa.sa_flags =3D 0; > sa.sa_handler =3D exit_handler; > sigaction(SIGTERM, &sa, NULL); > sigaction(SIGQUIT, &sa, NULL); > =20 > - c.mode =3D conf_mode(argc, argv); > + c->mode =3D conf_mode(argc, argv); > =20 > - if (c.mode =3D=3D MODE_PASTA) { > + if (c->mode =3D=3D MODE_PASTA) { > sa.sa_handler =3D pasta_child_handler; > if (sigaction(SIGCHLD, &sa, NULL)) > die_perror("Couldn't install signal handlers"); > @@ -358,67 +362,70 @@ int main(int argc, char **argv) > =20 > madvise(pkt_buf, sizeof(pkt_buf), MADV_HUGEPAGE); > =20 > - c.epollfd =3D epoll_create1(EPOLL_CLOEXEC); > - if (c.epollfd =3D=3D -1) > + c->epollfd =3D epoll_create1(EPOLL_CLOEXEC); > + if (c->epollfd =3D=3D -1) > die_perror("Failed to create epoll file descriptor"); > - flow_epollid_register(EPOLLFD_ID_DEFAULT, c.epollfd); > + flow_epollid_register(EPOLLFD_ID_DEFAULT, c->epollfd); > =20 > if (getrlimit(RLIMIT_NOFILE, &limit)) > die_perror("Failed to get maximum value of open files limit"); > =20 > - c.nofile =3D limit.rlim_cur =3D limit.rlim_max; > + c->nofile =3D limit.rlim_cur =3D limit.rlim_max; > if (setrlimit(RLIMIT_NOFILE, &limit)) > die_perror("Failed to set current limit for open files"); > =20 > - sock_probe_features(&c); > + sock_probe_features(c); > =20 > - conf(&c, argc, argv); > - trace_init(c.trace); > + conf(c, argc, argv); > + trace_init(c->trace); > =20 > - pasta_netns_quit_init(&c); > + pasta_netns_quit_init(c); > =20 > - tap_backend_init(&c); > + tap_backend_init(c); > =20 > - random_init(&c); > + random_init(c); > =20 > if (clock_gettime(CLOCK_MONOTONIC, &now)) > die_perror("Failed to get CLOCK_MONOTONIC time"); > =20 > flow_init(); > - fwd_scan_ports_init(&c); > + fwd_scan_ports_init(c); > =20 > - if ((!c.no_udp && udp_init(&c)) || (!c.no_tcp && tcp_init(&c))) > + if ((!c->no_udp && udp_init(c)) || (!c->no_tcp && tcp_init(c))) > passt_exit(EXIT_FAILURE); > =20 > - if (fwd_listen_init(&c)) > + if (fwd_listen_init(c)) > passt_exit(EXIT_FAILURE); > =20 > - proto_update_l2_buf(c.guest_mac); > + proto_update_l2_buf(c->guest_mac); > =20 > - if (c.ifi4 && !c.no_dhcp) > + if (c->ifi4 && !c->no_dhcp) > dhcp_init(); > =20 > - if (c.ifi6 && !c.no_dhcpv6) > - dhcpv6_init(&c); > + if (c->ifi6 && !c->no_dhcpv6) > + dhcpv6_init(c); > =20 > - pcap_init(&c); > + pcap_init(c); > =20 > - fwd_neigh_table_init(&c); > - nl_neigh_notify_init(&c); > + fwd_neigh_table_init(c); > + nl_neigh_notify_init(c); > =20 > - if (!c.foreground) { > + if (!c->foreground) { > if ((devnull_fd =3D open("/dev/null", O_RDWR | O_CLOEXEC)) < 0) > die_perror("Failed to open /dev/null"); > } > =20 > - if (isolate_prefork(&c)) > + if (isolate_prefork(c)) > die("Failed to sandbox process, exiting"); > =20 > - if (!c.foreground) { > - __daemon(c.pidfile_fd, devnull_fd); > + if (!c->foreground) { > + __daemon(c->pidfile_fd, devnull_fd); > + close(c->pidfile_fd); > + c->pidfile_fd =3D -1; > log_stderr =3D false; > } else { > - pidfile_write(c.pidfile_fd, getpid()); > + pidfile_write(c->pidfile_fd, getpid()); > + c->pidfile_fd =3D -1; > } > =20 > if (pasta_child_pid) { > @@ -426,19 +433,19 @@ int main(int argc, char **argv) > log_stderr =3D false; > } > =20 > - isolate_postfork(&c); > + isolate_postfork(c); > =20 > - timer_init(&c, &now); > + timer_init(c, &now); > =20 > loop: > /* NOLINTBEGIN(bugprone-branch-clone): intervals can be the same */ > /* cppcheck-suppress [duplicateValueTernary, unmatchedSuppression] */ > - nfds =3D epoll_wait(c.epollfd, events, NUM_EPOLL_EVENTS, TIMER_INTERVAL= ); > + nfds =3D epoll_wait(c->epollfd, events, NUM_EPOLL_EVENTS, TIMER_INTERVA= L); > /* NOLINTEND(bugprone-branch-clone) */ > if (nfds =3D=3D -1 && errno !=3D EINTR) > die_perror("epoll_wait() failed in main loop"); > =20 > - passt_worker(&c, nfds, events); > + passt_worker(c, nfds, events); > =20 > goto loop; > } > diff --git a/passt.h b/passt.h > index 1726965d..e1905f4c 100644 > --- a/passt.h > +++ b/passt.h > @@ -307,6 +307,8 @@ struct ctx { > bool migrate_exit; > }; > =20 > +extern struct ctx passt_ctx; > + > void proto_update_l2_buf(const unsigned char *eth_d); > =20 > #endif /* PASST_H */ > diff --git a/util.c b/util.c > index b64c29ed..ef6ba80e 100644 > --- a/util.c > +++ b/util.c > @@ -1108,6 +1108,13 @@ void abort_with_msg(const char *fmt, ...) > */ > void passt_exit(int status) > { > + if (passt_ctx.fd_tap_listen >=3D 0) > + close(passt_ctx.fd_tap_listen); > + if (passt_ctx.fd_repair_listen >=3D 0) > + close(passt_ctx.fd_repair_listen); > + if (passt_ctx.fd_control_listen >=3D 0) > + close(passt_ctx.fd_control_listen); > + > /* 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 > 2.52.0 >=20 >=20 --=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 --kimm7SsvLhMu71fe Content-Type: application/pgp-signature; name=signature.asc -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmonWQoACgkQzQJF27ox 2GcHqw//UWGuxfgZINp4fK7+2kDB6k2nUhL5I0ecYE3GjWYRWGId6kjDB+3kzXyg fr3dZOs8KNfIPqpvcTKErO377BhJIgBx4L0pXcqgo3YJDng5dcV8STvuIdPHP9oh Wp9+b2CiwC4umLSegMl3LiXsNI2x8fed2w0sHyvW1w8AqiYA1pqNHt0y59sVTRPq OH9/FIpvcK+zuayRKg1wOUq26dmNVejulmfZBkbz87HDe86/3G+C/wxSHeiCg7U5 oMAs22MI6zP4YuVIdFD4jw1KsH8Cky6AcGEhAWsoq/rwsfyYF82FYNSVQR/W2Ul4 EjvmwFd9nuEjvQrH4UhrzywrZrBq7pcQc7kda/oEqzH8d3OQqB+kJqa2Yt5qmz6B CWqDoMcODoVgRZYkcwW3issCfNe88czVQvj3gUZCIS3s936yO3VFUAvjVwUnOhjE mB18J1LTmUdZjgE2dPZrKOJoOcv9V/qg2Za/qaYaikM7WcH6Gd3TfvbNVR7E3lRF IScrxEYuQp4Mz/w0U98mjurLiTq4NU5ayTZ3b4xtEfqie99Xn1k/Cc98lhueJxYg YAKul5VBp6KIC+K/rCMYdjBc6r7yLHBL8WQsXd91sI0SqjPciNjrTGuhuxHrWGHp uZMNydEmqs3umnpmIZ/WHfhMiSW6n/VVCSDUbBe0+yf/oECD4Fg= =wQHT -----END PGP SIGNATURE----- --kimm7SsvLhMu71fe--