failed autopkgtests for evolver vs glibc 2.39 on amd64

Bug #2052929 reported by Simon Chopin
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
gcc
In Progress
Medium
evolver (Debian)
New
Unknown
evolver (Ubuntu)
New
Undecided
Unassigned
gcc-13 (Ubuntu)
Fix Released
Undecided
Unassigned
gcc-14 (Ubuntu)
Fix Released
Undecided
Unassigned
glibc (Ubuntu)
Invalid
High
Simon Chopin

Bug Description

The autopkgests for evolver fail when run against glibc 2.39 on amd64 with a segfault:

3537s autopkgtest [09:43:29]: test command6: [-----------------------
3537s Surface Evolver Version 2.70a (Debian 2.70+ds-8build1), August 27, 2013, 64-bit.
3537s Compiled for float128, 33 digits precision.
3537s Built with Geomview support.
3537s
3537s Enter command:
3537s Enter command: // Typical evolution to sphere
3537s Enter command: gogo := { g 5; r; g 5; hessian; r; g 5; hessian; }
3537s Enter command:
3537s Enter command: // Evolution to very high accuracy, using higher-order Lagrange elements.
3537s Enter command: // To be run on original datafile.
3537s Enter command: gogo2 := { g 5; r; g 5; hessian; r; g 5; hessian;
3537s more> lagrange 2; g 5; hessian;
3537s more> lagrange 4; g 5; hessian;
3537s more> lagrange 6; g 5; hessian;
3537s more> ideal_rad := (3*body[1].volume/4/pi)^(1/3);
3537s more> printf "Area error: %g\n",total_area - 4*pi*ideal_rad^2;
3537s more> printf "Vertex radius spread: %g\n",
3537s more> max(vertex,sqrt((x-.5)^2+(y-.5)^2+(z-.5)^2))
3537s more> - min(vertex,sqrt((x-.5)^2+(y-.5)^2+(z-.5)^2));
3537s more> }
3537s Enter command: g 5; v; r ; g 10; v;
3537s bash: line 1: 1012 Done echo "g 5; v; r ; g 10; v;"
3537s 1013 Segmentation fault (core dumped) | evolver-nox-q cube

CVE References

Simon Chopin (schopin)
tags: added: foundations-todo
Changed in glibc (Ubuntu):
assignee: nobody → Simon Chopin (schopin)
importance: Undecided → High
Revision history for this message
Matthias Klose (doko) wrote :

also segfaults without lto enabled, and even when built without optimization (-O0).

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7f8e092 in ?? () from /lib/x86_64-linux-gnu/libquadmath.so.0
(gdb) bt
#0 0x00007ffff7f8e092 in ?? () from /lib/x86_64-linux-gnu/libquadmath.so.0
#1 0x00007ffff7f9104a in ?? () from /lib/x86_64-linux-gnu/libquadmath.so.0
#2 0x00007ffff7c7f77a in __printf_function_invoke (buf=buf@entry=0x7fffffffbf70,
    callback=0x7ffff7f91000, args_value=<optimized out>, ndata_args=<optimized out>,
    info=info@entry=0x7fffffffb1e8) at ./printf_buffer_as_file.h:52
#3 0x00007ffff7c827de in printf_positional (buf=buf@entry=0x7fffffffbf70,
    format=format@entry=0x555555920bf0 "%3d. energy: %#*.*Qg scale: %#Qg\n",
    readonly_format=readonly_format@entry=0, ap=ap@entry=0x7fffffffbfc0,
    ap_savep=ap_savep@entry=0x7fffffffbb18, nspecs_done=1, nspecs_done@entry=0,
    lead_str_end=<optimized out>, work_buffer=<optimized out>, save_errno=<optimized out>,
    grouping=<optimized out>, thousands_sep=<optimized out>, mode_flags=<optimized out>)
    at ./stdio-common/vfprintf-internal.c:1345
#4 0x00007ffff7c84235 in __printf_buffer (buf=buf@entry=0x7fffffffbf70,
    format=0x555555920bf0 "%3d. energy: %#*.*Qg scale: %#Qg\n", ap=0x7fffffffbfc0,
    mode_flags=0) at ./stdio-common/vfprintf-internal.c:1041
#5 0x00007ffff7ca39d7 in __vsprintf_internal (string=<optimized out>,
    maxlen=maxlen@entry=18446744073709551615, format=<optimized out>,
    args=args@entry=0x7fffffffbfc0, mode_flags=mode_flags@entry=0) at ./libio/iovsprintf.c:62
#6 0x00007ffff7c814b7 in __sprintf (s=<optimized out>, format=<optimized out>)
    at ./stdio-common/sprintf.c:30
#7 0x00005555556fc006 in iterate () at ../../../src/iterate.c:277
#8 0x0000555555718fc0 in letter_command (c=103) at ../../../src/command.c:465
#9 0x00005555556645d6 in eval (ex_original=0x7fffffffe050, params=0x0, self_id=0,
    parent_frame=0x0) at ../../../src/evaltree.c:396
#10 0x0000555555593142 in command (text=0x7fffffffe140 "g 5; v; r ; g 10; v;", mode=1)
    at ../../../src/query.c:247
#11 0x00005555557179aa in old_menu (text=0x7fffffffe140 "g 5; v; r ; g 10; v;")
    at ../../../src/command.c:125
#12 0x00005555556b4494 in exec_commands (basefd=0x0,
    promptstring=0x555555919ed5 "Enter command: ") at ../../../src/tmain.c:839
#13 0x00005555556b4389 in main (argc=1, argv=0x7fffffffe400) at ../../../src/tmain.c:678

Changed in evolver (Debian):
status: Unknown → New
Revision history for this message
Simon Chopin (schopin) wrote :

This bug is actually in gcc-14, more specifically libquadmath, as they do a misaligned read from args to a float128, which produces the segfault.

It was hidden so far because args was allocated using alloca() which I guess must be naturally aligned, but in 2.39 they removed that in favor of appending those arguments to an existing buffer.

I'm testing the attached patch in a PPA before sending it upstream for review.

tags: added: patch
Revision history for this message
Simon Chopin (schopin) wrote :

Using the PPA's libquadmath makes the tests pass, so I sent the patch upstream:

https://gcc.gnu.org/pipermail/gcc-patches/2024-March/647635.html

Simon Chopin (schopin)
Changed in glibc (Ubuntu):
status: Triaged → Invalid
Revision history for this message
In , Doko-v (doko-v) wrote :

reported at
https://gcc.gnu.org/pipermail/gcc-patches/2024-March/647635.html

On x86, this compiles into movdqa which segfaults on unaligned access.

This kind of failure has been seen when running against glibc 2.39,
which incidentally changed the printf implementation to move away from
alloca() for this data to instead append it at the end of an existing
"scratch buffer", with arbitrary alignment, whereas alloca() was
probably more likely to be naturally aligned.

Tested by adding the patch to the Ubuntu gcc-14 package in
https://launchpad.net/~schopin/+archive/ubuntu/libquadmath

Signed-off-by: Simon Chopin <email address hidden>
---
 libquadmath/printf/printf_fp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libquadmath/printf/printf_fp.c b/libquadmath/printf/printf_fp.c
