From owner-svn-src-all@freebsd.org Thu Dec 22 13:46:18 2016 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id B7278C8A289; Thu, 22 Dec 2016 13:46:18 +0000 (UTC) (envelope-from hrs@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 913C918D6; Thu, 22 Dec 2016 13:46:18 +0000 (UTC) (envelope-from hrs@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id uBMDkHhr060781; Thu, 22 Dec 2016 13:46:17 GMT (envelope-from hrs@FreeBSD.org) Received: (from hrs@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id uBMDkH4h060780; Thu, 22 Dec 2016 13:46:17 GMT (envelope-from hrs@FreeBSD.org) Message-Id: <201612221346.uBMDkH4h060780@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: hrs set sender to hrs@FreeBSD.org using -f From: Hiroki Sato Date: Thu, 22 Dec 2016 13:46:17 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r310393 - head/usr.sbin/syslogd X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 22 Dec 2016 13:46:18 -0000 Author: hrs Date: Thu Dec 22 13:46:17 2016 New Revision: 310393 URL: https://svnweb.freebsd.org/changeset/base/310393 Log: - Fix a use-after-free bug when dq_timeout == 1 and sending SIGTERM to the process failed. It is an unusual situation but it can happen. - Split deadq_remove() into deadq_remove() and deadq_removebypid(). - Normalize variable names of struct deadq_entry *. Modified: head/usr.sbin/syslogd/syslogd.c Modified: head/usr.sbin/syslogd/syslogd.c ============================================================================== --- head/usr.sbin/syslogd/syslogd.c Thu Dec 22 13:38:50 2016 (r310392) +++ head/usr.sbin/syslogd/syslogd.c Thu Dec 22 13:46:17 2016 (r310393) @@ -236,9 +236,6 @@ static TAILQ_HEAD(, deadq_entry) deadq_h #define DQ_TIMO_INIT 2 -typedef struct deadq_entry *dq_t; - - /* * Struct to hold records of network addresses that are allowed to log * to us. @@ -334,7 +331,8 @@ static int addsock(struct sockaddr *, so static struct filed *cfline(const char *, const char *, const char *); static const char *cvthname(struct sockaddr *); static void deadq_enter(pid_t, const char *); -static int deadq_remove(pid_t); +static int deadq_remove(struct deadq_entry *); +static int deadq_removebypid(pid_t); static int decode(const char *, const CODE *); static void die(int) __dead2; static void dodie(int); @@ -1509,7 +1507,7 @@ reapchild(int signo __unused) continue; /* First, look if it's a process from the dead queue. */ - if (deadq_remove(pid)) + if (deadq_removebypid(pid)) continue; /* Now, look in list of active processes. */ @@ -2252,7 +2250,7 @@ static void markit(void) { struct filed *f; - dq_t q, next; + struct deadq_entry *dq, *dq0; now = time((time_t *)NULL); MarkSeq += TIMERINTVL; @@ -2273,14 +2271,12 @@ markit(void) } /* Walk the dead queue, and see if we should signal somebody. */ - for (q = TAILQ_FIRST(&deadq_head); q != NULL; q = next) { - next = TAILQ_NEXT(q, dq_entries); - - switch (q->dq_timeout) { + TAILQ_FOREACH_SAFE(dq, &deadq_head, dq_entries, dq0) { + switch (dq->dq_timeout) { case 0: /* Already signalled once, try harder now. */ - if (kill(q->dq_pid, SIGKILL) != 0) - (void)deadq_remove(q->dq_pid); + if (kill(dq->dq_pid, SIGKILL) != 0) + (void)deadq_remove(dq); break; case 1: @@ -2292,12 +2288,13 @@ markit(void) * didn't even really exist, in case we simply * drop it from the dead queue). */ - if (kill(q->dq_pid, SIGTERM) != 0) - (void)deadq_remove(q->dq_pid); - /* FALLTHROUGH */ - + if (kill(dq->dq_pid, SIGTERM) != 0) + (void)deadq_remove(dq); + else + dq->dq_timeout--; + break; default: - q->dq_timeout--; + dq->dq_timeout--; } } MarkSet = 0; @@ -2738,7 +2735,7 @@ p_open(const char *prog, pid_t *rpid) static void deadq_enter(pid_t pid, const char *name) { - dq_t p; + struct deadq_entry *dq; int status; /* @@ -2752,36 +2749,42 @@ deadq_enter(pid_t pid, const char *name) return; } - p = malloc(sizeof(struct deadq_entry)); - if (p == NULL) { + dq = malloc(sizeof(*dq)); + if (dq == NULL) { logerror("malloc"); exit(1); } - *p = (struct deadq_entry){ + *dq = (struct deadq_entry){ .dq_pid = pid, .dq_timeout = DQ_TIMO_INIT }; - TAILQ_INSERT_TAIL(&deadq_head, p, dq_entries); + TAILQ_INSERT_TAIL(&deadq_head, dq, dq_entries); } static int -deadq_remove(pid_t pid) +deadq_remove(struct deadq_entry *dq) { - dq_t q; - - TAILQ_FOREACH(q, &deadq_head, dq_entries) { - if (q->dq_pid == pid) - break; - } - if (q != NULL) { - TAILQ_REMOVE(&deadq_head, q, dq_entries); - free(q); + if (dq != NULL) { + TAILQ_REMOVE(&deadq_head, dq, dq_entries); + free(dq); return (1); } return (0); } +static int +deadq_removebypid(pid_t pid) +{ + struct deadq_entry *dq; + + TAILQ_FOREACH(dq, &deadq_head, dq_entries) { + if (dq->dq_pid == pid) + break; + } + return (deadq_remove(dq)); +} + static void log_deadchild(pid_t pid, int status, const char *name) {