On Sun, May 31, 2026 at 04:13:24PM -0400, Jon Maloy wrote:

conf_open_files() opens four file descriptors (fd_tap_listen, fd_repair_listen, pidfile_fd, 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.

A bit frustrating that Coverity doesn't recognize an exit() will close all remaining files, but ok. I'm ok with close()ing these at exit... but some details remain. First, for pidfile_fd specifically, we shouldn't need that after we've written the pid. So we should just close() at that point, rather than waiting until exit.

We now register the execution context so that passt_exit() can use to close these descriptors before calling _exit(). All exit paths (signal handler, die(), die_perror()) funnel through passt_exit(), so this covers all cases.

passt_exit() is the right place for this, but registering an actual global to point at a local that's sort of a global seems pretty perverse. It would make more sense to convert the main struct ctx itself to a global, rather than a main()-local. Note that I'm not suggesting changing all the places that use it (current or future) - I think it's still preferable for those (excepting I guess main() itself) to access it via a parameter. We can encourage that by using a bulky name for the global. I see that setting 'exit_cleanup_ctx' where you do also serves one other purpose - it stops early passt_exit()s from attempting to close() those fds before they're initialised, even to -1. We can fix that by using an initialiser on the global to set fd fields to -1 before main(), which is arguably more elegant than the cluster of assignments at the top of main() anyway.

Signed-off-by: Jon Maloy --- log.h | 2 ++ passt.c | 1 + util.c | 22 ++++++++++++++++++++++ 3 files changed, 25 insertions(+)

diff --git a/log.h b/log.h index 69cfb507..079f429c 100644 --- a/log.h +++ b/log.h @@ -63,7 +63,9 @@ extern bool debug_flag; /* This would make more sense in util.h, but because we use it in die(), that * would cause awkward circular reference problems. */ +struct ctx; void passt_exit(int status) __attribute__((noreturn)); +void passt_exit_set_ctx(struct ctx *c);

#define LOGFILE_SIZE_DEFAULT (1024 * 1024UL) #define LOGFILE_CUT_RATIO 30 /* When full, cut ~30% size */ diff --git a/passt.c b/passt.c index b6fc12d4..ec6aa57a 100644 --- a/passt.c +++ b/passt.c @@ -392,6 +392,7 @@ int main(int argc, char **argv) sock_probe_features(&c);

conf(&c, argc, argv); + passt_exit_set_ctx(&c); trace_init(c.trace);

pasta_netns_quit_init(&c); diff --git a/util.c b/util.c index b64c29ed..f15b1f9a 100644 --- a/util.c +++ b/util.c @@ -1097,6 +1097,17 @@ void abort_with_msg(const char *fmt, ...) abort(); }

+static struct ctx *exit_cleanup_ctx; + +/** + * passt_exit_set_ctx() - Register context for cleanup on exit + * @c: Execution context + */ +void passt_exit_set_ctx(struct ctx *c) +{ + exit_cleanup_ctx = c; +} + /** * passt_exit() - Perform vital cleanup and exit * @@ -1108,6 +1119,17 @@ void abort_with_msg(const char *fmt, ...) */ void passt_exit(int status) { + if (exit_cleanup_ctx) { + if (exit_cleanup_ctx->fd_tap_listen >= 0) + close(exit_cleanup_ctx->fd_tap_listen); + if (exit_cleanup_ctx->fd_repair_listen >= 0) + close(exit_cleanup_ctx->fd_repair_listen); + if (exit_cleanup_ctx->pidfile_fd >= 0) + close(exit_cleanup_ctx->pidfile_fd); + if (exit_cleanup_ctx->fd_control_listen >= 0) + close(exit_cleanup_ctx->fd_control_listen); + } + /* 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"); -- 2.52.0

-- 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