]> git.tuebingen.mpg.de Git - paraslash.git/commitdiff
Merge branch 'maint'
authorAndre Noll <maan@tuebingen.mpg.de>
Fri, 25 Nov 2022 12:27:13 +0000 (13:27 +0100)
committerAndre Noll <maan@tuebingen.mpg.de>
Fri, 25 Nov 2022 12:27:13 +0000 (13:27 +0100)
This fixes two old bugs related to signal handling which bite only
rarely. But if they do, it hurts plenty.

* maint:
  server: Fix race condition in com_stat().
  server: Avoid deadlock in daemon_log().

1  2 
command.c
signal.c

diff --combined command.c
index 00d2c5a61ada8fdcedfad2989e43844265ca46d5,427c90fe13fe6ccc0f6fd5259c24f45e69f211fa..c56a15822dfaf8a7b165b44480a6adcf90d5e1a3
+++ b/command.c
@@@ -6,10 -6,14 +6,10 @@@
  #include <sys/socket.h>
  #include <regex.h>
  #include <signal.h>
 -#include <sys/types.h>
 -#include <osl.h>
  #include <arpa/inet.h>
 -#include <sys/un.h>
  #include <netdb.h>
  #include <lopsub.h>
  
 -#include "server.lsg.h"
  #include "para.h"
  #include "error.h"
  #include "lsu.h"
  #include "command.h"
  #include "string.h"
  #include "afh.h"
 -#include "afs.h"
  #include "net.h"
  #include "server.h"
  #include "list.h"
 -#include "send.h"
  #include "sched.h"
 +#include "send.h"
  #include "vss.h"
  #include "daemon.h"
  #include "fd.h"
@@@ -47,12 -52,14 +47,14 @@@ extern struct misc_meta_data *mmd
  int send_afs_status(struct command_context *cc, int parser_friendly);
  static bool subcmd_should_die;
  
