From owner-freebsd-standards@FreeBSD.ORG Mon Oct 4 11:02:19 2004 Return-Path: Delivered-To: freebsd-standards@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id EB6BB16A4D8 for ; Mon, 4 Oct 2004 11:02:18 +0000 (GMT) Received: from freefall.freebsd.org (freefall.freebsd.org [216.136.204.21]) by mx1.FreeBSD.org (Postfix) with ESMTP id D832943D4C for ; Mon, 4 Oct 2004 11:02:18 +0000 (GMT) (envelope-from owner-bugmaster@freebsd.org) Received: from freefall.freebsd.org (peter@localhost [127.0.0.1]) by freefall.freebsd.org (8.12.11/8.12.11) with ESMTP id i94B2IPP031523 for ; Mon, 4 Oct 2004 11:02:18 GMT (envelope-from owner-bugmaster@freebsd.org) Received: (from peter@localhost) by freefall.freebsd.org (8.12.11/8.12.11/Submit) id i94B2IZF031517 for freebsd-standards@freebsd.org; Mon, 4 Oct 2004 11:02:18 GMT (envelope-from owner-bugmaster@freebsd.org) Date: Mon, 4 Oct 2004 11:02:18 GMT Message-Id: <200410041102.i94B2IZF031517@freefall.freebsd.org> X-Authentication-Warning: freefall.freebsd.org: peter set sender to owner-bugmaster@freebsd.org using -f From: FreeBSD bugmaster To: freebsd-standards@FreeBSD.org Subject: Current problem reports assigned to you X-BeenThere: freebsd-standards@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Standards compliance List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 04 Oct 2004 11:02:19 -0000 Current FreeBSD problem reports Critical problems Serious problems S Submitted Tracker Resp. Description ------------------------------------------------------------------------------- o [2001/03/05] bin/25542 standards /bin/sh: null char in quoted string p [2002/02/25] standards/35307standards standard include files are not standard c o [2002/12/13] kern/46239 standards posix semaphore implementation errors o [2003/04/21] standards/51209standards [PATCH] add a64l()/l64a/l64a_r functions p [2003/06/05] standards/52972standards /bin/sh arithmetic not POSIX compliant o [2003/06/18] kern/53447 standards poll(2) semantics differ from susV3/POSIX o [2003/07/12] standards/54410standards one-true-awk not POSIX compliant (no exte o [2003/09/15] standards/56906standards Several math(3) functions fail to set err o [2004/01/01] standards/60772standards _Bool and bool should be unsigned o [2004/09/22] standards/72006standards floating point formating in non-C locales 10 problems total. Non-critical problems S Submitted Tracker Resp. Description ------------------------------------------------------------------------------- f [1995/01/11] kern/105 standards Distributed libm (msun) has non-standard o [2000/09/24] bin/21519 standards sys/dir.h should be deprecated some more o [2001/01/16] bin/24390 standards Replacing old dir-symlinks when using /bi s [2001/01/24] standards/24590standards timezone function not compatible witn Sin s [2001/06/18] kern/28260 standards UIO_MAXIOV needs to be made public p [2001/11/20] standards/32126standards getopt(3) not Unix-98 conformant o [2002/02/27] misc/35381 standards incorrect floating-point display of large s [2002/03/19] standards/36076standards Implementation of POSIX fuser command o [2002/06/14] standards/39256standards [v]snprintf aren't POSIX-conformant for s o [2002/07/09] kern/40378 standards stdlib.h gives needless warnings with -an p [2002/08/12] standards/41576standards POSIX compliance of ln(1) o [2002/10/23] standards/44425standards getcwd() succeeds even if current dir has o [2002/12/09] standards/46119standards Priority problems for SCHED_OTHER using p o [2002/12/23] standards/46504standards Warnings in headers o [2003/06/22] standards/53613standards FreeBSD doesn't define EPROTO o [2003/07/24] standards/54809standards pcvt deficits o [2003/07/25] standards/54833standards more pcvt deficits o [2003/07/25] standards/54839standards pcvt deficits o [2003/07/31] standards/55112standards glob.h, glob_t's gl_pathc should be "size o [2003/09/05] standards/56476standards cd9660 unicode support simple hack o [2003/10/29] standards/58676standards grantpt(3) alters storage used by ptsname p [2003/12/26] standards/60597standards FreeBSD's /usr/include lacks of cpio.h s [2004/02/14] standards/62858standards malloc(0) not C99 compliant p [2004/02/21] standards/63173standards Patch to add getopt_long_only(3) to libc o [2004/03/29] kern/64875 standards [patch] add a system call: fdatasync() o [2004/05/07] standards/66357standards make POSIX conformance problem ('sh -e' & o [2004/05/11] standards/66531standards _gettemp uses a far smaller set of filena o [2004/08/22] standards/70813standards [PATCH] ls not Posix compliant o [2004/08/26] docs/70985 standards [patch] sh(1): incomplete documentation o o [2004/09/25] standards/72076standards [patch] German locales use old %d.%m.%y d 30 problems total. From owner-freebsd-standards@FreeBSD.ORG Wed Oct 6 17:30:27 2004 Return-Path: Delivered-To: freebsd-standards@hub.freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id C42F516A4CE for ; Wed, 6 Oct 2004 17:30:27 +0000 (GMT) Received: from freefall.freebsd.org (freefall.freebsd.org [216.136.204.21]) by mx1.FreeBSD.org (Postfix) with ESMTP id A43D543D31 for ; Wed, 6 Oct 2004 17:30:27 +0000 (GMT) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) i96HURtK081257 for ; Wed, 6 Oct 2004 17:30:27 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.12.11/8.12.11/Submit) id i96HURuw081255; Wed, 6 Oct 2004 17:30:27 GMT (envelope-from gnats) Resent-Date: Wed, 6 Oct 2004 17:30:27 GMT Resent-Message-Id: <200410061730.i96HURuw081255@freefall.freebsd.org> Resent-From: FreeBSD-gnats-submit@FreeBSD.org (GNATS Filer) Resent-To: freebsd-standards@FreeBSD.org Resent-Reply-To: FreeBSD-gnats-submit@FreeBSD.org, Dan Nelson Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id D829316A4CE for ; Wed, 6 Oct 2004 17:23:41 +0000 (GMT) Received: from dan.emsphone.com (dan.emsphone.com [199.67.51.101]) by mx1.FreeBSD.org (Postfix) with ESMTP id 6497243D45 for ; Wed, 6 Oct 2004 17:23:41 +0000 (GMT) (envelope-from dan@dan.emsphone.com) Received: (from dan@localhost) by dan.emsphone.com (8.12.11/8.12.11) id i96HNeT0099891; Wed, 6 Oct 2004 12:23:40 -0500 (CDT) (envelope-from dan) Message-Id: <200410061723.i96HNeT0099891@dan.emsphone.com> Date: Wed, 6 Oct 2004 12:23:40 -0500 (CDT) From: Dan Nelson To: FreeBSD-gnats-submit@FreeBSD.org X-Send-Pr-Version: 3.113 Subject: standards/72394: [PATCH] syslog is not thread-safe X-BeenThere: freebsd-standards@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Standards compliance List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 06 Oct 2004 17:30:27 -0000 >Number: 72394 >Category: standards >Synopsis: [PATCH] syslog is not thread-safe >Confidential: no >Severity: non-critical >Priority: low >Responsible: freebsd-standards >State: open >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Wed Oct 06 17:30:26 GMT 2004 >Closed-Date: >Last-Modified: >Originator: Dan Nelson >Release: FreeBSD 5.3-BETA7 i386 >Organization: The Allant Group, Inc. >Environment: System: FreeBSD dan.emsphone.com 5.3-BETA7 FreeBSD 5.3-BETA7 #361: Tue Oct 5 16:17:41 CDT 2004 zsh@dan.emsphone.com:/usr/src/sys/i386/compile/DANSMP i386 >Description: The syslog functions are not in the list of thread-unsafe functions at http://www.opengroup.org/onlinepubs/009695399/functions/xsh_chap02_09.html#tag_02_09_01 , so they are supposed to be safe. The log fd is not locked, though, so simultanteous calls to openlog/syslog/closelog may result in lost entries or lost fds. >How-To-Repeat: Compile this with kse or thr threads, run it, and count how many log entries end up in /var/log/messages. You may end up with missing lines, or kernel messages saying "unp_connect(): lost race to another thread". The closelog() forces an openlog() on the next call to syslog(). A more likely failure case would be if someone bounced syslogd, which will force every process with an open log fd to reopen it. If openlog/closelog is never called from within a thread and the fd stays open, syslog() itself is already thread-safe. #include #include #include void *logit(void *arg) { char *threadnum = arg; int i; for (i = 0; i < 100; i++) { syslog(LOG_WARNING, "I'm thread %s, log entry %03d", threadnum, i+1); closelog(); } return 0; } int main(void) { pthread_t t1, t2; pthread_create(&t1, NULL, &logit, "1"); pthread_create(&t2, NULL, &logit, "2"); pthread_join(t1, NULL); pthread_join(t2, NULL); syslog(LOG_WARNING, "Done"); return 0; } >Fix: Index: lib/libc/gen/syslog.c =================================================================== RCS file: /home/ncvs/src/lib/libc/gen/syslog.c,v retrieving revision 1.30 diff -u -p -r1.30 syslog.c --- lib/libc/gen/syslog.c 10 May 2004 17:12:52 -0000 1.30 +++ lib/libc/gen/syslog.c 6 Oct 2004 17:05:42 -0000 @@ -48,6 +48,7 @@ __FBSDID("$FreeBSD: src/lib/libc/gen/sys #include #include #include +#include #include #include #include @@ -62,10 +63,20 @@ __FBSDID("$FreeBSD: src/lib/libc/gen/sys static int LogFile = -1; /* fd for log */ static int connected; /* have done connect */ static int opened; /* have done openlog() */ -static int LogStat = 0; /* status bits, set by openlog() */ +static int LogStat; /* status bits, set by openlog() */ static const char *LogTag = NULL; /* string to tag the entry with */ static int LogFacility = LOG_USER; /* default facility code */ static int LogMask = 0xff; /* mask of priorities to be logged */ +static pthread_mutex_t syslog_mutex = PTHREAD_MUTEX_INITIALIZER; + +#define THREAD_LOCK() \ + do { \ + if (__isthreaded) _pthread_mutex_lock(&syslog_mutex); \ + } while(0) +#define THREAD_UNLOCK() \ + do { \ + if (__isthreaded) _pthread_mutex_unlock(&syslog_mutex); \ + } while(0) static void disconnectlog(void); /* disconnect from syslogd */ static void connectlog(void); /* (re)connect to syslogd */ @@ -141,11 +152,15 @@ vsyslog(pri, fmt, ap) pri &= LOG_PRIMASK|LOG_FACMASK; } + saved_errno = errno; + + THREAD_LOCK(); + /* Check priority against setlogmask values. */ - if (!(LOG_MASK(LOG_PRI(pri)) & LogMask)) + if (!(LOG_MASK(LOG_PRI(pri)) & LogMask)) { + THREAD_UNLOCK(); return; - - saved_errno = errno; + } /* Set default facility if none specified. */ if ((pri & LOG_FACMASK) == 0) @@ -155,8 +170,10 @@ vsyslog(pri, fmt, ap) tbuf_cookie.base = tbuf; tbuf_cookie.left = sizeof(tbuf); fp = fwopen(&tbuf_cookie, writehook); - if (fp == NULL) + if (fp == NULL) { + THREAD_UNLOCK(); return; + } /* Build the message. */ (void)time(&now); @@ -186,6 +203,7 @@ vsyslog(pri, fmt, ap) fmt_fp = fwopen(&fmt_cookie, writehook); if (fmt_fp == NULL) { fclose(fp); + THREAD_UNLOCK(); return; } @@ -243,8 +261,10 @@ vsyslog(pri, fmt, ap) if (!opened) openlog(LogTag, LogStat | LOG_NDELAY, 0); connectlog(); - if (send(LogFile, tbuf, cnt, 0) >= 0) + if (send(LogFile, tbuf, cnt, 0) >= 0) { + THREAD_UNLOCK(); return; + } /* * If the send() failed, the odds are syslogd was restarted. @@ -252,8 +272,10 @@ vsyslog(pri, fmt, ap) */ disconnectlog(); connectlog(); - if (send(LogFile, tbuf, cnt, 0) >= 0) + if (send(LogFile, tbuf, cnt, 0) >= 0) { + THREAD_UNLOCK(); return; + } /* * Output the message to the console; try not to block @@ -274,6 +296,7 @@ vsyslog(pri, fmt, ap) (void)_writev(fd, iov, 2); (void)_close(fd); } + THREAD_UNLOCK(); } static void disconnectlog() @@ -332,6 +355,7 @@ openlog(ident, logstat, logfac) const char *ident; int logstat, logfac; { + THREAD_LOCK(); if (ident != NULL) LogTag = ident; LogStat = logstat; @@ -342,15 +366,18 @@ openlog(ident, logstat, logfac) connectlog(); opened = 1; /* ident and facility has been set */ + THREAD_UNLOCK(); } void closelog() { + THREAD_LOCK(); (void)_close(LogFile); LogFile = -1; LogTag = NULL; connected = 0; + THREAD_UNLOCK(); } /* setlogmask -- set the log mask level */ @@ -360,8 +387,10 @@ setlogmask(pmask) { int omask; + THREAD_LOCK(); omask = LogMask; if (pmask != 0) LogMask = pmask; + THREAD_UNLOCK(); return (omask); } >Release-Note: >Audit-Trail: >Unformatted: From owner-freebsd-standards@FreeBSD.ORG Thu Oct 7 06:01:01 2004 Return-Path: Delivered-To: freebsd-standards@hub.freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 9A72716A56F for ; Thu, 7 Oct 2004 06:01:01 +0000 (GMT) Received: from freefall.freebsd.org (freefall.freebsd.org [216.136.204.21]) by mx1.FreeBSD.org (Postfix) with ESMTP id 8E1A843D2D for ; Thu, 7 Oct 2004 06:01:01 +0000 (GMT) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) i97611iq098348 for ; Thu, 7 Oct 2004 06:01:01 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.12.11/8.12.11/Submit) id i97610GA098343; Thu, 7 Oct 2004 06:01:01 GMT (envelope-from gnats) Date: Thu, 7 Oct 2004 06:01:01 GMT Message-Id: <200410070601.i97610GA098343@freefall.freebsd.org> To: freebsd-standards@FreeBSD.org From: Dan Nelson Subject: Re: standards/72394: [PATCH] syslog is not thread-safe X-BeenThere: freebsd-standards@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list Reply-To: Dan Nelson List-Id: Standards compliance List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 07 Oct 2004 06:01:01 -0000 The following reply was made to PR standards/72394; it has been noted by GNATS. From: Dan Nelson To: FreeBSD-gnats-submit@FreeBSD.org Cc: Subject: Re: standards/72394: [PATCH] syslog is not thread-safe Date: Thu, 7 Oct 2004 00:58:43 -0500 Update: if errno isn't valid, strerror() isn't thread-safe, so here's an addon patch that uses strerror_r instead: --- syslog.c~ Thu Oct 7 00:39:44 2004 +++ syslog.c Thu Oct 7 00:49:06 2004 @@ -139,7 +139,7 @@ vsyslog(pri, fmt, ap) char ch, *p; time_t now; int fd, saved_errno; - char *stdp, tbuf[2048], fmt_cpy[1024], timbuf[26]; + char *stdp, tbuf[2048], fmt_cpy[1024], timbuf[26], errstr[64]; FILE *fp, *fmt_fp; struct bufcookie tbuf_cookie; struct bufcookie fmt_cookie; @@ -215,7 +215,8 @@ vsyslog(pri, fmt, ap) for ( ; (ch = *fmt); ++fmt) { if (ch == '%' && fmt[1] == 'm') { ++fmt; - fputs(strerror(saved_errno), fmt_fp); + strerror_r(saved_errno, errstr, sizeof(errstr)); + fputs(errstr, fmt_fp); } else if (ch == '%' && fmt[1] == '%') { ++fmt; fputc(ch, fmt_fp);