From owner-freebsd-bugs@FreeBSD.ORG Wed Mar 2 15:20:14 2005 Return-Path: Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 939C416A4CE for ; Wed, 2 Mar 2005 15:20:14 +0000 (GMT) Received: from freefall.freebsd.org (freefall.freebsd.org [216.136.204.21]) by mx1.FreeBSD.org (Postfix) with ESMTP id 4CCCB43D39 for ; Wed, 2 Mar 2005 15:20:14 +0000 (GMT) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) by freefall.freebsd.org (8.13.1/8.13.1) with ESMTP id j22FKEep080987 for ; Wed, 2 Mar 2005 15:20:14 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.13.1/8.13.1/Submit) id j22FKE5Y080986; Wed, 2 Mar 2005 15:20:14 GMT (envelope-from gnats) Resent-Date: Wed, 2 Mar 2005 15:20:14 GMT Resent-Message-Id: <200503021520.j22FKE5Y080986@freefall.freebsd.org> Resent-From: FreeBSD-gnats-submit@FreeBSD.org (GNATS Filer) Resent-To: freebsd-bugs@FreeBSD.org Resent-Reply-To: FreeBSD-gnats-submit@FreeBSD.org, Gavin Atkinson Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 3692A16A4CF for ; Wed, 2 Mar 2005 15:16:13 +0000 (GMT) Received: from mail-gw0.york.ac.uk (mail-gw0.york.ac.uk [144.32.128.245]) by mx1.FreeBSD.org (Postfix) with ESMTP id 26B8043D48 for ; Wed, 2 Mar 2005 15:16:11 +0000 (GMT) (envelope-from ga9@buffy.york.ac.uk) Received: from buffy.york.ac.uk (buffy.york.ac.uk [144.32.226.160]) by mail-gw0.york.ac.uk (8.12.10/8.12.10) with ESMTP id j22FG6YX019558 for ; Wed, 2 Mar 2005 15:16:06 GMT Received: from buffy.york.ac.uk (localhost [127.0.0.1]) by buffy.york.ac.uk (8.13.1/8.13.1) with ESMTP id j22FG5ja081679 for ; Wed, 2 Mar 2005 15:16:06 GMT (envelope-from ga9@buffy.york.ac.uk) Received: (from ga9@localhost) by buffy.york.ac.uk (8.13.1/8.13.1/Submit) id j22FG5nL081678; Wed, 2 Mar 2005 15:16:05 GMT (envelope-from ga9) Message-Id: <200503021516.j22FG5nL081678@buffy.york.ac.uk> Date: Wed, 2 Mar 2005 15:16:05 GMT From: Gavin Atkinson To: FreeBSD-gnats-submit@FreeBSD.org X-Send-Pr-Version: 3.113 Subject: bin/78304: Signal handler abuse in comsat(8) X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list Reply-To: Gavin Atkinson List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 02 Mar 2005 15:20:14 -0000 >Number: 78304 >Category: bin >Synopsis: Signal handler abuse in comsat(8) >Confidential: no >Severity: non-critical >Priority: low >Responsible: freebsd-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Wed Mar 02 15:20:13 GMT 2005 >Closed-Date: >Last-Modified: >Originator: Gavin Atkinson >Release: FreeBSD 5.3-STABLE i386 >Organization: >Environment: System: FreeBSD buffy.york.ac.uk 5.3-STABLE FreeBSD 5.3-STABLE #0: Sat Feb 12 20:42:16 GMT 2005 root@buffy.york.ac.uk:/usr/obj/usr/src/sys/BUFFY i386 >Description: I've been inspired by the talk given by Henning Brauer given at the UKUUG Winter 2005 on writing safe signal handlers, and have set about fixing some of them in the FreeBSD source tree. Slides for this (well, an almost identical) presentation are at http://www.openbsd.org/papers/opencon04/index.html The comsat daemon hides particularly nasty abuses of signals. In one signal handler, it failes to save and restore errno around a call that can change it, which will cause problems if the main routine is interrupted at the wrong time. In another signal handler, a large number of functions are called which are not guaranteed to be safe to call within the context of a signal handler. Some of these functions (e.g. realloc()) may be particularly dangerous as the handler could possibly be called while the main routine is already in the memory allocator. This latter signal has been fixed to simply set a flag, which is now polled and acted upon in the main loop. >How-To-Repeat: N/A >Fix: --- comsat-sigs.patch begins here --- Index: src/libexec/comsat/comsat.c =================================================================== RCS file: /usr/cvs/src/libexec/comsat/comsat.c,v retrieving revision 1.17 diff -u -r1.17 comsat.c --- src/libexec/comsat/comsat.c 14 Feb 2005 17:42:56 -0000 1.17 +++ src/libexec/comsat/comsat.c 27 Feb 2005 23:04:47 -0000 @@ -77,11 +77,13 @@ struct utmp *utmp = NULL; time_t lastmsgtime; int nutmp, uf; +volatile sig_atomic_t needreadutmp = 0; void jkfprintf(FILE *, char[], char[], off_t); void mailfor(char *); void notify(struct utmp *, char[], off_t, int); void onalrm(int); +void readutmp(void); void reapchildren(int); int @@ -109,7 +111,7 @@ } (void)time(&lastmsgtime); (void)gethostname(hostname, sizeof(hostname)); - onalrm(0); + readutmp(); (void)signal(SIGALRM, onalrm); (void)signal(SIGTTOU, SIG_IGN); (void)signal(SIGCHLD, reapchildren); @@ -121,6 +123,10 @@ errno = 0; continue; } + if (needreadutmp) { + needreadutmp = 0; + readutmp(); + } if (!nutmp) /* no one has logged in yet */ continue; sigblock(sigmask(SIGALRM)); @@ -134,12 +140,22 @@ void reapchildren(int signo) { - while (wait3(NULL, WNOHANG, NULL) > 0); + int save_errno = errno; + + while (wait3(NULL, WNOHANG, NULL) > 0) + ; + errno = save_errno; } void onalrm(int signo) { + needreadutmp = 1; +} + +void +readutmp(void) +{ static u_int utmpsize; /* last malloced size for utmp */ static u_int utmpmtime; /* last modification time for utmp */ struct stat statbf; --- comsat-sigs.patch ends here --- >Release-Note: >Audit-Trail: >Unformatted: