From nobody Sun Apr 09 23:58:01 2023 X-Original-To: dev-commits-src-branches@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4PvpvZ3QX3z44STr; Sun, 9 Apr 2023 23:58:02 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4PvpvZ16Hsz4Fg6; Sun, 9 Apr 2023 23:58:02 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1681084682; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=m37QgbMBU0NL3uS2/TUFXormvDW2b0E+stOTFW+Gmos=; b=ub2sIdyKoM3wzNxaTnsEL+x5N6V59NmXLPiM9WsEcmgT6XIOrm9zB2ja9V3vx64hDZ1zkZ 1RebUZB5VLBENqKhrcVsrSL3mqXBAFrpbp8x1sL/tbeN45PUO2QhFyGX5rkKRRTrrrCFqj VEkPuSD3fSViP2HbnGBNAPtILCTKeqVp5K0gPFUWTJyN89Mklnjnrh+tIXtBX774cn4dKr ckUyzvY5C0QTncDzyYzOwLRZlzk2lhLLHtugfKVg2MnL/0FThBLC27NTTuLZDye34r3eJX cJO2FdMJB2PW0qINdEbHKEYqG1ggZNlKtT00G3p7iClvOPs7YxPnHakM8rvQsQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1681084682; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=m37QgbMBU0NL3uS2/TUFXormvDW2b0E+stOTFW+Gmos=; b=K/tp4qrR1Qxpu5mtmUxZjekasvsxwQ0ce/jRgcVj1dRoZBm2jpOPRgrLFz+3MtSEGKr3da ksc2XErLtSOotv368iuHgMKtk94Ilz2P6JOKGlMqJ1pCkAXAwGhpp4ZjI/jjM0AUyH57aN 5fvrFlHk0JI4K75CIcoXk1O9m0OgyQAvoLuMEWHsWbwkGAryX2NWKBZXU/yFGKvDd1hv2O RNDJuxbfRDhILe+EjOs1+vWlTq35u3VowbYwbJvImkL5nPt7zY7Cr4OMMkJ/P7p9u2q7Kd 7rudrtOsKQ6+vxIr1wV/0zqjmKWr3hKdvdo/hFmXpR7X64cYGi+Goj2SncYNUQ== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1681084682; a=rsa-sha256; cv=none; b=g4eQr2QBJKJ9ki74SPF2yRqqQPgI7cpXPYdjlqvO+3+I139DVQ6HMWuGGsZRsuxctwCQ8X lU60U76Lp9ztvqTyZ5hB2cG7XIrrVENTKV17UTNsllwZMQcdX6mIlwqNggsJVJBzHve5iN p1LDkwwPawyaq97fitRqU8N980LlEWlkoxW/ebqKyK9yQPxA4oliyfzCGPqjc9xrKB7tAx 8IOdiwqFc+Sh3pYrV5RRz4t+84mxkIXEfYmNFHzURyRjPb1OD+m9hHL1ogUyJpd7EIAKp0 R0JCz9fTQ6UzgxxYlp+THOFsgfSSJ7rpmfUy17KA6ejUOCxnbf9vlQrcxbcOHA== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4PvpvY3PHLzSmp; Sun, 9 Apr 2023 23:58:01 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 339Nw1jl012020; Sun, 9 Apr 2023 23:58:01 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 339Nw1tG012019; Sun, 9 Apr 2023 23:58:01 GMT (envelope-from git) Date: Sun, 9 Apr 2023 23:58:01 GMT Message-Id: <202304092358.339Nw1tG012019@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: Kyle Evans Subject: git: 7ad1bebb0f1d - stable/13 - daemon: move signal setup into a function List-Id: Commits to the stable branches of the FreeBSD src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-branches List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-branches@freebsd.org X-BeenThere: dev-commits-src-branches@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: kevans X-Git-Repository: src X-Git-Refname: refs/heads/stable/13 X-Git-Reftype: branch X-Git-Commit: 7ad1bebb0f1d4b95d6bcdb59b90f1b234e6532b4 Auto-Submitted: auto-generated X-ThisMailContainsUnwantedMimeParts: N The branch stable/13 has been updated by kevans: URL: https://cgit.FreeBSD.org/src/commit/?id=7ad1bebb0f1d4b95d6bcdb59b90f1b234e6532b4 commit 7ad1bebb0f1d4b95d6bcdb59b90f1b234e6532b4 Author: Ihor Antonov AuthorDate: 2023-03-21 04:40:04 +0000 Commit: Kyle Evans CommitDate: 2023-04-09 22:49:52 +0000 daemon: move signal setup into a function No functional change intended. Reviewed by: kevans (cherry picked from commit 9ee1faeebab594712ed302ea51949410c9744920) --- usr.sbin/daemon/daemon.c | 142 ++++++++++++++++++++++++++--------------------- 1 file changed, 79 insertions(+), 63 deletions(-) diff --git a/usr.sbin/daemon/daemon.c b/usr.sbin/daemon/daemon.c index 54e2aeea6874..983dbdc7ed8c 100644 --- a/usr.sbin/daemon/daemon.c +++ b/usr.sbin/daemon/daemon.c @@ -83,6 +83,7 @@ struct daemon_state { bool log_reopen; }; +static void setup_signals(struct daemon_state *); static void restrict_process(const char *); static void handle_term(int); static void handle_chld(int); @@ -168,10 +169,40 @@ main(int argc, char *argv[]) sigset_t mask_susp; daemon_state_init(&state); + + /* + * Signal handling logic: + * + * - SIGTERM is masked while there is no child. + * + * - SIGCHLD is masked while reading from the pipe. SIGTERM has to be + * caught, to avoid indefinite blocking on read(). + * + * - Both SIGCHLD and SIGTERM are masked before calling sigsuspend() + * to avoid racing. + * + * - After SIGTERM is recieved and propagated to the child there are + * several options on what to do next: + * - read until EOF + * - read until EOF but only for a while + * - bail immediately + * Currently the third option is used, because otherwise there is no + * guarantee that read() won't block indefinitely if the child refuses + * to depart. To handle the second option, a different approach + * would be needed (procctl()?). + * + * - Child's exit might be detected by receiveing EOF from the pipe. + * But the child might have closed its stdout and stderr, so deamon + * must wait for the SIGCHLD to ensure that the child is actually gone. + */ sigemptyset(&mask_susp); sigemptyset(&mask_read); sigemptyset(&mask_term); sigemptyset(&mask_orig); + sigaddset(&mask_susp, SIGTERM); + sigaddset(&mask_susp, SIGCHLD); + sigaddset(&mask_term, SIGTERM); + sigaddset(&mask_read, SIGCHLD); /* * Supervision mode is enabled if one of the following options are used: @@ -312,57 +343,20 @@ main(int argc, char *argv[]) pidfile_write(state.parent_pidfh); if (state.supervision_enabled) { - struct sigaction act_term = { 0 }; - struct sigaction act_chld = { 0 }; - struct sigaction act_hup = { 0 }; - - /* Avoid PID racing with SIGCHLD and SIGTERM. */ - act_term.sa_handler = handle_term; - sigemptyset(&act_term.sa_mask); - sigaddset(&act_term.sa_mask, SIGCHLD); - - act_chld.sa_handler = handle_chld; - sigemptyset(&act_chld.sa_mask); - sigaddset(&act_chld.sa_mask, SIGTERM); - - act_hup.sa_handler = handle_hup; - sigemptyset(&act_hup.sa_mask); - - /* Block these when avoiding racing before sigsuspend(). */ - sigaddset(&mask_susp, SIGTERM); - sigaddset(&mask_susp, SIGCHLD); - /* Block SIGTERM when we lack a valid child PID. */ - sigaddset(&mask_term, SIGTERM); - /* - * When reading, we wish to avoid SIGCHLD. SIGTERM - * has to be caught, otherwise we'll be stuck until - * the read() returns - if it returns. - */ - sigaddset(&mask_read, SIGCHLD); /* Block SIGTERM to avoid racing until we have forked. */ if (sigprocmask(SIG_BLOCK, &mask_term, &mask_orig)) { warn("sigprocmask"); daemon_terminate(&state); } - if (sigaction(SIGTERM, &act_term, NULL) == -1) { - warn("sigaction"); - daemon_terminate(&state); - } - if (sigaction(SIGCHLD, &act_chld, NULL) == -1) { - warn("sigaction"); - daemon_terminate(&state); - } + + setup_signals(&state); + /* * Try to protect against pageout kill. Ignore the * error, madvise(2) will fail only if a process does * not have superuser privileges. */ (void)madvise(NULL, 0, MADV_PROTECT); - if (state.log_reopen && state.output_fd >= 0 && - sigaction(SIGHUP, &act_hup, NULL) == -1) { - warn("sigaction"); - daemon_terminate(&state); - } restart: if (pipe(state.pipe_fd)) { err(1, "pipe"); @@ -419,9 +413,9 @@ restart: /* * else: pid > 0 * fork succeeded, this is the parent branch, this can only happen when - * supervision is enabled + * supervision is enabled. * - * Unblock SIGTERM after we know we have a valid child PID to signal. + * Unblock SIGTERM - now there is a valid child PID to signal to. */ if (sigprocmask(SIG_UNBLOCK, &mask_term, NULL)) { warn("sigprocmask"); @@ -431,28 +425,7 @@ restart: state.pipe_fd[1] = -1; setproctitle("%s[%d]", state.title, (int)pid); - /* - * As we have closed the write end of pipe for parent process, - * we might detect the child's exit by reading EOF. The child - * might have closed its stdout and stderr, so we must wait for - * the SIGCHLD to ensure that the process is actually gone. - */ for (;;) { - /* - * We block SIGCHLD when listening, but SIGTERM we accept - * so the read() won't block if we wish to depart. - * - * Upon receiving SIGTERM, we have several options after - * sending the SIGTERM to our child: - * - read until EOF - * - read until EOF but only for a while - * - bail immediately - * - * We go for the third, as otherwise we have no guarantee - * that we won't block indefinitely if the child refuses - * to depart. To handle the second option, a different - * approach would be needed (procctl()?) - */ if (child_gone && state.child_eof) { break; } @@ -516,6 +489,49 @@ daemon_sleep(time_t secs, long nsecs) } } +/* + * Setup SIGTERM, SIGCHLD and SIGHUP handlers. + * To avoid racing SIGCHLD with SIGTERM corresponding + * signal handlers mask the other signal. + */ +static void +setup_signals(struct daemon_state *state) +{ + struct sigaction act_term = { 0 }; + struct sigaction act_chld = { 0 }; + struct sigaction act_hup = { 0 }; + + /* Setup SIGTERM */ + act_term.sa_handler = handle_term; + sigemptyset(&act_term.sa_mask); + sigaddset(&act_term.sa_mask, SIGCHLD); + if (sigaction(SIGTERM, &act_term, NULL) == -1) { + warn("sigaction"); + daemon_terminate(state); + } + + /* Setup SIGCHLD */ + act_chld.sa_handler = handle_chld; + sigemptyset(&act_chld.sa_mask); + sigaddset(&act_chld.sa_mask, SIGTERM); + if (sigaction(SIGCHLD, &act_chld, NULL) == -1) { + warn("sigaction"); + daemon_terminate(state); + } + + /* Setup SIGHUP if configured */ + if (!state->log_reopen || state->output_fd < 0) { + return; + } + + act_hup.sa_handler = handle_hup; + sigemptyset(&act_hup.sa_mask); + if (sigaction(SIGHUP, &act_hup, NULL) == -1) { + warn("sigaction"); + daemon_terminate(state); + } +} + static void open_pid_files(struct daemon_state *state) {