Message ID | 87eemtyjam.fsf@oldenburg2.str.redhat.com |
---|---|
State | New |
Headers | show |
Series | x86: Harden printf against non-normal long double values (bug 26649) | expand |
On Tue, Sep 22, 2020 at 7:33 AM Florian Weimer via Libc-alpha <libc-alpha@sourceware.org> wrote: > > The behavior of isnan/__builtin_isnan on bit patterns that do not > correspond to something that the CPU would produce from valid inputs > is currently under-defined in the toolchain. (The GCC built-in and > glibc disagree.) > > The isnan check in PRINTF_FP_FETCH in stdio-common/printf_fp.c > assumes the GCC behavior that returns true for non-normal numbers > which are not specified as NaN. (The glibc implementation returns > false for such numbers.) > > At present, passing non-normal numbers to __mpn_extract_long_double > causes this function to produce irregularly shaped multi-precision > integers, triggering undefined behavior in __printf_fp_l. > > With GCC 10 and glibc 2.32, this behavior is not visible because > __builtin_isnan is used, which avoids calling > __mpn_extract_long_double in this case. This commit updates the > implementation of __mpn_extract_long_double so that regularly shaped > multi-precision integers are produced in this case, avoiding > undefined behavior in __printf_fp_l. > > --- > sysdeps/i386/ldbl2mpn.c | 7 +++++ > sysdeps/x86/Makefile | 4 +++ > sysdeps/x86/tst-ldbl-nonnormal-printf.c | 52 +++++++++++++++++++++++++++++++++ > 3 files changed, 63 insertions(+) > > diff --git a/sysdeps/i386/ldbl2mpn.c b/sysdeps/i386/ldbl2mpn.c > index ec8464eef7..867c66ef8d 100644 > --- a/sysdeps/i386/ldbl2mpn.c > +++ b/sysdeps/i386/ldbl2mpn.c > @@ -115,6 +115,13 @@ __mpn_extract_long_double (mp_ptr res_ptr, mp_size_t size, > && res_ptr[N - 1] == 0) > /* Pseudo zero. */ > *expt = 0; > + else > + /* The sign bit is explicit, but add it in case it is missing in > + the input. Otherwise, callers will not be able to produce the > + expected multi-precision integer layout by shifting the sign > + bit into the MSB. */ > + res_ptr[N - 1] |= (mp_limb_t) 1 << (LDBL_MANT_DIG - 1 > + - ((N - 1) * BITS_PER_MP_LIMB)); > > return N; > } > diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile > index c369faf00d..081cc72e93 100644 > --- a/sysdeps/x86/Makefile > +++ b/sysdeps/x86/Makefile > @@ -11,6 +11,10 @@ tests += tst-get-cpu-features tst-get-cpu-features-static \ > tests-static += tst-get-cpu-features-static > endif > > +ifeq ($(subdir),math) > +tests += tst-ldbl-nonnormal-printf > +endif # $(subdir) == math > + > ifeq ($(subdir),setjmp) > gen-as-const-headers += jmp_buf-ssp.sym > sysdep_routines += __longjmp_cancel > diff --git a/sysdeps/x86/tst-ldbl-nonnormal-printf.c b/sysdeps/x86/tst-ldbl-nonnormal-printf.c > new file mode 100644 > index 0000000000..72797fc33b > --- /dev/null > +++ b/sysdeps/x86/tst-ldbl-nonnormal-printf.c > @@ -0,0 +1,52 @@ > +/* Test printf with x86-specific non-normal long double value. > + Copyright (C) 2020 Free Software Foundation, Inc. > + > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + <https://www.gnu.org/licenses/>. */ > + > +#include <stdio.h> > +#include <string.h> > +#include <support/check.h> > + > +/* Fill the stack with non-zero values. This makes a crash in > + snprintf more likely. */ > +static void __attribute__ ((noinline, noclone)) > +fill_stack (void) > +{ > + char buffer[65536]; > + memset (buffer, 0xc0, sizeof (buffer)); > + asm ("" ::: "memory"); > +} > + > +static int > +do_test (void) > +{ > + fill_stack (); > + > + long double value; > + memcpy (&value, "\x00\x04\x00\x00\x00\x00\x00\x00\x00\x04", 10); > + > + char buf[30]; > + int ret = snprintf (buf, sizeof (buf), "%Lg", value); > + TEST_COMPARE (ret, strlen (buf)); > + if (strcmp (buf, "nan") != 0) > + /* If snprintf does not recognize the non-normal number as a NaN, > + it has added the missing sign bit. */ > + TEST_COMPARE_STRING (buf, "3.02201e-4624"); > + return 0; > +} > + > +#include <support/test-driver.c> > > -- > Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn, > Commercial register: Amtsgericht Muenchen, HRB 153243, > Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill > Please create a patch to move i386/ldbl2mpn.c to x86 and remove x86_64/ldbl2mpn.c. And rebase your patch. Thanks.
diff --git a/sysdeps/i386/ldbl2mpn.c b/sysdeps/i386/ldbl2mpn.c index ec8464eef7..867c66ef8d 100644 --- a/sysdeps/i386/ldbl2mpn.c +++ b/sysdeps/i386/ldbl2mpn.c @@ -115,6 +115,13 @@ __mpn_extract_long_double (mp_ptr res_ptr, mp_size_t size, && res_ptr[N - 1] == 0) /* Pseudo zero. */ *expt = 0; + else + /* The sign bit is explicit, but add it in case it is missing in + the input. Otherwise, callers will not be able to produce the + expected multi-precision integer layout by shifting the sign + bit into the MSB. */ + res_ptr[N - 1] |= (mp_limb_t) 1 << (LDBL_MANT_DIG - 1 + - ((N - 1) * BITS_PER_MP_LIMB)); return N; } diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile index c369faf00d..081cc72e93 100644 --- a/sysdeps/x86/Makefile +++ b/sysdeps/x86/Makefile @@ -11,6 +11,10 @@ tests += tst-get-cpu-features tst-get-cpu-features-static \ tests-static += tst-get-cpu-features-static endif +ifeq ($(subdir),math) +tests += tst-ldbl-nonnormal-printf +endif # $(subdir) == math + ifeq ($(subdir),setjmp) gen-as-const-headers += jmp_buf-ssp.sym sysdep_routines += __longjmp_cancel diff --git a/sysdeps/x86/tst-ldbl-nonnormal-printf.c b/sysdeps/x86/tst-ldbl-nonnormal-printf.c new file mode 100644 index 0000000000..72797fc33b --- /dev/null +++ b/sysdeps/x86/tst-ldbl-nonnormal-printf.c @@ -0,0 +1,52 @@ +/* Test printf with x86-specific non-normal long double value. + Copyright (C) 2020 Free Software Foundation, Inc. + + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + <https://www.gnu.org/licenses/>. */ + +#include <stdio.h> +#include <string.h> +#include <support/check.h> + +/* Fill the stack with non-zero values. This makes a crash in + snprintf more likely. */ +static void __attribute__ ((noinline, noclone)) +fill_stack (void) +{ + char buffer[65536]; + memset (buffer, 0xc0, sizeof (buffer)); + asm ("" ::: "memory"); +} + +static int +do_test (void) +{ + fill_stack (); + + long double value; + memcpy (&value, "\x00\x04\x00\x00\x00\x00\x00\x00\x00\x04", 10); + + char buf[30]; + int ret = snprintf (buf, sizeof (buf), "%Lg", value); + TEST_COMPARE (ret, strlen (buf)); + if (strcmp (buf, "nan") != 0) + /* If snprintf does not recognize the non-normal number as a NaN, + it has added the missing sign bit. */ + TEST_COMPARE_STRING (buf, "3.02201e-4624"); + return 0; +} + +#include <support/test-driver.c>