From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/2958 Path: news.gmane.org!not-for-mail From: Rich Felker Newsgroups: gmane.linux.lib.musl.general Subject: Re: Proposed syslog patch [Re: [musl] Further bugs in syslog()] Date: Sat, 23 Mar 2013 17:27:04 -0400 Message-ID: <20130323212704.GV20323@brightrain.aerifal.cx> References: <20130323034538.GS20323@brightrain.aerifal.cx> <20130323040558.GU20323@brightrain.aerifal.cx> <20130323161755.GN19010@port70.net> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: ger.gmane.org 1364074036 7523 80.91.229.3 (23 Mar 2013 21:27:16 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Sat, 23 Mar 2013 21:27:16 +0000 (UTC) To: musl@lists.openwall.com Original-X-From: musl-return-2959-gllmg-musl=m.gmane.org@lists.openwall.com Sat Mar 23 22:27:43 2013 Return-path: Envelope-to: gllmg-musl@plane.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by plane.gmane.org with smtp (Exim 4.69) (envelope-from ) id 1UJVyY-0005Di-It for gllmg-musl@plane.gmane.org; Sat, 23 Mar 2013 22:27:42 +0100 Original-Received: (qmail 20400 invoked by uid 550); 23 Mar 2013 21:27:17 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: Original-Received: (qmail 20389 invoked from network); 23 Mar 2013 21:27:17 -0000 Content-Disposition: inline In-Reply-To: <20130323161755.GN19010@port70.net> User-Agent: Mutt/1.5.21 (2010-09-15) Xref: news.gmane.org gmane.linux.lib.musl.general:2958 Archived-At: On Sat, Mar 23, 2013 at 05:17:55PM +0100, Szabolcs Nagy wrote: > looks ok, one nitpick > > * Rich Felker [2013-03-23 00:05:58 -0400]: > > void openlog(const char *ident, int opt, int facility) > > @@ -59,7 +54,14 @@ void openlog(const char *ident, int opt, int facility) > > int cs; > > pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &cs); > > LOCK(lock); > > - __openlog(ident, opt, facility); > > + > > + free(log_ident); > > + log_ident = strdup(ident); > > this can fail and then log_ident = 0 Yes, this was the intent; there's no way to report errors, and being in a status of NULL log_ident is the same as the case where it was never set at all. However I agree with your reasoning below... > .... > > @@ -90,14 +89,14 @@ static void _vsyslog(int priority, const char *message, va_list ap) > > priority, timebuf, > > log_ident ? log_ident : "", > > "["+!pid, pid, "]"+!pid); > > + if (l >= sizeof buf) return; > > if ident is longer than buf, then syslog will silently fail here, > but if ident is so big that strdup fails above it does not fail here That it violates the principle of least surprise that moderately long values of log_ident will prevent any log message from appearing, but insanely long ones will be silently replaced with a blank string. > maybe we should limit ident to a sensible value and truncate it > (i dont know if posix allows that but seems more consistent > and then no dynamic allocation is needed in openlog) POSIX provides no way to inspect the output that's produced, so there are no testable conformance requirements on the output... I think I like your approach: just using a small fixed-size buffer. Rich