index 8effcee88fa..d86aa650d38 100644
--- a/libquadmath/printf/printf_fp.c
+++ b/libquadmath/printf/printf_fp.c
@@ -363,7 +363,7 @@ __quadmath_printf_fp (struct __quadmath_printf_file *fp,

   /* Fetch the argument value. */
     {
- fpnum = **(const __float128 **) args[0];
+ memcpy(&fpnum, *(void* const *) args[0], sizeof(fpnum));

       /* Check for special values: not a number or infinity. */
       if (isnanq (fpnum))

Matthias Klose (doko)
Changed in gcc-14 (Ubuntu):
status: New → In Progress
Changed in gcc:
importance: Unknown → Medium
status: Unknown → New
Revision history for this message
In , Rguenth (rguenth) wrote :

The question is whether the caller misbehaves according to the ABI here? There's likely a known alignment present we could re-instantiate with a
__builtin_assume_aligned?

Revision history for this message
In , Jakub-gcc (jakub-gcc) wrote :
Revision history for this message
In , Andreas Schwab (schwab-linux-m68k) wrote :

Is the stack properly aligned at this point?

Revision history for this message
In , Fw-6 (fw-6) wrote :

This looks like a glibc bug to me.

Revision history for this message
In , Andreas Schwab (schwab-linux-m68k) wrote :

Without a test case it is hard to tell.

Revision history for this message
In , Andreas Schwab (schwab-linux-m68k) wrote :

Looks like the issue is that args_pa_user is not kept aligned. On the other hand, flt128_va already uses memcpy, so it does not expect the memory to be aligned.

Revision history for this message
In , Jsm28 (jsm28) wrote :

Note also that in glibc, _Float128 support in printf code can only be used in limited circumstances: either on powerpc64le, as one of the multiple supported long double formats there, or through the sharing of the printf code with the implementation of strfromf128.

In particular, there are no glibc printf formats corresponding directly to _FloatN / _FloatNx types. There was support in principle at the WG14 meeting in Strasbourg in January for having some form of printf/scanf support for such types in C2y, but major work is still needed on the wording that was proposed in N3184.

Revision history for this message
In , Jakub-gcc (jakub-gcc) wrote :

I guess we should go with the above patch after fixing formatting, but it isn't enough,
printf_fphex.c has similar code.
Even in glibc which doesn't support printing _Float128 nor any other type which would require similar alignment, the hooks only register a function to fill in some mem and allows specification of size, but can't specify alignment.

Revision history for this message
In , Jvdelisle (jvdelisle) wrote :

Adding myself here as I need hex format for gfortran EX format specifiers.

Revision history for this message
In , Jakub-gcc (jakub-gcc) wrote :

Created attachment 57853
gcc14-pr114533.patch

Untested fix. Unfortunately, we don't have any testsuite for libquadmath, hope it will be tested during libgfortran testing.

Revision history for this message
In , Doko-v (doko-v) wrote :

while not a test case, that was seen when running autopkg tests of the evolver package against glibc 2.39 packages.

https://bugs.launchpad.net/ubuntu/+source/glibc/+bug/2052929

the failing evolver test is:

echo "g 5; v; r ; g 10; v;" | evolver-nox-q cube

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package gcc-14 - 14-20240330-1ubuntu2

---------------
gcc-14 (14-20240330-1ubuntu2) noble; urgency=medium

  * No-change rebuild for CVE-2024-3094

 -- Steve Langasek <email address hidden> Sun, 31 Mar 2024 01:02:56 +0000

Changed in gcc-14 (Ubuntu):
status: In Progress → Fix Released
Revision history for this message
In , Cvs-commit (cvs-commit) wrote :

The master branch has been updated by Jakub Jelinek <email address hidden>:

https://gcc.gnu.org/g:8455d6f6cd43b7b143ab9ee19437452fceba9cc9

commit r14-9769-g8455d6f6cd43b7b143ab9ee19437452fceba9cc9
Author: Jakub Jelinek <email address hidden>
Date: Wed Apr 3 10:02:35 2024 +0200

    libquadmath: Don't assume the storage for __float128 arguments is aligned [PR114533]

    With the register_printf_type/register_printf_modifier/register_printf_specifier
    APIs the C library is just told the size of the argument and is provided with
    a callback to fetch the argument from va_list using va_arg into C library provided
    memory. The C library isn't told what alignment requirement it has, but we were
    using direct load of a __float128 value from that memory which assumes
    __alignof (__float128) alignment.

    The following patch fixes that by using memcpy instead.

    I haven't been able to reproduce an actual crash, tried
     #include <quadmath.h>
     #include <stdlib.h>
     #include <stdio.h>

    int main ()
    {
      __float128 r;
      int prec = 20;
      int width = 46;
      char buf[128];

      r = 2.0q;
      r = sqrtq (r);
      int n = quadmath_snprintf (buf, sizeof buf, "%+-#*.20Qe", width, r);
      if ((size_t) n < sizeof buf)
        printf ("%s\n", buf);
        /* Prints: +1.41421356237309504880e+00 */
      quadmath_snprintf (buf, sizeof buf, "%Qa", r);
      if ((size_t) n < sizeof buf)
        printf ("%s\n", buf);
        /* Prints: 0x1.6a09e667f3bcc908b2fb1366ea96p+0 */
      n = quadmath_snprintf (NULL, 0, "%+-#46.*Qe", prec, r);
      if (n > -1)
        {
          char *str = malloc (n + 1);
          if (str)
            {
              quadmath_snprintf (str, n + 1, "%+-#46.*Qe", prec, r);
              printf ("%s\n", str);
              /* Prints: +1.41421356237309504880e+00 */
            }
          free (str);
        }
      printf ("%+-#*.20Qe\n", width, r);
      printf ("%Qa\n", r);
      printf ("%+-#46.*Qe\n", prec, r);
      printf ("%d %Qe %d %Qe %d %Qe\n", 1, r, 2, r, 3, r);
      return 0;
    }
    In any case, I think memcpy for loading from it is right.

    2024-04-03 Simon Chopin <email address hidden>
                Jakub Jelinek <email address hidden>

            PR libquadmath/114533
            * printf/printf_fp.c (__quadmath_printf_fp): Use memcpy to copy
            __float128 out of args.
            * printf/printf_fphex.c (__quadmath_printf_fphex): Likewise.

    Signed-off-by: Simon Chopin <email address hidden>

Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package gcc-13 - 13.2.0-23ubuntu3

---------------
gcc-13 (13.2.0-23ubuntu3) noble; urgency=medium

  * Use gcc:SoftVersion for -for-host dependencies (Helmut Grohne).
    Addresses: #1067904.
  * Apply proposed patch for PR libquadmath/114533 (Simon Chopin).
    Addresses: #1064426. LP: #2052929.

 -- Matthias Klose <email address hidden> Sat, 30 Mar 2024 14:29:46 +0100

Changed in gcc-13 (Ubuntu):
status: New → Fix Released
Changed in gcc:
status: New → In Progress
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.