+ /*
+  * Don't call PARA_XXX_LOG() here as we might already hold the log mutex. See
+  * generic_signal_handler() for details.
+  */
  static void command_handler_sighandler(int s)
  {
-       if (s != SIGTERM)
-               return;
-       PARA_EMERG_LOG("terminating on signal %d\n", SIGTERM);
-       subcmd_should_die = true;
+       if (s == SIGTERM)
+               subcmd_should_die = true;
  }
  
  /*
@@@ -78,7 -85,7 +80,7 @@@ static char *vss_status_tohuman(unsigne
   */
  static char *vss_get_status_flags(unsigned int flags)
  {
 -      char *msg = para_malloc(5 * sizeof(char));
 +      char *msg = alloc(5 * sizeof(char));
  
        msg[0] = (flags & VSS_PLAYING)? 'P' : '_';
        msg[1] = (flags & VSS_NOMORE)? 'O' : '_';
@@@ -390,6 -397,7 +392,6 @@@ static int com_si(struct command_contex
                "server_pid: %d\n"
                "afs_pid: %d\n"
                "connections (active/accepted/total): %u/%u/%u\n"
 -              "current loglevel: %s\n"
                "supported audio formats: %s\n",
                ut, mmd->num_played,
                (int)getppid(),
                mmd->active_connections,
                mmd->num_commands,
                mmd->num_connects,
 -              ENUM_STRING_VAL(LOGLEVEL),
                AUDIO_FORMAT_HANDLERS
        );
        mutex_unlock(mmd_mutex);
@@@ -505,6 -514,7 +507,7 @@@ static int com_stat(struct command_cont
         * while we sleep.
         */
        para_block_signal(SIGTERM);
+       para_block_signal(SIGUSR1);
        for (;;) {
                sigset_t set;
                /*
                 * open a race window similar to the one described above.
                 */
                pselect(1, NULL, NULL, NULL, &ts, &set);
-               if (subcmd_should_die)
+               if (subcmd_should_die) {
+                       PARA_EMERG_LOG("terminating on SIGTERM\n");
                        goto out;
+               }
                ret = -E_SERVER_CRASH;
                if (getppid() == 1)
                        goto out;
@@@ -587,56 -599,9 +592,56 @@@ static int com_hup(__a_unused struct co
  }
  EXPORT_SERVER_CMD_HANDLER(hup);
  
 +static int com_ll(struct command_context *cc, struct lls_parse_result *lpr)
 +{
 +      unsigned ll, perms;
 +      char *errctx;
 +      const char *sev[] = {SEVERITIES}, *arg;
 +      int ret = lls(lls_check_arg_count(lpr, 0, 1, &errctx));
 +
 +      if (ret < 0) {
 +              send_errctx(cc, errctx);
 +              return ret;
 +      }
 +      if (lls_num_inputs(lpr) == 0) { /* reporting is an unprivileged op. */
 +              const char *severity;
 +              mutex_lock(mmd_mutex);
 +              severity = sev[mmd->loglevel];
 +              mutex_unlock(mmd_mutex);
 +              return send_sb_va(&cc->scc, SBD_OUTPUT, "%s\n", severity);
 +      }
 +      /*
 +       * Changing the loglevel changes the state of both the afs and the vss,
 +       * so we require both AFS_WRITE and VSS_WRITE.
 +       */
 +      perms = AFS_WRITE | VSS_WRITE;
 +      if ((cc->u->perms & perms) != perms)
 +              return -ERRNO_TO_PARA_ERROR(EPERM);
 +      arg = lls_input(0, lpr);
 +      for (ll = 0; ll < NUM_LOGLEVELS; ll++)
 +              if (!strcmp(arg, sev[ll]))
 +                      break;
 +      if (ll >= NUM_LOGLEVELS)
 +              return -ERRNO_TO_PARA_ERROR(EINVAL);
 +      PARA_INFO_LOG("new log level: %s\n", sev[ll]);
 +      /* Ask the server and afs processes to adjust their log level. */
 +      mutex_lock(mmd_mutex);
 +      mmd->loglevel = ll;
 +      mutex_unlock(mmd_mutex);
 +      return 1;
 +}
 +EXPORT_SERVER_CMD_HANDLER(ll);
 +
  static int com_term(__a_unused struct command_context *cc,
                __a_unused struct lls_parse_result *lpr)
  {
 +      /*
 +       * The server catches SIGTERM and propagates this signal to all its
 +       * children. We are about to exit anyway, but we'd leak tons of memory
 +       * if being terminated by the signal. So we ignore the signal here and
 +       * terminate via the normal exit path, deallocating all memory.
 +       */
 +      para_sigaction(SIGTERM, SIG_IGN);
        kill(getppid(), SIGTERM);
        return 1;
  }
@@@ -785,6 -750,14 +790,6 @@@ out
  }
  EXPORT_SERVER_CMD_HANDLER(jmp);
  
 -/* deprecated, does nothing */
 -static int com_tasks(__a_unused struct command_context *cc,
 -              __a_unused struct lls_parse_result *lpr)
 -{
 -      return 1;
 -}
 -EXPORT_SERVER_CMD_HANDLER(tasks);
 -
  static void reset_signals(void)
  {
        para_sigaction(SIGCHLD, SIG_IGN);
  }
  
  struct connection_features {
 -      int dummy; /* none at the moment */
 +      bool sha256_requested; /* can be removed after 0.7.0 */
  };
  
  static int parse_auth_request(char *buf, int len, const struct user **u,
                *p = '\0';
                p++;
                create_argv(p, ",", &features);
 +              /*
 +               * Still accept sideband and AES feature requests (as a no-op)
 +               * because some 0.6.x clients request them. The two checks
 +               * below may be removed after 0.7.1.
 +               */
                for (i = 0; features[i]; i++) {
                        if (strcmp(features[i], "sideband") == 0)
                                continue;
                        if (strcmp(features[i], "aes_ctr128") == 0)
                                continue;
 +                      /*
 +                       * ->sha256_requested can go away after 0.7.0 but the
 +                       * check has to stay until 0.9.0.
 +                       */
 +                      if (strcmp(features[i], "sha256") == 0)
 +                              cf->sha256_requested = true;
                        else {
                                ret = -E_BAD_FEATURE;
                                goto out;
@@@ -872,13 -834,13 +877,13 @@@ static int run_command(struct command_c
        }
        perms = server_command_perms[ret];
        if ((perms & cc->u->perms) != perms)
 -              return -E_PERM;
 +              return -ERRNO_TO_PARA_ERROR(EPERM);
        lcmd = lls_cmd(ret, server_cmd_suite);
        end = iov->iov_base + iov->iov_len;
        for (i = 0; p < end; i++)
                p += strlen(p) + 1;
        argc = i;
 -      argv = para_malloc((argc + 1) * sizeof(char *));
 +      argv = arr_alloc(argc + 1, sizeof(char *));
        for (i = 0, p = iov->iov_base; p < end; i++) {
                argv[i] = para_strdup(p);
                p += strlen(p) + 1;
@@@ -932,8 -894,8 +937,8 @@@ int handle_connect(int fd
  {
        int ret;
        unsigned char rand_buf[APC_CHALLENGE_SIZE + 2 * SESSION_KEY_LEN];
 -      unsigned char challenge_hash[HASH_SIZE];
 -      char *command = NULL, *buf = para_malloc(HANDSHAKE_BUFSIZE) /* must be on the heap */;
 +      unsigned char challenge_hash[HASH2_SIZE];
 +      char *command = NULL, *buf = alloc(HANDSHAKE_BUFSIZE) /* must be on the heap */;
        size_t numbytes;
        struct command_context cc_struct = {.u = NULL}, *cc = &cc_struct;
        struct iovec iov;
        /* send Welcome message */
        ret = write_va_buffer(fd, "This is para_server, version "
                PACKAGE_VERSION ".\n"
 -              "Features: sideband,aes_ctr128\n"
 +              "Features: sha256\n" /* no longer announce this after 0.8.0 */
        );
        if (ret < 0)
                goto net_err;
         * of the random data.
         */
        ret = -E_BAD_AUTH;
 -      if (numbytes != HASH_SIZE)
 -              goto net_err;
 -      hash_function((char *)rand_buf, APC_CHALLENGE_SIZE, challenge_hash);
 -      if (memcmp(challenge_hash, buf, HASH_SIZE))
 -              goto net_err;
 +      if (cf.sha256_requested) {
 +              if (numbytes != HASH2_SIZE)
 +                      goto net_err;
 +              hash2_function((char *)rand_buf, APC_CHALLENGE_SIZE, challenge_hash);
 +              if (memcmp(challenge_hash, buf, HASH2_SIZE))
 +                      goto net_err;
 +      } else { /* old client. This can be removed after 0.7.0 */
 +              if (numbytes != HASH_SIZE)
 +                      goto net_err;
 +              hash_function((char *)rand_buf, APC_CHALLENGE_SIZE, challenge_hash);
 +              if (memcmp(challenge_hash, buf, HASH_SIZE))
 +                      goto net_err;
 +      }
        /* auth successful */
        alarm(0);
        PARA_INFO_LOG("good auth for %s\n", cc->u->name);
diff --combined signal.c
index bdafeaf296e026508698c306e4b498e0710a9cee,9b328caf6550da385eb64c824217da7f3abd4588..d9a6aa37057415117c32a6a18c33d81977499392
+++ b/signal.c
@@@ -27,7 -27,7 +27,7 @@@ static int signal_pipe[2]
   * signal arrives, the signal handler writes the number of the signal received
   * to one end of the signal pipe. The application can test for pending signals
   * by checking if the file descriptor of the other end of the signal pipe is
 - * ready for reading, see select(2).
 + * ready for reading.
   *
   * \return This function either succeeds or calls exit(3) to terminate the
   * current process. On success, a signal task structure is returned.
@@@ -48,7 -48,7 +48,7 @@@ struct signal_task *signal_init_or_die(
        ret = mark_fd_nonblocking(signal_pipe[1]);
        if (ret < 0)
                goto err_out;
 -      st = para_calloc(sizeof(*st));
 +      st = zalloc(sizeof(*st));
        st->fd = signal_pipe[0];
        return st;
  err_out:
@@@ -72,10 -72,13 +72,13 @@@ static void generic_signal_handler(int 
                errno = save_errno;
                return;
        }
-       if (ret < 0)
-               PARA_EMERG_LOG("%s\n", strerror(errno));
-       else
-               PARA_EMERG_LOG("short write to signal pipe\n");
+       /*
+        * This is a fatal error which should never happen. We must not call
+        * PARA_XXX_LOG() here because this might acquire the log mutex which
+        * is already taken by the main program if the interrupt occurs while a
+        * log message is being printed. The mutex will not be released as long
+        * as this signal handler is running, so a deadlock ensues.
+        */
        exit(EXIT_FAILURE);
  }
  
@@@ -202,14 -205,16 +205,14 @@@ void para_unblock_signal(int sig
  /**
   * Return the number of the next pending signal.
   *
 - * \param rfds The fd_set containing the signal pipe.
 - *
   * \return On success, the number of the received signal is returned. If there
   * is no signal currently pending, the function returns zero. On read errors
   * from the signal pipe, the process is terminated.
   */
 -int para_next_signal(fd_set *rfds)
 +int para_next_signal(void)
  {
        size_t n;
 -      int s, ret = read_nonblock(signal_pipe[0], &s, sizeof(s), rfds, &n);
 +      int s, ret = read_nonblock(signal_pipe[0], &s, sizeof(s), &n);
  
        if (ret < 0) {
                PARA_EMERG_LOG("%s\n", para_strerror(-ret));