From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/10405 Path: news.gmane.org!.POSTED!not-for-mail From: Noam Meltzer Newsgroups: gmane.linux.lib.musl.general Subject: Re: [PATCH] fix strdupa evaulating expression twice Date: Sun, 28 Aug 2016 16:53:50 +0000 Message-ID: References: <20160828150816.GF15995@brightrain.aerifal.cx> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/alternative; boundary=089e01183a502435ad053b249800 X-Trace: blaine.gmane.org 1472403259 4420 195.159.176.226 (28 Aug 2016 16:54:19 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Sun, 28 Aug 2016 16:54:19 +0000 (UTC) To: musl Original-X-From: musl-return-10418-gllmg-musl=m.gmane.org@lists.openwall.com Sun Aug 28 18:54:15 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 1be3La-0000bF-00 for gllmg-musl@m.gmane.org; Sun, 28 Aug 2016 18:54:14 +0200 Original-Received: (qmail 15998 invoked by uid 550); 28 Aug 2016 16:54:13 -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 15975 invoked from network); 28 Aug 2016 16:54:12 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:references:in-reply-to:from:date:message-id:subject:to; bh=6y9IW1GDBYyy/zPEhe06U0am/eMqA/4Cy/Q6QWp//AE=; b=Ld85dgNWb+RAxjEqRwekKG5iXZWOy5YsZS7WDm+X+bhxoZT4A94hPjXE/7cQ33nai3 V5lo/lsrIAklPPT3i76q/T6krnMOy0OGHdAxXU5HGOp1bisx70wsIpXTeph82lqLDTSd WG2Hok/uiSreHyEpVOlfQUYtMC7paArQdkrd4Sk4zg3DIqe96hN3WyLc57Mpog/o3h/C L/r9PFLStaeiCQlrslNt1IsA2MxJ6dOyRFX8VXqFtyz5MP2vRqsiHJwS5934cC4nXX16 2Nd9bgovfxP/l1MAn76NS2+LCdDShL2TuIHmSpk44dpnWg/g9/EZCzX0IG/l/9niWJGq ES3g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to; bh=6y9IW1GDBYyy/zPEhe06U0am/eMqA/4Cy/Q6QWp//AE=; b=ltrnhwT6Mnj62P/ijQkAuVcNfUL/2ewd0RwevXYz0jCtGxF+pHHgD+Ttqi2wTbummJ x3HgHHgr12Xx5RkL+7j4DCgxMOXQHUWlL17ZNFdYKVuoweKyms6OPtwxsFKPff93Rkj8 WTsc6QPg/pXf7L/sacDM+inL5ATnG9xY4wSnoCIYbTHnsMjEDIjsPZ2TLxAB2WcXm8fq zcMJuOOidEgNh0PcITT33L+ERXREuqdLOtlw4oO+292Yo+6Fi9NOe86Ao4fZ6y3nOGYS 9aVSyzVTfmx4btJ5oIaGmPfMA2uHpuZoM7gGQOv1KP1oSTym0jjMKl68aBAHFSXMhjmZ jEmQ== X-Gm-Message-State: AE9vXwPdlyMt/K59ECMNjGv4DyclwNQfpp2KLXN8rY/Qw5bcsrqBBdQoBMNk9YQicVfSSNxzhYZTBU6qSJLoiw== X-Received: by 10.194.166.37 with SMTP id zd5mr13563508wjb.170.1472403240922; Sun, 28 Aug 2016 09:54:00 -0700 (PDT) In-Reply-To: <20160828150816.GF15995@brightrain.aerifal.cx> Xref: news.gmane.org gmane.linux.lib.musl.general:10405 Archived-At: --089e01183a502435ad053b249800 Content-Type: text/plain; charset=UTF-8 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. 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 --089e01183a502435ad053b249800 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable


On Sun= , Aug 28, 2016 at 6:08 PM Rich Felker <dalias@libc.org> 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 <tsnoam@gmail.com>
> 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.
> ---
>=C2=A0 include/string.h | 5 ++++-
>=C2=A0 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);
>=C2=A0 #endif
>
>=C2=A0 #ifdef _GNU_SOURCE
> -#define=C2=A0 =C2=A0 =C2=A0 strdupa(x)=C2=A0 =C2=A0 =C2=A0 strcpy(all= oca(strlen(x)+1),x)
> +#define=C2=A0 =C2=A0 =C2=A0 strdupa(x) (__extension__ ({ \
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0const char *__xval = =3D x; \
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0strcpy(alloca(strlen(= __xval)+1),__xval); \
> +=C2=A0 =C2=A0 =C2=A0}))
>=C2=A0 int strverscmp (const char *, const char *);
>=C2=A0 int strcasecmp_l (const char *, const char *, locale_t);
>=C2=A0 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 essentia= lly
always-unsafe and should probably be removed or at least made into a
warning...

I understand what you're saying and I te= nd to agree. However:
=C2=A0* The entire section of the code is wrapped = with the _GNU_SOURCE test macro (so the __extension__ trick should work).=C2=A0* IMHO, if strdupa is to kept, it should at least provide expected= behaviour.

p.s.
I spent about a day of work chasing a bug caused by the c= urrent implementation. So what I actually looking for is to save this pain = from others one way or another.

=C2= =A0- Noam
--089e01183a502435ad053b249800--