From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 2BD823858D35 for ; Mon, 22 May 2023 18:35:19 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 2BD823858D35 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1684780518; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=dKC1/GjvjyHKdH6eBtC0BvjnAeqAtZ6i+c9Vyf6ztnU=; b=QnbVs0GFirsNjs3C0iWmFLIYKdz1DUnm+NfnWvgD7cDaaO/GxJn/9aA4/3GlnKRXovN4we yJeq/8VcusT6CSkLOgv2R9IE6xeuM1nV6s+xD0YnnmFXgntv1qiUO789ByqOsMIRrmd3EE 0J81Wf6eqEzmr5zcMMTAaT1azRgnUCM= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-602-xloNZZqANCWkETDm_xNixQ-1; Mon, 22 May 2023 14:35:17 -0400 X-MC-Unique: xloNZZqANCWkETDm_xNixQ-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 1147D85A5BB; Mon, 22 May 2023 18:35:17 +0000 (UTC) Received: from oak (unknown [10.22.10.124]) by smtp.corp.redhat.com (Postfix) with ESMTPS id DBA7C400DFF; Mon, 22 May 2023 18:35:16 +0000 (UTC) Date: Mon, 22 May 2023 14:35:15 -0400 From: Joe Simmons-Talbott To: Paul Eggert Cc: Adhemerval Zanella Netto , libc-alpha@sourceware.org, Gnulib bugs Subject: Re: [PATCH] time: strftime_l: Use malloc rather than an unbounded alloca. Message-ID: <20230522183515.GK176347@oak> References: <20230510195946.3728273-1-josimmon@redhat.com> <830b766a-1280-bb30-c80f-13f17fba6819@linaro.org> <80685184-312b-34f5-3ae4-337c470563c3@cs.ucla.edu> MIME-Version: 1.0 In-Reply-To: <80685184-312b-34f5-3ae4-337c470563c3@cs.ucla.edu> X-Scanned-By: MIMEDefang 3.1 on 10.11.54.9 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-11.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Wed, May 17, 2023 at 10:40:05AM -0700, Paul Eggert wrote: > On 2023-05-17 04:04, Adhemerval Zanella Netto wrote: > > do you think it would be feasible to assume > > _POSIX_TZNAME_MAX or 48 (for Factory zone) to avoid the malloc call on > > strftime_l? > > ALthough it's technically feasible, it'd be some work. Among other things, > whatever limit we chose would have to become visible via and > sysconf and etc., and we'd need to document that there is now a limit. > Better, I think, would be to continue to follow the GNU coding standards and > not impose an arbitrary limit on time zone abbreviation length, even for the > highly unusual case where code is calling wcsftime or wcsftime_l. > > How about avoiding the malloc in the following way? I installed the attached > patch into Gnulib lib/nstrftime.c, which has forked from glibc but with some > work could be merged back in, and this should also fix the glibc bugs with > buffer sizes exceeding INT_MAX. > > If you don't want all the hassle of merging, you can cherry-pick this patch. > I haven't tested the patch, though, as Gnulib does not use this code. I've pushed your suggested patch after testing on x86_64 and i686. [1] Thanks, Joe [1] https://sourceware.org/pipermail/libc-alpha/2023-May/148384.html > From 197eec6075bbaf2b97137115a09a6bbce43b4dd4 Mon Sep 17 00:00:00 2001 > From: Paul Eggert > Date: Wed, 17 May 2023 10:27:40 -0700 > Subject: [PATCH] nstrftime: suggest to glibc how to avoid alloca > > * lib/nstrftime.c (widen) [COMPILE_WIDE]: Remove. > (__strftime_internal) [COMPILE_WIDE): Instead of converting the > multibyte time zone abbreviation into a potentially unbounded > alloca buffer, convert it directly into the output buffer. > Although this code is not used in Gnulib, this can help the glibc > developers avoid the problem on the glibc side. > --- > ChangeLog | 10 ++++++++++ > lib/nstrftime.c | 39 +++++++++++++++++++++++++-------------- > 2 files changed, 35 insertions(+), 14 deletions(-) > > diff --git a/ChangeLog b/ChangeLog > index ecbc25ef06..36b3c65b81 100644 > --- a/ChangeLog > +++ b/ChangeLog > @@ -1,3 +1,13 @@ > +2023-05-17 Paul Eggert > + > + nstrftime: suggest to glibc how to avoid alloca > + * lib/nstrftime.c (widen) [COMPILE_WIDE]: Remove. > + (__strftime_internal) [COMPILE_WIDE): Instead of converting the > + multibyte time zone abbreviation into a potentially unbounded > + alloca buffer, convert it directly into the output buffer. > + Although this code is not used in Gnulib, this can help the glibc > + developers avoid the problem on the glibc side. > + > 2023-05-15 Bruno Haible > > doc: New chapter "Strings and Characters". > diff --git a/lib/nstrftime.c b/lib/nstrftime.c > index 68bb560910..35a9307e1a 100644 > --- a/lib/nstrftime.c > +++ b/lib/nstrftime.c > @@ -226,15 +226,6 @@ extern char *tzname[]; > # undef __mbsrtowcs_l > # define __mbsrtowcs_l(d, s, l, st, loc) __mbsrtowcs (d, s, l, st) > # endif > -# define widen(os, ws, l) \ > - { \ > - mbstate_t __st; \ > - const char *__s = os; \ > - memset (&__st, '\0', sizeof (__st)); \ > - l = __mbsrtowcs_l (NULL, &__s, 0, &__st, loc); \ > - ws = (wchar_t *) alloca ((l + 1) * sizeof (wchar_t)); \ > - (void) __mbsrtowcs_l (ws, &__s, l, &__st, loc); \ > - } > #endif > > > @@ -1374,11 +1365,31 @@ __strftime_internal (STREAM_OR_CHAR_T *s, STRFTIME_ARG (size_t maxsize) > #ifdef COMPILE_WIDE > { > /* The zone string is always given in multibyte form. We have > - to transform it first. */ > - wchar_t *wczone; > - size_t len; > - widen (zone, wczone, len); > - cpy (len, wczone); > + to convert it to wide character. */ > + size_t w = pad == L_('-') || width < 0 ? 0 : width; > + char const *z = zone; > + mbstate_t st = {0}; > + size_t len = __mbsrtowcs_l (p, &z, maxsize - i, &st, loc); > + if (len == (size_t) -1) > + return 0; > + size_t incr = len < w ? w : len; > + if (incr >= maxsize - i) > + { > + errno = ERANGE; > + return 0; > + } > + if (p) > + { > + if (len < w) > + { > + size_t delta = w - len; > + wmemmove (p + delta, p, len); > + wchar_t wc = pad == L_('0') || pad == L_('+') ? L'0' : L' '; > + wmemset (p, wc, delta); > + } > + p += incr; > + } > + i += incr; > } > #else > cpy (strlen (zone), zone); > -- > 2.39.2 >