Fix an invalid free() in command handlers.

[paraslash.git] / server.c

diff --git a/server.c b/server.c
index 4822ebb5c3066404e46f72b8154acc6f452bde04..a4115eb65734391128481f6fa421d548aba78e20 100644 (file)
--- a/server.c
+++ b/server.c
@@ -115,7 +115,6 @@ int mmd_mutex;
 
 /** The file containing user information (public key, permissions). */
 static char *user_list_file = NULL;
-static int mmd_shm_id;
 
 
 /** The task responsible for server command handling. */
@@ -162,16 +161,16 @@ static void init_colors_or_die(void)
 static void init_ipc_or_die(void)
 {
        void *shm;
-       int ret = shm_new(sizeof(struct misc_meta_data));
+       int shmid, ret = shm_new(sizeof(struct misc_meta_data));
 
        if (ret < 0)
                goto err_out;
-
-       ret = shm_attach(ret, ATTACH_RW, &shm);
+       shmid = ret;
+       ret = shm_attach(shmid, ATTACH_RW, &shm);
+       shm_destroy(shmid);
        if (ret < 0)
                goto err_out;
        mmd = shm;
-       mmd_shm_id = ret;
 
        ret = mutex_new();
        if (ret < 0)
@@ -228,7 +227,7 @@ void parse_config_or_die(int override)
                        .initialize = 0,
                        .check_required = 1,
                        .check_ambiguity = 0,
-                       .print_errors = 1
+                       .print_errors = !conf.daemon_given
                };
                server_cmdline_parser_config_file(cf, &conf, &params);
                conf.daemon_given = tmp;
@@ -301,10 +300,11 @@ static void signal_post_select(struct sched *s, struct task *t)
                PARA_EMERG_LOG("terminating on signal %d\n", st->signum);
 genocide:
                kill(0, SIGTERM);
+               free(mmd->afd.afhi.chunk_table);
+               free(mmd->afd.afhi.info_string);
+               close_listed_fds();
                mutex_destroy(mmd_mutex);
                shm_detach(mmd);
-               shm_destroy(mmd_shm_id);
-
                exit(EXIT_FAILURE);
        }
 }
@@ -331,8 +331,6 @@ static void init_signal_task(void)
                goto err;
        if (signal(SIGPIPE, SIG_IGN) == SIG_ERR)
                goto err;
-       if (signal(SIGUSR1, SIG_IGN) == SIG_ERR)
-               goto err;
        add_close_on_fork_list(st->fd);
        register_task(&st->task);
        return;
@@ -354,6 +352,8 @@ static void command_post_select(struct sched *s, struct task *t)
        int new_fd, ret, i;
        char *peer_name;
        pid_t child_pid;
+       uint32_t *chunk_table;
+       char *info_string;
 
        if (!FD_ISSET(sct->listen_fd, &s->rfds))
                return;
@@ -366,6 +366,16 @@ static void command_post_select(struct sched *s, struct task *t)
        mmd->num_connects++;
        mmd->active_connections++;
        random();
+       /* The chunk table and the info_string are pointers located in the
+        * mmd struct that point to dynamically allocated memory that must be
+        * freed by the parent and the child. However, as the mmd struct is in
+        * a shared memory area, there's no guarantee that after the fork these
+        * pointers are still valid in child context. As these two pointers are
+        * not used in the child anyway, we save them to local variables and
+        * free the memory via that copy in the child.
+        */
+       info_string = mmd->afd.afhi.info_string;
+       chunk_table = mmd->afd.afhi.chunk_table;
        child_pid = fork();
        if (child_pid < 0) {
                ret = -ERRNO_TO_PARA_ERROR(errno);
@@ -376,6 +386,9 @@ static void command_post_select(struct sched *s, struct task *t)
                /* parent keeps accepting connections */
                return;
        }
+       /* mmd might already have changed at this point */
+       free(info_string);
+       free(chunk_table);
        alarm(ALARM_TIMEOUT);
        close_listed_fds();
        para_signal_shutdown();
@@ -472,6 +485,12 @@ static int init_afs(void)
        return afs_server_socket[0];
 }
 
+__noreturn static void tmp_sigchld_handler(__a_unused int s)
+{
+       PARA_EMERG_LOG("caught early SIGCHLD\n");
+       exit(EXIT_FAILURE);
+}
+
 static void server_init(int argc, char **argv)
 {
        struct server_cmdline_parser_params params = {
@@ -502,6 +521,27 @@ static void server_init(int argc, char **argv)
                daemonize();
        PARA_NOTICE_LOG("initializing audio format handlers\n");
        afh_init();
+
+       /*
+        * Although afs uses its own signal handling we must ignore SIGUSR1
+        * _before_ the afs child process gets born by init_afs() below.  It's
+        * racy to do this in the child because the parent might send SIGUSR1
+        * before the child gets a chance to ignore this signal -- only the
+        * good die young.
+        */
+       if (signal(SIGUSR1, SIG_IGN) == SIG_ERR) {
+               PARA_EMERG_LOG("failed to ignore SIGUSR1\n");
+               exit(EXIT_FAILURE);
+       }
+       /*
+        * We have to install a SIGCHLD handler before the afs process is being
+        * forked off. Otherwise, para_server does not notice if afs dies before
+        * the SIGCHLD handler has been installed by init_signal_task() below.
+        */
+       if (signal(SIGCHLD, tmp_sigchld_handler) == SIG_ERR) {
+               PARA_EMERG_LOG("failed to install temporary SIGCHLD handler\n");
+               exit(EXIT_FAILURE);
+       }
        PARA_NOTICE_LOG("initializing the audio file selector\n");
        afs_socket = init_afs();
        init_signal_task();