Miscelaneous fixes after source code audition
authorMatthew Mondor <mmondor@pulsar-zone.net>
Fri, 30 Apr 2004 00:01:18 +0000 (00:01 +0000)
committerMatthew Mondor <mmondor@pulsar-zone.net>
Fri, 30 Apr 2004 00:01:18 +0000 (00:01 +0000)
mmsoftware/apache-mmstat/apache-mmstat.c
mmsoftware/mmanoncvs/mmanoncvs.8
mmsoftware/mmanoncvs/mmanoncvs.c
mmsoftware/mmanoncvs/mmanoncvs.conf.5
mmsoftware/mmanoncvs/mmanoncvs.list.5
mmsoftware/mmidentd/mmidentd.c
mmsoftware/mmlib/mmreadcfg.c
mmsoftware/mmlib/mmserver2.3
mmsoftware/mmlib/mmserver2.c
mmsoftware/mmsendmail/mmsendmail.c

index cd0c0d3..eb2be1d 100644 (file)
@@ -1,4 +1,4 @@
-/* $Id: apache-mmstat.c,v 1.1 2003/07/02 08:27:51 mmondor Exp $ */
+/* $Id: apache-mmstat.c,v 1.2 2004/04/30 00:01:17 mmondor Exp $ */
 
 /*
  * Copyright (C) 2003, Matthew Mondor
@@ -133,7 +133,7 @@ int main(int argc, char **argv)
     if (getuid() != 0) {
        syslog(LOG_NOTICE, "%s: Not started as uid 0 from apache!? (uid %d)",
                argv[ARG_COMMAND], getuid());
-       exit(-1);
+       exit(EXIT_FAILURE);
     }
 
     /* We only accept a fixed number of arguments so that we restrict the
@@ -143,7 +143,7 @@ int main(int argc, char **argv)
     if (argc != ARG_MAX) {
        syslog(LOG_NOTICE, "%s: Started with wrong parameters",
                argv[ARG_COMMAND]);
-       exit(-1);
+       exit(EXIT_FAILURE);
     }
 
     /* Make sure that supplied user and group(s) are valid, and if so,
@@ -157,13 +157,13 @@ int main(int argc, char **argv)
        if ((uid = mmgetuid(argv[ARG_USER])) == -1) {
            syslog(LOG_NOTICE, "%s: Unknown user '%s'", argv[ARG_COMMAND],
                    argv[ARG_USER]);
-           exit(-1);
+           exit(EXIT_FAILURE);
        }
 
        if (!(gids = mmgetgidarray(&ngids, argv[ARG_GROUPS]))) {
            syslog(LOG_NOTICE, "%s: One of following groups unknown: '%s'",
                    argv[ARG_COMMAND], argv[ARG_GROUPS]);
-           exit(-1);
+           exit(EXIT_FAILURE);
        }
 
        /* NOTE: mmdropprivs() uses setegid(2), setgid(2), setgroups(2),
@@ -173,7 +173,7 @@ int main(int argc, char **argv)
        if (!mmdropprivs(uid, gids, ngids)) {
            syslog(LOG_NOTICE, "%s: Cannot change uid and gids to safe privs",
                    argv[ARG_COMMAND]);
-           exit(-1);
+           exit(EXIT_FAILURE);
        }
        mmfreegidarray(gids);
     }
@@ -202,26 +202,26 @@ int main(int argc, char **argv)
 
     if (setenv("MMSTATCONF", argv[ARG_CONF], TRUE) != 0) {
        syslog(LOG_NOTICE, "%s: Cannot setenv(3)", argv[ARG_COMMAND]);
-       exit(-1);
+       exit(EXIT_FAILURE);
     }
 
     if (!mmstat_init(&mms, TRUE, TRUE)) {
        syslog(LOG_NOTICE, "%s: Cannot initialize mmstat(3)",
                argv[ARG_COMMAND]);
-       exit(-1);
+       exit(EXIT_FAILURE);
     }
 
     /* We preferably don't want the line buffer to be on the stack */
     if ((linebuf = malloc(LINESIZ)) == NULL) {
        syslog(LOG_NOTICE, "%s: Cannot allocate line buffer",
                argv[ARG_COMMAND]);
-       exit(-1);
+       exit(EXIT_FAILURE);
     }
 
     log_parse(&mms, linebuf);
 
     /* NOTREACHED */
-    exit(0);
+    exit(EXIT_SUCCESS);
 }
 
 
