View Issue Details

IDProjectCategoryView StatusLast Update
0000921luatexmplib bugpublic2015-10-22 11:03
Reporterphg Assigned Toluigi scarso  
PrioritynormalSeverityminorReproducibilityhave not tried
Status closedResolutionfixed 
Summary0000921: segfault due to reallocation of factorial cache during mplib initialization
DescriptionIn mpmathdecimal.w, the ``factorials`` array
is reallocated with each call to ``mp_initialize_decimal_math()``.
This causes invalid reads since the array size is out of date.

Patch against rev 5122 attached.
TagsNo tags attached.

Activities

phg

2015-01-04 23:47

reporter  

0001-mpmathdecimal.w-guard-allocation-of-factorials-cache.patch (3,198 bytes)   
From 4981864e7521b68bbebdcbc4ca25aeea6ac959dd Mon Sep 17 00:00:00 2001
From: Philipp Gesang <phg@phi-gamma.net>
Date: Sun, 4 Jan 2015 23:24:06 +0100
Subject: [PATCH] mpmathdecimal.w: guard allocation of factorials cache
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

During the initialization of the decimal number system, MP sets up a
cache for factorial values already calculated. The function is called
whenever a new metapost instance is constructed, causing an existing
cache pointer to be overwritten. This empty cache store is then indexed
at invalid positions since the size info only applies to the previously
allocated array. Consequently, Luatex segfaults almost instantly.

This patch guards the initialization of the factorial cache.

Code that triggers the bug:

    \ifdefined \luatexsuppresslongerror
      \input luamplib.sty
      \mplibnumbersystem {decimal}
      \directlua {metapost = luamplib}
    \fi

    \directlua {
      metapost.load "metafun"
      metapost.load "metafun"
    }

    \mplibcode
      beginfig(0);
        draw fullcircle scaled 42pt withcolor blue;
      endfig;
    \endmplibcode

    \bye

For this to work with the Luatex-Plain format, the following patch must
be applied to force decimal arithmetic:

    --- luatex-mplib.lua.orig	2015-01-04 23:27:23.426036828 +0100
    +++ luatex-mplib.lua	2015-01-04 23:27:25.412716332 +0100
    @@ -147,6 +147,7 @@
             function metapost.load(name)
                 local mpx = mplib.new {
                     ini_version = true,
    +                math_mode = "decimal",
                     find_file = metapost.finder,
                 }
                 local result

This bug was discovered by user “franckpastor” on Github:

  https://github.com/lualatex/luamplib/issues/49

Signed-off-by: Philipp Gesang <phg@phi-gamma.net>
---
 source/texk/web2c/mplibdir/mpmathdecimal.w | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/source/texk/web2c/mplibdir/mpmathdecimal.w b/source/texk/web2c/mplibdir/mpmathdecimal.w
index b530d09..fad3f17 100644
--- a/source/texk/web2c/mplibdir/mpmathdecimal.w
+++ b/source/texk/web2c/mplibdir/mpmathdecimal.w
@@ -384,9 +384,12 @@ void * mp_initialize_decimal_math (MP mp) {
   decNumberFromString(&PI_decNumber, PI_STRING, &set);
   decNumberFromString(&epsilon_decNumber, epsilon, &set);
   decNumberFromString(&EL_GORDO_decNumber, EL_GORDO, &set);
-  factorials = (decNumber **)mp_xmalloc(mp,PRECALC_FACTORIALS_CACHESIZE,sizeof(decNumber *));
-  factorials[0] = (decNumber *)mp_xmalloc(mp,1,sizeof(decNumber));
-  decNumberCopy(factorials[0], &one);
+  if (!factorials)
+  {
+    factorials = (decNumber **)mp_xmalloc(mp,PRECALC_FACTORIALS_CACHESIZE,sizeof(decNumber *));
+    factorials[0] = (decNumber *)mp_xmalloc(mp,1,sizeof(decNumber));
+    decNumberCopy(factorials[0], &one);
+  }
 
   /* alloc */
   math->allocate = mp_new_number;
@@ -1745,4 +1748,4 @@ void mp_init_randoms (MP mp, int seed) {
 @ @c
 void mp_decimal_number_modulo (mp_number *a, mp_number b) {
    decNumberRemainder(a->data.num, a->data.num, b.data.num, &set);
-}
\ No newline at end of file
+}
-- 
2.2.1

phg

2015-01-04 23:55

reporter   ~0001304

Valgrind report running an unpatched Luatex:

    ==16375== Process terminating with default action of signal 11 (SIGSEGV): dumping core
    ==16375== Access not within mapped region at address 0x8
    ==16375== at 0xBF8DF9: decNumberCopy (decNumber.c:3362)
    ==16375== by 0x77E302: sinecosine (mpmathdecimal.w:1665)
    ==16375== by 0x77E5DC: mp_decimal_sin_cos (mpmathdecimal.w:1713)
    ==16375== by 0x6F55EB: mp_solve_choices (mp.w:8498)
    ==16375== by 0x6F0D21: mp_make_choices (mp.w:7742)
    ==16375== by 0x76A88A: mp_scan_path (mp.w:23996)
    ==16375== by 0x7697BE: mp_scan_expression (mp.w:23694)
    ==16375== by 0x754F9B: mp_do_assignment (mp.w:28834)
    ==16375== by 0x753EE3: mp_do_statement (mp.w:28513)
    ==16375== by 0x75836F: mp_execute (mp.w:29893)
    ==16375== by 0x4659C9: mplib_execute (lmplib.c:569)
    ==16375== by 0x6C5C82: luaD_precall (ldo.c:319)


For everything else (e.g. code to reproduce) see the
attached patch. Tested with revision 5122.

luigi scarso

2015-01-05 07:36

developer   ~0001305

I'm looking into it.

luigi scarso

2015-05-20 13:28

developer   ~0001379

Fixed in the upcoming TexLive 2015.

Issue History

Date Modified Username Field Change
2015-01-04 23:47 phg New Issue
2015-01-04 23:47 phg File Added: 0001-mpmathdecimal.w-guard-allocation-of-factorials-cache.patch
2015-01-04 23:55 phg Note Added: 0001304
2015-01-05 07:36 luigi scarso Note Added: 0001305
2015-03-21 15:09 Hans Hagen Assigned To => luigi scarso
2015-03-21 15:09 Hans Hagen Status new => assigned
2015-05-20 13:28 luigi scarso Note Added: 0001379
2015-10-22 11:03 Hans Hagen Status assigned => closed
2015-10-22 11:03 Hans Hagen Resolution open => fixed