telnetd: fix problem with zombies (by Paul Fox <pgf@brightstareng.com>)

syslogd: strip trailing NULs

diff --git a/archival/unzip.c b/archival/unzip.c
index 8462822f1a1529fc1f5277ba6ea9e79fc60f344c..001f2e128b99a4df18dd15e9ab0605f552712a23 100644 (file)
--- a/archival/unzip.c
+++ b/archival/unzip.c
@@ -57,7 +57,7 @@ typedef union {
                uint16_t filename_len;                  /* 22-23 */
                uint16_t extra_len;                     /* 24-25 */
        } formatted ATTRIBUTE_PACKED;
-} zip_header_t;
+} zip_header_t ATTRIBUTE_PACKED;
 
 /* Check the offset of the last element, not the length.  This leniency
  * allows for poor packing, whereby the overall struct may be too long,

diff --git a/networking/telnetd.c b/networking/telnetd.c
index cccf03dfd54944bd88ff059eab52392117511db5..108bbf44f5770fe085810ab289f3fce7fc8fa550 100644 (file)
--- a/networking/telnetd.c
+++ b/networking/telnetd.c
@@ -279,6 +279,10 @@ make_new_session(
        /* make new session and process group */
        setsid();
 
+       /* Restore default signal handling */
+       signal(SIGCHLD, SIG_DFL);
+       signal(SIGPIPE, SIG_DFL);
+
        /* open the child's side of the tty. */
        /* NB: setsid() disconnects from any previous ctty's. Therefore
         * we must open child's side of the tty AFTER setsid! */
@@ -302,14 +306,18 @@ make_new_session(
        /* Uses FILE-based I/O to stdout, but does fflush(stdout),
         * so should be safe with vfork.
         * I fear, though, that some users will have ridiculously big
-        * issue files, and they may block writing to fd 1. */
+        * issue files, and they may block writing to fd 1,
+        * (parent is supposed to read it, but parent waits
+        * for vforked child to exec!) */
        print_login_issue(issuefile, NULL);
 
        /* Exec shell / login / whatever */
        login_argv[0] = loginpath;
        login_argv[1] = NULL;
-       execvp(loginpath, (char **)login_argv);
-       /* Safer with vfork, and we shouldn't send message
+       /* exec busybox applet (if PREFER_APPLETS=y), if that fails,
+        * exec external program */
+       BB_EXECVP(loginpath, (char **)login_argv);
+       /* _exit is safer with vfork, and we shouldn't send message
         * to remote clients anyway */
        _exit(1); /*bb_perror_msg_and_die("execv %s", loginpath);*/
 }
@@ -374,7 +382,7 @@ free_session(struct tsession *ts)
 
 #else /* !FEATURE_TELNETD_STANDALONE */
 
-/* Used in main() only, thus exits. */
+/* Used in main() only, thus "return 0" actually is exit(0). */
 #define free_session(ts) return 0
 
 #endif
@@ -384,20 +392,22 @@ static void handle_sigchld(int sig)
        pid_t pid;
        struct tsession *ts;
 
-       pid = waitpid(-1, &sig, WNOHANG);
-       if (pid > 0) {
+       /* Looping: more than one child may have exited */
+       while (1) {
+               pid = waitpid(-1, NULL, WNOHANG);
+               if (pid <= 0)
+                       break;
                ts = sessions;
                while (ts) {
                        if (ts->shell_pid == pid) {
                                ts->shell_pid = -1;
-                               return;
+                               break;
                        }
                        ts = ts->next;
                }
        }
 }
 
-
 int telnetd_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
 int telnetd_main(int argc, char **argv)
 {
@@ -430,7 +440,7 @@ int telnetd_main(int argc, char **argv)
                if (!(opt & OPT_FOREGROUND)) {
                        /* DAEMON_CHDIR_ROOT was giving inconsistent
                         * behavior with/without -F, -i */
-                       bb_daemonize_or_rexec(0 /*DAEMON_CHDIR_ROOT*/, argv);
+                       bb_daemonize_or_rexec(0 /*was DAEMON_CHDIR_ROOT*/, argv);
                }
        }
        /* Redirect log to syslog early, if needed */
