bugGforth - Bugs: bug #46988, Fix use of uninitalized memory

 
 

bug #46988: Fix use of uninitalized memory

Submitter:  Jasper Lievisse Adriaanse <jasperla>
Submitted:  Wed 27 Jan 2016 08:10:28 AM UTC
   
 
Category:  None Severity:  3 - Normal
Item Group:  None Status:  Fixed
Privacy:  Public Assigned to:  anton
Open/Closed:  Closed
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Thu 28 Jan 2016 06:10:10 PM UTC, comment #10: 

ok, I should learn that the summary is the title of the bug:-)

Anton Ertl <anton>
Group administrator
Wed 27 Jan 2016 06:21:45 PM UTC, comment #9: 

Thanks, the patch from Anton resolves the issue here with MALLOC_OPTIONS=S (implies J).

Jasper Lievisse Adriaanse <jasperla>
Wed 27 Jan 2016 06:18:19 PM UTC, comment #8: 

Line 418 for gforth-0.7.3, line 480 for git head, after the closing bracket of this loop:

    for (; routines[i]!=0; i++) {
...
    }

Bernd Paysan <paysan>
Group administrator
Wed 27 Jan 2016 06:17:17 PM UTC, comment #7: 

The attached patch should do it.

(file #36181)

Anton Ertl <anton>
Group administrator
Wed 27 Jan 2016 06:06:41 PM UTC, comment #6: 

Could you please give me some context on the line? As http://git.savannah.gnu.org/cgit/gforth.git/tree/engine/engine.c#n418 seems the wrong place.

Jasper Lievisse Adriaanse <jasperla>
Wed 27 Jan 2016 05:55:32 PM UTC, comment #5: 

Looks like the culprit is the relocate loop while symbol[i]!=0.  That last zero doesn't get copied.  If I copy that last 0, valgrind reports nothing for gforth-ditc.

So I suggest to keep the malloc and correctly set the terminating 0 for the allocated arrays.  That way we can check for unintended use of the uninitialized part of the array.

Add the following before line 418 in engine/engine.c:

xts[i] = symbols[i] = 0;

and try if the bug persists (with the mallocs).

Bernd Paysan <paysan>
Group administrator
Wed 27 Jan 2016 05:38:28 PM UTC, comment #4: 

I can reproduce the crash 100% of the time:

GNU gdb 6.3
Copyright 2004 Free Software Foundation, Inc.
GDB is free software, covered by the GNU General Public License, and you are
welcome to change it and/or distribute copies of it under certain conditions.
Type "show copying" to see the conditions.
There is absolutely no warranty for GDB.  Type "show warranty" for details.
This GDB was configured as "amd64-unknown-openbsd5.9"...
Core was generated by `gforth-ditc'.
Program terminated with signal 11, Segmentation fault.
Reading symbols from /usr/lib/libpthread.so.20.1...done.
Loaded symbols for /usr/lib/libpthread.so.20.1
Loaded symbols for /usr/obj/ports/gforth-0.7.3/gforth-0.7.3/gforth-ditc
Reading symbols from /usr/local/lib/libavcall.so.1.0...done.
Loaded symbols for /usr/local/lib/libavcall.so.1.0
Reading symbols from /usr/local/lib/libffi.so.1.2...done.
Loaded symbols for /usr/local/lib/libffi.so.1.2
Reading symbols from /usr/local/lib/libltdl.so.5.0...done.
Loaded symbols for /usr/local/lib/libltdl.so.5.0
Reading symbols from /usr/lib/libm.so.9.0...done.
Loaded symbols for /usr/lib/libm.so.9.0
Symbols already loaded for /usr/lib/libpthread.so.20.1
Reading symbols from /usr/lib/libc.so.84.2...done.
Loaded symbols for /usr/lib/libc.so.84.2
Reading symbols from /usr/libexec/ld.so...done.
Loaded symbols for /usr/libexec/ld.so
#0  0x00001e33f031ec7b in check_prims (symbols1=0x1e35fbc01000) at ./main.c:954
954       for (i=0; symbols1[i]!=0; i++)
(gdb) bt
#0  0x00001e33f031ec7b in check_prims (symbols1=0x1e35fbc01000) at ./main.c:954
#1  0x00001e33f031ef25 in gforth_loader (imagefile=0x1e3619f01100, filename=0x1e33f0423970 "gforth.fi")
    at ./main.c:1860
#2  0x00001e33f031fdd1 in main (argc=15, argv=0x7f7ffffcf538, env=0x7f7ffffcf5b8) at ./main.c:2323
(gdb) bt full
#0  0x00001e33f031ec7b in check_prims (symbols1=0x1e35fbc01000) at ./main.c:954
        i = 1536
#1  0x00001e33f031ef25 in gforth_loader (imagefile=0x1e3619f01100, filename=0x1e33f0423970 "gforth.fi")
    at ./main.c:1860
        header = {base = 0x1e3619f01100 "", checksum = 16837413913267837873, image_size = 140187732341696,
  dict_size = 33208421971100, data_stack_size = 7165064483209180463, fp_stack_size = 7435269714738113633,
  return_stack_size = 3328212250303557478, locals_stack_size = 8245922036098215479,
  boot_entry = 0x74720069662e6874, throw_entry = 0x1e0069662e68, unused1 = 33217707870848,
  xt_base = 0x1e33f031f40a, data_stack_base = 0x7f7ffffcf330 "/usr/local/lib/gforth/0.7.3/gforth.fi",
  fp_stack_base = 0x7f7ffffcf370 "0\177\177", return_stack_base = 0x1e33f0423970 "gforth.fi",
  locals_stack_base = 0x1b8a3e9a00 <Address 0x1b8a3e9a00 out of bounds>}
        image = 0x7f7ffffcf803 "/usr/local/share/gforth/0.7.3:."
        imp = 0x1 <Address 0x1 out of bounds>
        magic = "\002\177\177\000"
        magic7 = 0 '\0'
        preamblesize = 0
        data_offset = 0
        check_sum = 140187732341776
        ausize = 0
        charsize = 0
        cellsize = 3
        sizebyte = 7
#2  0x00001e33f031fdd1 in main (argc=15, argv=0x7f7ffffcf538, env=0x7f7ffffcf5b8) at ./main.c:2323
        path = 0x7f7ffffcf7a1 ".:/usr/local/lib/gforth/site-forth:/usr/local/share/gforth/site-forth:/usr/local/lib/gforth/0.7.3:/usr/local/share/gforth/0.7.3:."
        imagename = 0x1e33f0423970 "gforth.fi"
        image_file = (FILE *) 0x1e3619f01100
        image = 0x7f7ffffcf5b8 "\177\177"
        retvalue = 0
(gdb)

Jasper Lievisse Adriaanse <jasperla>
Wed 27 Jan 2016 05:34:58 PM UTC, comment #3: 

Valgrind reports some usage of uninitialized memory with gforth-ditc, the other engines don't have such reports.

> valgrind --track-origins=yes ./gforth-ditc

==31224== Memcheck, a memory error detector
==31224== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==31224== Using Valgrind-3.10.1 and LibVEX; rerun with -h for copyright info
==31224== Command: ./gforth-ditc
==31224==
==31224== Conditional jump or move depends on uninitialised value(s)
==31224==    at 0x40AC1C: gforth_loader (in /home/bernd/gforths/gforth/gforth-ditc)
==31224==    by 0x40BCF9: gforth_start (in /home/bernd/gforths/gforth/gforth-ditc)
==31224==    by 0x40BDA5: gforth_main (in /home/bernd/gforths/gforth/gforth-ditc)
==31224==    by 0x404BE8: main (in /home/bernd/gforths/gforth/gforth-ditc)
==31224==  Uninitialised value was created by a heap allocation
==31224==    at 0x4C2A00F: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==31224==    by 0x404DA2: gforth_engine (in /home/bernd/gforths/gforth/gforth-ditc)
==31224==    by 0x40ABE2: gforth_loader (in /home/bernd/gforths/gforth/gforth-ditc)
==31224==    by 0x40BCF9: gforth_start (in /home/bernd/gforths/gforth/gforth-ditc)
==31224==    by 0x40BDA5: gforth_main (in /home/bernd/gforths/gforth/gforth-ditc)
==31224==    by 0x404BE8: main (in /home/bernd/gforths/gforth/gforth-ditc)
==31224==
==31224== Conditional jump or move depends on uninitialised value(s)
==31224==    at 0x40A544: gforth_relocate (in /home/bernd/gforths/gforth/gforth-ditc)
==31224==    by 0x40B1B2: gforth_loader (in /home/bernd/gforths/gforth/gforth-ditc)
==31224==    by 0x40BCF9: gforth_start (in /home/bernd/gforths/gforth/gforth-ditc)
==31224==    by 0x40BDA5: gforth_main (in /home/bernd/gforths/gforth/gforth-ditc)
==31224==    by 0x404BE8: main (in /home/bernd/gforths/gforth/gforth-ditc)
==31224==  Uninitialised value was created by a heap allocation
==31224==    at 0x4C2A00F: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==31224==    by 0x404DA2: gforth_engine (in /home/bernd/gforths/gforth/gforth-ditc)
==31224==    by 0x40ABE2: gforth_loader (in /home/bernd/gforths/gforth/gforth-ditc)
==31224==    by 0x40BCF9: gforth_start (in /home/bernd/gforths/gforth/gforth-ditc)
==31224==    by 0x40BDA5: gforth_main (in /home/bernd/gforths/gforth/gforth-ditc)
==31224==    by 0x404BE8: main (in /home/bernd/gforths/gforth/gforth-ditc)
==31224==

This report goes away with the calloc() fix.  Setting the arrays to junk doesn't cause a crash even with gforth-ditc; so I read it as "might crash" rather than "does crash".

Bernd Paysan <paysan>
Group administrator
Wed 27 Jan 2016 05:07:56 PM UTC, comment #2: 

Yes, the uninitialized memory should not be used.  But if the reporter observed a crash, it apparently is used, and that is a bug that is only superficially fixed by the suggested patch.

So I am reopening it and will look into it.

Anton Ertl <anton>
Group administrator
Wed 27 Jan 2016 04:43:21 PM UTC, comment #1: 

Hm, all used xts and symbols are correctly initialized.  There are these offsets which are used for helping relocation, this means that some parts of the array is not used, and thus also never initialized.

Using calloc will remove any false positive detection, though.

Bernd Paysan <paysan>
Group administrator
Wed 27 Jan 2016 08:10:28 AM UTC, original submission:  

OpenBSD's malloc(3) (see [1]) implementation allows for adding junk to the allocated area (see options 'j' and 'J'). Through this it was discovered that gforth_engine() was using uninitialized memory and would crash. The attached patch resolves the issue.

1: http://www.openbsd.org/cgi-bin/man.cgi?query=malloc.conf

Jasper Lievisse Adriaanse <jasperla>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #36181:  patch added by anton (391B - application/octet-stream - fix)
file #36171:  patch-engine_engine_c added by jasperla (853B - application/octet-stream)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by anton (Posted a comment)
  • -email is unavailable- added by paysan (Posted a comment)
  • -email is unavailable- added by jasperla (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 14 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-01-28 anton SummaryFixed in git head Fix use of uninitalized memory
    2016-01-28 anton StatusIn Progress Fixed
        Open/ClosedOpen Closed
        SummaryFix use of uninitalized memory Fixed in git head
    2016-01-27 paysan StatusConfirmed In Progress
    2016-01-27 anton Attached File- Added patch, #36181
        StatusIn Progress Confirmed
    2016-01-27 anton StatusFixed In Progress
        Assigned topaysan anton
        Open/ClosedClosed Open
    2016-01-27 paysan StatusNone Fixed
        Assigned toNone paysan
        Open/ClosedOpen Closed
    2016-01-27 jasperla Attached File- Added patch-engine_engine_c, #36171

    Back to the top

    Powered by Savane 3.13-758e.
    Corresponding source code