@@ -229,7 +229,7 @@ int main(int argc, char **argv)
 static void sighandler(int sig)
 {
     /* We only catch SIGSEGV with this handler, and exit normally. */
-    exit(0);
+    exit(EXIT_SUCCESS);
 }
 
 
@@ -271,7 +271,9 @@ static void log_parse(mmstat_t *mms, char *line)
                *ptr = '_';
                break;
            case '*':           /* Considered as wildcards by mmstat(3) */
+               /* FALLTHROUGH */
            case '?':
+               /* FALLTHROUGH */
            case '%':           /* Why not, we use stdarg(3) alot */
                *ptr = '$';
                break;
index 3e00617..597958b 100644 (file)
@@ -1,6 +1,6 @@
-.\" $Id: mmanoncvs.8,v 1.8 2003/11/22 08:01:47 mmondor Exp $
+.\" $Id: mmanoncvs.8,v 1.9 2004/04/30 00:01:17 mmondor Exp $
 .\"
-.\" Copyright (C) 2003, Matthew Mondor
+.\" Copyright (C) 2003-2004, Matthew Mondor
 .\" All rights reserved.
 .\"
 .\" Redistribution and use in source and binary forms, with or without
@@ -708,7 +708,7 @@ configuration file to read
 .Sh AUTHOR
 .Nm
 was written by Matthew Mondor, and is
-Copyright (c) 2003, Matthew Mondor, All Rights Reserved.
+Copyright (c) 2003-2004, Matthew Mondor, All Rights Reserved.
 .Sh SEE ALSO
 .Xr mmanoncvs.conf 5 ,
 .Xr mmanoncvs.list 5 ,
index ada42e2..cbc8e71 100644 (file)
@@ -1,7 +1,7 @@
-/* $Id: mmanoncvs.c,v 1.21 2003/11/22 08:01:47 mmondor Exp $ */
+/* $Id: mmanoncvs.c,v 1.22 2004/04/30 00:01:17 mmondor Exp $ */
 
 /*
- * Copyright (C) 2003, Matthew Mondor
+ * Copyright (C) 2003-2004, Matthew Mondor
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
 
 MMCOPYRIGHT("@(#) Copyright (c) 2003\n\
 \tMatthew Mondor. All rights reserved.\n");
-MMRCSID("$Id: mmanoncvs.c,v 1.21 2003/11/22 08:01:47 mmondor Exp $");
+MMRCSID("$Id: mmanoncvs.c,v 1.22 2004/04/30 00:01:17 mmondor Exp $");
 
 
 
 /* DEFINITIONS */
 
 #define PROGRAM_NAME   "mmanoncvs"
