From 6b7c4477f0dd50bd4a2ad63ca41016d9a4ce9219 Mon Sep 17 00:00:00 2001 From: Andrew Bower Date: Sun, 13 Oct 2024 14:19:09 +0100 Subject: [PATCH] saned: use pidfd to avoid unnecessary frequent wake-ups Avoid waking up every 500ms in standalone mode to reap any terminated child processes if pidfds are available. This benefits Linux users with kernel version 5.3 and glibc 2.36 or later. Introduces a linked list of fds combining listening sockets and pidfds and creates a poll set from this list as required. Child process tracking is now generalised so that the Avahi service is handled in the same way as client connection processes. --- configure.ac | 1 + frontend/saned.c | 381 ++++++++++++++++++++++++++++------------------- 2 files changed, 226 insertions(+), 156 deletions(-) diff --git a/configure.ac b/configure.ac index ddb52b992..6807c6823 100644 --- a/configure.ac +++ b/configure.ac @@ -313,6 +313,7 @@ AC_FUNC_MMAP AC_CHECK_FUNCS(atexit ioperm i386_set_ioperm \ mkdir strftime strstr strtod \ cfmakeraw tcsendbreak strcasecmp strncasecmp _portaccess \ + pidfd_open \ getaddrinfo getnameinfo poll setitimer iopl getuid getpass) dnl sys/io.h might provide ioperm but not inb,outb (like for diff --git a/frontend/saned.c b/frontend/saned.c index 942077aa5..5ca81cc26 100644 --- a/frontend/saned.c +++ b/frontend/saned.c @@ -79,6 +79,9 @@ #include #include +#ifdef HAVE_PIDFD_OPEN +#include +#endif #include #include @@ -168,8 +171,6 @@ poll (struct pollfd *ufds, unsigned int nfds, int timeout) # define SANED_SERVICE_DNS "_sane-port._tcp" # define SANED_NAME "saned" -pid_t avahi_pid = -1; - char *avahi_svc_name; static AvahiClient *avahi_client = NULL; @@ -222,13 +223,36 @@ static AvahiEntryGroup *avahi_group = NULL; # define PATH_MAX 1024 #endif +/* Linked list of child processes */ +enum saned_child_type { + SANED_CHILD_CLIENT, + SANED_CHILD_AVAHI, +}; struct saned_child { + enum saned_child_type type; pid_t pid; + int pidfd; struct saned_child *next; }; struct saned_child *children; int numchildren; +/* Linked list of fds to be polled */ +enum saned_fd_type { + SANED_FD_LISTENER, + SANED_FD_PROCESS, +}; +struct saned_fd +{ + enum saned_fd_type type; + int fd; + short interesting_events; + struct saned_fd *next; +}; +struct saned_fd *saned_fds; +int num_saned_fds; + + #define SANED_CONFIG_FILE "saned.conf" #define SANED_PID_FILE "/var/run/saned.pid" @@ -2295,12 +2319,85 @@ process_request (Wire * w) return 0; } +static int +add_fd (int fd, enum saned_fd_type type, short interesting_events) +{ + struct saned_fd *f; + + f = (struct saned_fd *) malloc (sizeof(struct saned_fd)); + if (f == NULL) + goto fail; + + f->type = type; + f->fd = fd; + f->interesting_events = interesting_events; + f->next = saned_fds; + + saned_fds = f; + num_saned_fds++; + + return fd; + +fail: + DBG (DBG_ERR, "add_fd: cannot manage fd, %s\n", strerror(errno)); + close(fd); + return -1; +} + +static void +close_fds(unsigned type_mask, int specific_fd) +{ + struct saned_fd *f, **nextp; + + for (nextp = &saned_fds; (f = *nextp); /* */) { + if (type_mask & (1 << f->type) || + specific_fd == f->fd) { + close(f->fd); + *nextp = f->next; + num_saned_fds--; + free(f); + } else + nextp = &f->next; + } +} + +static void +add_child (pid_t pid, enum saned_child_type type) +{ + struct saned_child *c; + + c = (struct saned_child *) malloc (sizeof(struct saned_child)); + if (c == NULL) + goto fail; + + c->type = type; + c->pid = pid; + c->pidfd = -1; + c->next = children; + +#ifdef HAVE_PIDFD_OPEN + c->pidfd = pidfd_open(pid, 0); + if (c->pidfd == -1) + DBG (DBG_DBG, "add_child: could not open pidfd for child process, %s\n", strerror(errno)); + else + add_fd(c->pidfd, SANED_FD_PROCESS, POLLIN); +#endif + + children = c; + numchildren++; + return; + +fail: + /* If the child process cannot be managed, kill it now. */ + DBG (DBG_ERR, "add_child: cannot manage child process, %s\n", strerror(errno)); + kill(pid, SIGTERM); +} + static int wait_child (pid_t pid, int *status, int options) { - struct saned_child *c; - struct saned_child *p = NULL; + struct saned_child *c, **nextp; int ret; ret = waitpid(pid, status, options); @@ -2308,57 +2405,19 @@ wait_child (pid_t pid, int *status, int options) if (ret <= 0) return ret; -#if WITH_AVAHI - if ((avahi_pid > 0) && (ret == avahi_pid)) - { - avahi_pid = -1; + for (nextp = &children; (c = *nextp); /* */) { + if (c->pid == ret) { + *nextp = c->next; numchildren--; - return ret; - } -#endif /* WITH_AVAHI */ - - for (c = children; c != NULL; p = c, c = c->next) - { - if (c->pid == ret) - { - if (c == children) - children = c->next; - else if (p != NULL) - p->next = c->next; - - free(c); - - numchildren--; - - break; - } - } - + close_fds(0, c->pidfd); + free(c); + break; + } else + nextp = &c->next; + } return ret; } -static int -add_child (pid_t pid) -{ - struct saned_child *c; - - c = (struct saned_child *) malloc (sizeof(struct saned_child)); - - if (c == NULL) - { - DBG (DBG_ERR, "add_child: out of memory\n"); - return -1; - } - - c->pid = pid; - c->next = children; - - children = c; - numchildren++; - - return 0; -} - static void handle_connection (int fd) @@ -2435,8 +2494,8 @@ handle_client (int fd) else if (pid > 0) { /* parent */ - add_child (pid); close(fd); + add_child (pid, SANED_CHILD_CLIENT); } else { @@ -2446,21 +2505,29 @@ handle_client (int fd) } } +static void +kill_children(int sig) +{ + struct saned_child *c; + + /* The only type of child we kill is Avahi. */ + for (c = children; c; c = c->next) + if (c->type == SANED_CHILD_AVAHI) + kill(c->pid, sig); +} + static void bail_out (int error) { DBG (DBG_ERR, "%sbailing out, waiting for children...\n", (error) ? "FATAL ERROR; " : ""); -#if WITH_AVAHI - if (avahi_pid > 0) - kill (avahi_pid, SIGTERM); -#endif /* WITH_AVAHI */ - + kill_children(SIGTERM); while (numchildren > 0) wait_child (-1, NULL, 0); DBG (DBG_ERR, "bail_out: all children exited\n"); + close_fds(-1, -1); exit ((error) ? 1 : 0); } @@ -2482,7 +2549,7 @@ sig_int_term_handler (int signum) #if WITH_AVAHI static void -saned_avahi (struct pollfd *fds, int nfds); +saned_avahi (void); static void saned_create_avahi_services (AvahiClient *c); @@ -2495,16 +2562,16 @@ saned_avahi_group_callback (AvahiEntryGroup *g, AvahiEntryGroupState state, void static void -saned_avahi (struct pollfd *fds, int nfds) +saned_avahi (void) { - struct pollfd *fdp = NULL; + int avahi_pid; int error; avahi_pid = fork (); if (avahi_pid > 0) { - numchildren++; + add_child(avahi_pid, SANED_CHILD_AVAHI); return; } else if (avahi_pid < 0) @@ -2516,11 +2583,8 @@ saned_avahi (struct pollfd *fds, int nfds) signal (SIGINT, NULL); signal (SIGTERM, NULL); - /* Close network fds */ - for (fdp = fds; nfds > 0; nfds--, fdp++) - close (fdp->fd); - - free(fds); + /* Close parent fds */ + close_fds(-1, -1); avahi_svc_name = avahi_strdup(SANED_NAME); @@ -2855,17 +2919,15 @@ read_config (void) #ifdef SANED_USES_AF_INDEP static void -do_bindings_family (int family, int *nfds, struct pollfd **fds, struct addrinfo *res) +do_bindings_family (int family, struct addrinfo *res) { struct addrinfo *resp; - struct pollfd *fdp; short sane_port; int fd = -1; int on = 1; int i; sane_port = bind_port; - fdp = *fds; for (resp = res, i = 0; resp != NULL; resp = resp->ai_next, i++) { @@ -2955,23 +3017,15 @@ do_bindings_family (int family, int *nfds, struct pollfd **fds, struct addrinfo } } - fdp->fd = fd; - fdp->events = POLLIN; - - (*nfds)++; - fdp++; + add_fd(fd, SANED_FD_LISTENER, POLLIN); } - - *fds = fdp; } static void -do_bindings (int *nfds, struct pollfd **fds) +do_bindings (void) { struct addrinfo *res; - struct addrinfo *resp; struct addrinfo hints; - struct pollfd *fdp; int err; DBG (DBG_DBG, "do_bindings: trying to get port for service \"%s\" (getaddrinfo)\n", SANED_SERVICE_NAME); @@ -2996,31 +3050,15 @@ do_bindings (int *nfds, struct pollfd **fds) } } - for (resp = res, *nfds = 0; resp != NULL; resp = resp->ai_next, (*nfds)++) - ; - - *fds = malloc (*nfds * sizeof (struct pollfd)); - - if (fds == NULL) - { - DBG (DBG_ERR, "do_bindings: not enough memory for fds\n"); - freeaddrinfo (res); - bail_out (1); - } - - fdp = *fds; - *nfds = 0; - /* bind IPv6 first, IPv4 second */ #ifdef ENABLE_IPV6 - do_bindings_family (AF_INET6, nfds, &fdp, res); + do_bindings_family (AF_INET6, res); #endif - do_bindings_family (AF_INET, nfds, &fdp, res); + do_bindings_family (AF_INET, res); - resp = NULL; freeaddrinfo (res); - if (*nfds <= 0) + if (res == NULL) { DBG (DBG_ERR, "do_bindings: couldn't bind an address. Exiting.\n"); bail_out (1); @@ -3030,7 +3068,7 @@ do_bindings (int *nfds, struct pollfd **fds) #else /* !SANED_USES_AF_INDEP */ static void -do_bindings (int *nfds, struct pollfd **fds) +do_bindings (void) { struct sockaddr_in sin; struct servent *serv; @@ -3054,15 +3092,6 @@ do_bindings (int *nfds, struct pollfd **fds) DBG (DBG_WARN, "do_bindings: to your /etc/services file (or equivalent). Proceeding anyway.\n"); } - *nfds = 1; - *fds = malloc (*nfds * sizeof (struct pollfd)); - - if (fds == NULL) - { - DBG (DBG_ERR, "do_bindings: not enough memory for fds\n"); - bail_out (1); - } - memset (&sin, 0, sizeof (sin)); sin.sin_family = AF_INET; @@ -3093,8 +3122,7 @@ do_bindings (int *nfds, struct pollfd **fds) bail_out (1); } - (*fds)->fd = fd; - (*fds)->events = POLLIN; + add_fd(fd, SANED_FD_LISTENER, POLLIN); } #endif /* SANED_USES_AF_INDEP */ @@ -3211,16 +3239,16 @@ runas_user (char *user) static void run_standalone (char *user) { - struct pollfd *fds = NULL; - struct pollfd *fdp = NULL; - int nfds; + struct pollfd *poll_set = NULL; + int poll_set_valid = SANE_FALSE; + int running = SANE_TRUE; int fd = -1; int i; int ret; FILE *pidfile; - do_bindings (&nfds, &fds); + do_bindings (); if (run_foreground == SANE_FALSE) { @@ -3256,7 +3284,11 @@ run_standalone (char *user) else DBG (DBG_ERR, "Could not write PID file: %s\n", strerror (errno)); - chdir ("/"); + if (chdir ("/") != 0) + { + DBG (DBG_ERR, "Could not change to root directory: %s\n", strerror (errno)); + exit(1); + } dup2 (fd, STDIN_FILENO); dup2 (fd, STDOUT_FILENO); @@ -3275,16 +3307,45 @@ run_standalone (char *user) #if WITH_AVAHI DBG (DBG_INFO, "run_standalone: spawning Avahi process\n"); - saned_avahi (fds, nfds); + saned_avahi (); /* NOT REACHED (Avahi process) */ #endif /* WITH_AVAHI */ DBG (DBG_MSG, "run_standalone: waiting for control connection\n"); - while (1) + while (running) { - ret = poll (fds, nfds, 500); + struct saned_child *child; + struct saned_fd *sfd; + int timeout_needed = SANE_FALSE; + int do_rebind = SANE_FALSE; + int do_reap; + + for (child = children; child; child = child->next) + if (child->pidfd == -1) + timeout_needed = SANE_TRUE; + do_reap = timeout_needed; + + if (!poll_set_valid) + { + void *new_poll_set = realloc(poll_set, num_saned_fds * sizeof *poll_set); + if (new_poll_set == NULL && num_saned_fds != 0) + { + DBG (DBG_ERR, "run_standalone: poll set allocation failed: %s\n", strerror (errno)); + free(poll_set); + bail_out (1); + } + poll_set = (struct pollfd *) new_poll_set; + for (sfd = saned_fds, i = 0; sfd; sfd = sfd->next, i++) { + poll_set[i].fd = sfd->fd; + poll_set[i].events = sfd->interesting_events; + } + assert(i == num_saned_fds); + poll_set_valid = SANE_TRUE; + } + + ret = poll (poll_set, num_saned_fds, timeout_needed ? 500 : -1); if (ret < 0) { if (errno == EINTR) @@ -3292,59 +3353,67 @@ run_standalone (char *user) else { DBG (DBG_ERR, "run_standalone: poll failed: %s\n", strerror (errno)); - free (fds); + close_fds(-1, -1); bail_out (1); } } - /* Wait for children */ - while (wait_child (-1, NULL, WNOHANG) > 0) - ; - - if (ret == 0) - continue; - - for (i = 0, fdp = fds; i < nfds; i++, fdp++) + /* Do not allow fd list to change while iterating over poll events, otherwise + * we shall have to look them up each time. */ + for (sfd = saned_fds, i = 0; ret != 0 && i < num_saned_fds; i++, sfd = sfd->next) { - /* Error on an fd */ - if (fdp->revents & (POLLERR | POLLHUP | POLLNVAL)) - { - for (i = 0, fdp = fds; i < nfds; i++, fdp++) - close (fdp->fd); + struct pollfd *pfd = poll_set + i; - free (fds); + assert(sfd); + assert(sfd->fd == pfd->fd); - DBG (DBG_WARN, "run_standalone: invalid fd in set, attempting to re-bind\n"); - - /* Reopen sockets */ - do_bindings (&nfds, &fds); - - break; - } - else if (! (fdp->revents & POLLIN)) + if (pfd->revents == 0) continue; + else + ret--; - fd = accept (fdp->fd, 0, 0); - if (fd < 0) - { - DBG (DBG_ERR, "run_standalone: accept failed: %s\n", strerror (errno)); - continue; + switch (sfd->type) { + case SANED_FD_LISTENER: + if (pfd->revents & (POLLERR | POLLHUP | POLLNVAL)) + do_rebind = SANE_TRUE; + else if (pfd->revents & POLLIN) + { + fd = accept (sfd->fd, 0, 0); + if (fd < 0 && errno != EAGAIN) + DBG (DBG_ERR, "run_standalone: accept failed: %s\n", strerror (errno)); + else if (fd >= 0) + { + handle_client (fd); + if (run_once == SANE_TRUE) + running = SANE_FALSE; /* We have handled the only connection we're going to handle */ + } + } + break; + case SANED_FD_PROCESS: + if (pfd->revents & POLLIN) { + do_reap = SANE_TRUE; + poll_set_valid = SANE_FALSE; /* We will expect to drop a pidfd */ } + } + } - handle_client (fd); - - if (run_once == SANE_TRUE) - break; /* We have handled the only connection we're going to handle */ + if (do_rebind) + { + DBG (DBG_WARN, "run_standalone: invalid fd in set, attempting to re-bind\n"); + close_fds(1 << SANED_FD_LISTENER, -1); + do_bindings (); + poll_set_valid = SANE_FALSE; } - if (run_once == SANE_TRUE) - break; + if (do_reap) + while (wait_child (-1, NULL, WNOHANG) > 0); } - for (i = 0, fdp = fds; i < nfds; i++, fdp++) - close (fdp->fd); - - free (fds); + free(poll_set); + close_fds(-1, -1); + kill_children(SIGTERM); + while (numchildren > 0) + wait_child (-1, NULL, 0); }