From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/10406 Path: news.gmane.org!.POSTED!not-for-mail From: Bobby Bingham Newsgroups: gmane.linux.lib.musl.general Subject: Re: [PATCH] fix strdupa evaulating expression twice Date: Sun, 28 Aug 2016 12:04:38 -0500 Message-ID: <20160828170438.dhcbk2my2gkcp7ot@gordon> References: <20160828150816.GF15995@brightrain.aerifal.cx> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 X-Trace: blaine.gmane.org 1472403898 29798 195.159.176.226 (28 Aug 2016 17:04:58 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Sun, 28 Aug 2016 17:04:58 +0000 (UTC) User-Agent: Mutt/1.6.2 (2016-06-11) To: musl@lists.openwall.com Original-X-From: musl-return-10419-gllmg-musl=m.gmane.org@lists.openwall.com Sun Aug 28 19:04:51 2016 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by blaine.gmane.org with smtp (Exim 4.84_2) (envelope-from ) id 1be3Vr-0007An-60 for gllmg-musl@m.gmane.org; Sun, 28 Aug 2016 19:04:51 +0200 Original-Received: (qmail 23787 invoked by uid 550); 28 Aug 2016 17:04:50 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Original-Received: (qmail 23769 invoked from network); 28 Aug 2016 17:04:49 -0000 Content-Disposition: inline In-Reply-To: X-Operating-System: Linux gordon 3.18.1-gentoo Xref: news.gmane.org gmane.linux.lib.musl.general:10406 Archived-At: On Sun, Aug 28, 2016 at 04:53:50PM +0000, Noam Meltzer wrote: > On Sun, Aug 28, 2016 at 6:08 PM Rich Felker wrote: > > > On Sun, Aug 28, 2016 at 10:59:26AM +0000, Noam Meltzer wrote: > > > > > > > > From bf23b7b8fd39eaca6a05173eaf543e1bce3319ab Mon Sep 17 00:00:00 2001 > > > From: Noam Meltzer > > > Date: Sun, 28 Aug 2016 13:53:24 +0300 > > > Subject: [PATCH] fix strdupa evaulating expression twice > > > > > > calling strdupa with va_arg as its expression caused unexpected > > > behaviour. now the expression is evaulated only once. > > > --- > > > include/string.h | 5 ++++- > > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > > > diff --git a/include/string.h b/include/string.h > > > index ff9badb..976faaf 100644 > > > --- a/include/string.h > > > +++ b/include/string.h > > > @@ -85,7 +85,10 @@ size_t strlcpy (char *, const char *, size_t); > > > #endif > > > > > > #ifdef _GNU_SOURCE > > > -#define strdupa(x) strcpy(alloca(strlen(x)+1),x) > > > +#define strdupa(x) (__extension__ ({ \ > > > + const char *__xval = x; \ > > > + strcpy(alloca(strlen(__xval)+1),__xval); \ > > > + })) > > > int strverscmp (const char *, const char *); > > > int strcasecmp_l (const char *, const char *, locale_t); > > > int strncasecmp_l (const char *, const char *, size_t, locale_t); > > > > The intent of the form as written is to be actual C (modulo use of > > alloca) rather than "GNU C". Aside from that, strdupa is essentially > > always-unsafe and should probably be removed or at least made into a > > warning... > > > > I understand what you're saying and I tend to agree. However: > * The entire section of the code is wrapped with the _GNU_SOURCE test > macro (so the __extension__ trick should work). > * IMHO, if strdupa is to kept, it should at least provide expected > behaviour. _GNU_SOURCE says that the program has requested glibc extensions, like strdupa. It does not imply anything about the compiler being used and whether it implements GCC extensions. That said, it looks like glibc only defines strdupa #if __GNUC__, so this function is simply unavailable when using a compiler that doesn't implement this extension. I'm not sure what the musl policy is here, but maybe we could do something similar? > > p.s. > I spent about a day of work chasing a bug caused by the current > implementation. So what I actually looking for is to save this pain from > others one way or another. > > - Noam