DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 82388385843A
Date: Thu, 18 Nov 2021 14:19:08 +0100
To: cygwin@cygwin.com
Subject: Re: possible snprintf() regression in 3.3.2
Message-ID: <YZZSzPhbqF6cQAiu@calimero.vinschen.de>
Mail-Followup-To: cygwin@cygwin.com
References: <20211117003718.GF10332@venus.tony.develop-help.com>
 <20211117182108.b38599f5e13071bf269a0d48@nifty.ne.jp>
 <YZT1S8wDnaBuYf5u@calimero.vinschen.de>
 <20211118000649.GG10332@venus.tony.develop-help.com>
 <20211118203538.a049809d57731fe375801c15@nifty.ne.jp>
MIME-Version: 1.0
Content-Disposition: inline
In-Reply-To: <20211118203538.a049809d57731fe375801c15@nifty.ne.jp>
Precedence: list
From: Corinna Vinschen via Cygwin <cygwin@cygwin.com>
Reply-To: cygwin@cygwin.com
Cc: Corinna Vinschen <corinna-cygwin@cygwin.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: cygwin-bounces+archive-cygwin=delorie.com@cygwin.com
Sender: "Cygwin" <cygwin-bounces+archive-cygwin=delorie.com@cygwin.com>

On Nov 18 20:35, Takashi Yano via Cygwin wrote:
> On Thu, 18 Nov 2021 11:06:49 +1100
> Tony Cook wrote:
> > On Wed, Nov 17, 2021 at 01:27:55PM +0100, Corinna Vinschen via Cygwin wrote:
> > > I don't have a good solution.  The old ldtoa code is lacking, for
> > > switching newlib to gdtoa I simply don't have the time.  On the newlib
> > > list was a short discussion starting at
> > > https://sourceware.org/pipermail/newlib/2021/018626.html but nothing
> > > came out of it yet.
> > > 
> > > Patches gratefully accepted (except just reverting the above change).
> > 
> > From what I can tell the problem has nothing to do with the extra
> > precision, but has to do with misusing ndigits for the buffer size
> > with a %f format string, leading to a buffer overflow.
> > 
> > At entry to _ldtoa_r() ndigits is 9, but for a %f format with a large
> > number the number of digits is more closely related to the magnitude
> > of the number, not ndigits.
> > 
> > With the input number (9e99) and the supplied format I'd expect 109
> > characters output, but outbuf is only:
> > 
> >    ndigits + MAX_EXP_DIGITS + 10 = 9 + 5 + 10 = 24
> > 
> > characters in length.
> 
> Then, isn't the following the right thing?
> 
> diff --git a/newlib/libc/stdlib/ldtoa.c b/newlib/libc/stdlib/ldtoa.c
> index 7da61457b..826a1b2ed 100644
> --- a/newlib/libc/stdlib/ldtoa.c
> +++ b/newlib/libc/stdlib/ldtoa.c
> @@ -2794,6 +2794,7 @@ _ldtoa_r (struct _reent *ptr, long double d, int mode, int ndigits,
>    LDPARMS rnd;
>    LDPARMS *ldp = &rnd;
>    char *outstr;
> +  char outbuf[NDEC + MAX_EXP_DIGITS + 10];
>    union uconv du;
>    du.d = d;
>  
> @@ -2840,8 +2841,6 @@ _ldtoa_r (struct _reent *ptr, long double d, int mode, int ndigits,
>    if (ndigits > NDEC)
>      ndigits = NDEC;
>  
> -  char outbuf[ndigits + MAX_EXP_DIGITS + 10];
> -
>    etoasc (e, outbuf, ndigits, mode, ldp);
>    s = outbuf;
>    if (eisinf (e) || eisnan (e))

Ouch.

My patch raised NDEC from 43 to 1023 to allow aproximately the same
number of digits as glibc.  Newlib strives to support embedded targets
and bare metal.  Some of them are lucky if they have a stack size of 1K.
The outbuf buffer is created on the stack, so I used ndigits to save
stack space.

While that patch fixes the reported problem, it will make users of
smaller-than-Cygwin targets pretty unhappy.

A workaround would be to malloc outbuf instead.  Given that printf
doesn't work without malloc anyway, that might be a working workaround,
until somebody takes heart and provides newlib with a new ldtoa
solution.

Either way, this discussion should better take place on the newlib
mailing list, given the embedded stakeholders on this list, ideally
as reply to

https://sourceware.org/pipermail/newlib/2021/018626.html

If push comes to shove, we probably have to revert my patch for the time
being, I guess.


Thanks,
Corinna

-- 
Problem reports:      https://cygwin.com/problems.html
FAQ:                  https://cygwin.com/faq/
Documentation:        https://cygwin.com/docs.html
Unsubscribe info:     https://cygwin.com/ml/#unsubscribe-simple