@@ -466,6 +476,8 @@ int telnetd_main(int argc, char **argv)
 
        if (opt & OPT_WATCHCHILD)
                signal(SIGCHLD, handle_sigchld);
+       else /* prevent dead children from becoming zombies */
+               signal(SIGCHLD, SIG_IGN);
 
 /*
    This is how the buffers are used. The arrows indicate the movement
@@ -497,7 +509,7 @@ int telnetd_main(int argc, char **argv)
        while (ts) {
                struct tsession *next = ts->next; /* in case we free ts. */
                if (ts->shell_pid == -1) {
-                       /* Child died ad we detected that */
+                       /* Child died and we detected that */
                        free_session(ts);
                } else {
                        if (ts->size1 > 0)       /* can write to pty */
@@ -514,7 +526,7 @@ int telnetd_main(int argc, char **argv)
        if (!IS_INETD) {
                FD_SET(master_fd, &rdfdset);
                /* This is needed because free_session() does not
-                * take into account master_fd when it finds new
+                * take master_fd into account when it finds new
                 * maxfd among remaining fd's */
                if (master_fd > maxfd)
                        maxfd = master_fd;

diff --git a/sysklogd/syslogd.c b/sysklogd/syslogd.c
index ba46792b6e6401ec19a0a3acfe33091be452b445..2bf5b5c80442c5a5ad31ebed9a051515fcd90c3d 100644 (file)
--- a/sysklogd/syslogd.c
+++ b/sysklogd/syslogd.c
@@ -381,8 +381,8 @@ static void parse_fac_prio_20(int pri, char *res20)
 }
 
 /* len parameter is used only for "is there a timestamp?" check.
- * NB: some callers cheat and supply 0 when they know
- * that there is no timestamp, short-cutting the test. */
+ * NB: some callers cheat and supply len==0 when they know
+ * that there is no timestamp, short-circuiting the test. */
 static void timestamp_and_log(int pri, char *msg, int len)
 {
        char *timestamp;
@@ -427,10 +427,10 @@ static void split_escape_and_log(char *tmpbuf, int len)
                if (*p == '<') {
                        /* Parse the magic priority number */
                        pri = bb_strtou(p + 1, &p, 10);
-                       if (*p == '>') p++;
-                       if (pri & ~(LOG_FACMASK | LOG_PRIMASK)) {
+                       if (*p == '>')
+                               p++;
+                       if (pri & ~(LOG_FACMASK | LOG_PRIMASK))
                                pri = (LOG_USER | LOG_NOTICE);
-                       }
                }
 
                while ((c = *p++)) {
@@ -526,14 +526,22 @@ static void do_syslogd(void)
 
        for (;;) {
                size_t sz;
-
+ read_again:
                sz = safe_read(sock_fd, G.recvbuf, MAX_READ - 1);
-               if (sz <= 0) {
-                       //if (sz == 0)
-                       //      continue; /* EOF from unix socket??? */
+               if (sz < 0) {
                        bb_perror_msg_and_die("read from /dev/log");
                }
 
+               /* Drop trailing NULs (typically there is one NUL) */
+               while (1) {
+                       if (sz == 0)
+                               goto read_again;
+                       if (G.recvbuf[sz-1])
+                               break;
+                       sz--;
+               }
+               G.recvbuf[sz] = '\0'; /* make sure it *is* NUL terminated */
+
                /* TODO: maybe suppress duplicates? */
 #if ENABLE_FEATURE_REMOTE_LOG
                /* We are not modifying log messages in any way before send */
@@ -549,7 +557,6 @@ static void do_syslogd(void)
                        }
                }
 #endif
-               G.recvbuf[sz] = '\0';
                split_escape_and_log(G.recvbuf, sz);
        } /* for */
 }