-#define PROGRAM_VERSION        "0.0.1/mmondor"
+#define PROGRAM_VERSION        "0.0.2/mmondor"
 
 struct configuration {
     char LOCK_PATH[256], USER[32], GROUPS[256], ALOCK_PATH[256], TMPDIR[256],
@@ -220,7 +220,7 @@ static pid_t pid;
 static long recursion_level = 0;
 static off_t ssize_max;
 static gid_t repgroup;
-/* Used by main_mirrot(), tree_copy(), tmpdir_save() and sighandler() */
+/* Used by main_mirror(), tree_copy(), tmpdir_save() and sighandler() */
 static list_t gtree;
 static char gtmpdir[PATH_MAX], gtmpdir2[PATH_MAX];
 static char *copybuf;
@@ -494,14 +494,17 @@ in %s\n", cols[1], lines, CONF.LISTFILE);
  */
 static void sighandler(int sig)
 {
-    static int level = 0;
+    sigset_t set;
 
     /* A signal handler may be broken by another signal. Because we perform
      * the same operation for all three signals we handle, and that we only
-     * want to perform it once, just return immediately when recursion occurs.
+     * want to perform it once, block signals we expect to receive.
      */
-    if (++level > 1)
-       return;
+    (void) sigemptyset(&set);
+    (void) sigaddset(&set, SIGTERM);
+    (void) sigaddset(&set, SIGINT);
+    (void) sigaddset(&set, SIGSEGV);
+    (void) sigprocmask(SIG_BLOCK, &set, NULL);
 
     switch (sig) {
     case SIGTERM:
@@ -509,6 +512,8 @@ static void sighandler(int sig)
     case SIGINT:
        /* FALLTHROUGH */
     case SIGSEGV:
+       /* FALLTHROUGH */
+    default:
        (void) fprintf(stderr, "Received signal %d, attempting to cleanup\n",
                       sig);
        tree_close(&gtree, CONF.CVS_LOCKING);
index 5d746dc..0e68739 100644 (file)
@@ -1,6 +1,6 @@
-.\" $Id: mmanoncvs.conf.5,v 1.5 2003/11/22 08:01:48 mmondor Exp $
+.\" $Id: mmanoncvs.conf.5,v 1.6 2004/04/30 00:01:18 mmondor Exp $
 .\"
-.\" Copyright (C) 2003, Matthew Mondor
+.\" Copyright (C) 2003-2004, Matthew Mondor
 .\" All rights reserved.
 .\"
 .\" Redistribution and use in source and binary forms, with or without
@@ -268,7 +268,7 @@ KEEP_EXEC           TRUE
 .Sh AUTHOR
 .Nm mmanoncvs
 was written by Matthew Mondor, and is
-Copyright (c) 2003, Matthew Mondor, All rights reserved.
+Copyright (c) 2003-2004, Matthew Mondor, All rights reserved.
 .Sh FILES
 .Bl -tag -width indent -offset indent
 .It Pa /usr/local/etc/mmanoncvs.conf
index e2850b3..0d67e49 100644 (file)
@@ -1,6 +1,6 @@
-.\" $Id: mmanoncvs.list.5,v 1.4 2003/11/11 06:37:33 mmondor Exp $
+.\" $Id: mmanoncvs.list.5,v 1.5 2004/04/30 00:01:18 mmondor Exp $
 .\"
-.\" Copyright (C) 2003, Matthew Mondor
+.\" Copyright (C) 2003-2004, Matthew Mondor
 .\" All rights reserved.
 .\"
 .\" Redistribution and use in source and binary forms, with or without
@@ -108,7 +108,7 @@ None
 .Sh AUTHOR
 .Nm mmanoncvs
 was written by Matthew Mondor, and is
-Copyright (c) 2003, Matthew Mondor, All rights reserved.
+Copyright (c) 2003-2004, Matthew Mondor, All rights reserved.
 .Sh FILES
 .Bl -tag -width indent -offset indent
 .It Pa /usr/local/etc/mmanoncvs.list
index efd44b1..042f732 100644 (file)
@@ -1,4 +1,4 @@
-/* $Id: mmidentd.c,v 1.2 2004/04/17 01:50:27 mmondor Exp $ */
+/* $Id: mmidentd.c,v 1.3 2004/04/30 00:01:18 mmondor Exp $ */
 
 /*
  * Copyright (C) 2003, Matthew Mondor
 #define RBUFSIZE       64
 #define RETRIES                64
 #define TIMEOUT                30
-#define ISCHAR(c)      ((c) > 32)
+#define ISCHAR(c)      isprint(c)
 
 
 
index 7b0632e..70dcde9 100644 (file)
@@ -1,4 +1,4 @@
-/* $Id: mmreadcfg.c,v 1.12 2004/03/22 06:59:36 mmondor Exp $ */
+/* $Id: mmreadcfg.c,v 1.13 2004/04/30 00:01:18 mmondor Exp $ */
 
 /*
  * Copyright (C) 1991-2004, Matthew Mondor
@@ -43,6 +43,7 @@
 #include <fcntl.h>
 #include <unistd.h>
 #include <stdlib.h>
+#include <limits.h>
 #include <pwd.h>
 #include <grp.h>
 #include <ctype.h>
@@ -60,7 +61,7 @@
 
 MMCOPYRIGHT("@(#) Copyright (c) 1991-2004\n\
 \tMatthew Mondor. All rights reserved.\n");
-MMRCSID("$Id: mmreadcfg.c,v 1.12 2004/03/22 06:59:36 mmondor Exp $");
+MMRCSID("$Id: mmreadcfg.c,v 1.13 2004/04/30 00:01:18 mmondor Exp $");
 
 
 
@@ -294,7 +295,7 @@ mmreadcfg(cres_t *res, carg_t *arg, const char *cfg)
                {
                    long v;
 
-                   v = atol(wptr);
+                   v = strtol(wptr, NULL, 0);
                    if (v < nod->args->CA_Min) {
                        res->CR_Err = CRE_VAL_TOO_LOW;
                        res->CR_Keyword = nod->args;
index 7e95fbe..6baed66 100644 (file)
@@ -1,4 +1,4 @@
-.\" $Id: mmserver2.3,v 1.13 2004/04/18 06:45:53 mmondor Exp $
+.\" $Id: mmserver2.3,v 1.14 2004/04/30 00:01:18 mmondor Exp $
 .\"
 .\" Copyright (C) 2004, Matthew Mondor
 .\" All rights reserved.
@@ -605,7 +605,13 @@ rather than
 .Xr flock 2
 or
 .Xr poll 2 .
-At least on NetBSD, disabling the lock does not cause problems.
+At least on NetBSD, disabling the lock does not seem to generally cause
+problems, but it does in the case where the processes are swapped out. It then
+seems that some processes will wait on the "netconn" channel (in
+.Xr accept 2 )
+rather than at
+.Xr poll 2
+on the "select" channel. Using a serialization lock fixes this issue.
 .It bool exit_interrupt_requests;
 Tells weither or not requests currently being processed should be interrupted
 whenever the parent process is to exit (generally as the result of reception
index d274798..35da593 100644 (file)
@@ -1,4 +1,4 @@
-/* $Id: mmserver2.c,v 1.22 2004/04/18 06:54:28 mmondor Exp $ */
+/* $Id: mmserver2.c,v 1.23 2004/04/30 00:01:18 mmondor Exp $ */
 
 /*
  * Copyright (C) 2004, Matthew Mondor
  * - Add some support for request maximum timeout (at least for using_execve
  *   applications, since there is no other way to cause execve(2) requests
  *   to interrupt than to kill the process from another one (the parent could).
+ *   This however is tricky. Applications which don't use execve(2) don't need
+ *   such a timer as they can set one up themselves. However, if they do use
+ *   execve(2), the children processes cannot rely on SIGALRM anymore. They
+ *   then need another process to interrupt them using SIGTERM if they need
+ *   to be interrupted after a timeout. The parent could do it with it's
+ *   timer_ctx via mmalarm(3) easily, but because the children are answering
+ *   and handling the incomming requests, how can they tell the parent to
+ *   start such a timer for them, unless I used a special IPC mechanism.
+ *   If they had to launch their own control process using fork(2) it would
+ *   slow down the system too much... I could perhaps use some kind of
+ *   AF_LOCAL SOCK_DGRAM trick. A child process would simply send a packet
+ *   with it's pid_t, causing a new timeout to be created in the parent to
+ *   kill that pid_t with SIGTERM if it expires. However, we also need to
+ *   get rid of any existing entry at the SIGCHLD handler... server_execve()
+ *   would be responsible for starting the timer before calling execve(2),
+ *   noticing the parent to do so. The SIGCHLD handler in the parent would
+ *   be responsible for stopping the timer if it did not expire already.
+ * - Implement support to allow applications set a hint for setproctitle(3),
+ *   similarily to what implemented in mmspawnd(8). This also may affect
+ *   syslog(3), we could openlog(3) using that hint...
  */
 
 
 
 MMCOPYRIGHT("@(#) Copyright (c) 2004\n\
 \tMatthew Mondor. All rights reserved.\n");
-MMRCSID("$Id: mmserver2.c,v 1.22 2004/04/18 06:54:28 mmondor Exp $");
+MMRCSID("$Id: mmserver2.c,v 1.23 2004/04/30 00:01:18 mmondor Exp $");
 
 
 
@@ -111,6 +131,10 @@ struct child_process_node {
     struct socket_node *snode;
     struct address_node *anode;
 };
+/* The following nodes are also only used for applications using execve(2), so
+ * that the SIGCHLD handler of the parent may efficently link the pid_t of the
+ * dieing children processes to their status node.
+ */
 struct child_pid_status_node {
     hashnode_t node;
     pid_t pid;         /* Key */
@@ -209,8 +233,6 @@ struct local {
     struct socket_node *socket_node;
     struct address_node *address_node;
     struct server_request request;
-    /* Peristent sockets used for SOCK_DGRAM/UDP only */
-    int udp_socket, udp6_socket, local_socket;
     /* Used so that we may jump back to the main child loop */
     jmp_buf jmpbuf;
     /* The pollfd array corresponding to the listening sockets */
@@ -1603,7 +1625,7 @@ static bool parent_main_address_iterator(hashnode_t *hnode, void *udata)
 /* ARGSUSED */
 static void parent_main_expire(timerid_t tid, void *udata)
 {
-    bool *expired = udata;
+    volatile bool *expired = udata;
 
     *expired = TRUE;
 }
@@ -1702,10 +1724,8 @@ static void parent_main_pool_manager(timerid_t tid, void *udata)
      */
     if (parent.ready_avg_cnt == global.config.children_average_seconds) {
        parent.ready_avg /= parent.ready_avg_cnt;
-       if (parent.ready_avg > global.config.children_maxspare) {
-           for (i = 0,
-                   to = parent.ready_avg - global.config.children_maxspare,
-                   node = DLIST_TOP(&parent.processes_list);
+       if ((to = parent.ready_avg - global.config.children_maxspare) > 0) {
+           for (i = 0, node = DLIST_TOP(&parent.processes_list);
                    i < to && node != NULL; node = DLIST_NEXT(node)) {
                if (node->status == PSTAT_READY) {
                    (void) kill(node->pid, SIGTERM);
@@ -1981,7 +2001,7 @@ static pid_t child_launch(void)
        cnode->status = PSTAT_READY;
        cnode->served = 0;
        if (snode != NULL) {
-           /* Link fast pid_t->status lookup node  now that we have pid_t */
+           /* Link fast pid_t->status lookup node now that we have pid_t */
            cnode->snode = NULL;
            cnode->anode = NULL;
            snode->pid = pid;
@@ -2202,7 +2222,7 @@ static void child_main(void)
                 * for efficiency, but allows to define a clean user API which
                 * does not disclose or depend on internal library structures.
                 * We at least are not holding any locks at this time.
-                * We also allow cases where local.address_node == NULL
+                * We also allow situations where local.address_node == NULL
                 * in which case no resolving or limits are wanted.
                 */
                local.request.client_socket = local.socket;
@@ -2261,7 +2281,7 @@ static void child_main(void)
 
        } else if (err == -1)
            syslog(LOG_NOTICE, "child_main() - poll() - %s", strerror(errno));
-       /* An error occurs, let another a chance to obtain the accept(2) lock
+       /* An error occurred, let another a chance to obtain the accept(2) lock
         * and continue looping.
         */
        if (global.config.serialization_lock)
@@ -2409,7 +2429,7 @@ snode->config.address_cache_concurrency_limit);
                hnode = hnode2;
            }
        }
-       /* Obtain anode lock since we need to check-and-set */
+       /* Obtain anode lock since we need to test-and-set */
        LOCK(*snode->lock);
        /* If an hnode wasn't already assigned to our anode,
         * link hnode to our anode with the lock held.
@@ -2481,6 +2501,7 @@ static void child_sighandler(int sig)
        /* NOTREACHED */
        break;
     case SIGALRM:
+       /* Call sigalrm handler hook if any */
        if (global.config.child_sigalrm_hook != NULL)
            global.config.child_sigalrm_hook();
        break;
@@ -2526,10 +2547,9 @@ static void child_close(bool resume)
        }
        local.socket_node = NULL;
     }
-    if (resume) {
-       local.status->status = PSTAT_READY;
+    if (resume)
+       /* This will automatically set process to PSTAT_READY */
        (void) longjmp(local.jmpbuf, 0);
-    }
 }
 
 /* Child process exit point, Depending on state, this process will be
index 6abea95..05ec692 100644 (file)
@@ -1,4 +1,4 @@
-/* $Id: mmsendmail.c,v 1.8 2003/10/13 11:20:04 mmondor Exp $ */
+/* $Id: mmsendmail.c,v 1.9 2004/04/30 00:01:18 mmondor Exp $ */
 
 /*
  * Copyright (C) 2000-2003, Matthew Mondor
@@ -104,7 +104,7 @@ int main(int argc, char **argv)
     fdbi = fdb = NULL;
     msgsock = -1;
     fdbrb = NULL;
-    ret = -1;
+    ret = EXIT_FAILURE;
 
     if (argc > 6) {
        hostname = argv[1];
@@ -162,7 +162,7 @@ int main(int argc, char **argv)
     server.sin_addr.s_addr = ip->s_addr;
     server.sin_port = htons(port);
 
-    // Attempt to connect to host
+    /* Attempt to connect to host */
     if ((connect(msgsock, (struct sockaddr *)&server, sizeof(server)))
            != 0) {
        fprintf(stderr, "* Cannot connect to server");
@@ -225,7 +225,7 @@ int main(int argc, char **argv)
 
                        /* Was message accepted? */
                        if (result() == 250)
-                           ret = 0;
+                           ret = EXIT_SUCCESS;
                        else {
                            syslog(LOG_NOTICE, "Message refused by server");
                            fprintf(stderr, " * Message refused by server\n");
@@ -304,5 +304,5 @@ static void sighandler(int sig)
 
     syslog(LOG_NOTICE, "SIGNAL: %s", s);
     fprintf(stderr, " * Received signal: %s\n", s);
-    exit(-1);
+    exit(EXIT_FAILURE);
 }