bugGNU Astronomy Utilities - Bugs: bug #49347, Invalid writes and reads in Table

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #49347: Invalid writes and reads in Table

Submitter:  Mohammad Akhlaghi <makhlaghi>
Submitted:  Sat 15 Oct 2016 12:59:05 AM UTC
 
Category:  Table Severity:  4 - Important
Item Group:  Crash Status:  Fixed
Privacy:  Public Assigned to:  makhlaghi
Open/Closed:  Closed

Fri 21 Oct 2016 09:09:40 PM UTC, comment #2: 

I had a closer look and found the source of the problem, it is now fixed and has been pushed to the main repository. From the commit message:

The `data' element of Table's `outcolumn' structure (in `bin/table/main.h') has a `void *' type to allow various types. However, to allocate space for the output column we were using the `sizeof' operator on it! This would give a size of 1 and would cause a crash some systems.

In the previous commit (5c41564: New function to return the size of FITS datatype), a new function was added to the Gnuastro FITS functions to return the number of bytes in each datatype. With that function, the datatype's size is now also stored in the `outcolumn' structure and the allocation is done based on that.

Mohammad Akhlaghi <makhlaghi>
Project Administrator
Sun 16 Oct 2016 01:19:15 AM UTC, comment #1: 

As for

==9389== 48 bytes in 1 blocks are definitely lost in loss record 590 of 658
==9389==    at 0x4C29BBE: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==9389==    by 0x41A6F7: gal_txtarray_txt_to_array (txtarray.c:264)
==9389==    by 0x407C40: sanitycheck (ui.c:687)

it seems that the trouble is in

struct tableparams *p->up->txtarray

gal_txtarray_txt_to_array allocates memory for it, but freeandreport does not do cleanup for the structure field txtarray

Vladimir Markelov <vvm>
Project Member
Sat 15 Oct 2016 12:59:05 AM UTC, original submission:  

After inspecting the output of the GNU Hydra for Gnuastro 0.2, it was interesting to see issues errors on 32-bit systems.

The first thing that grabs most attention are the numerous warnings are for using `%lu' for size_t values. So in This commit, `%lu' was replaced with `%zu' (from C99 for dealing with `size_t' on both 32-bit and 64-bit systems).

After compilation and in `make check' the two `table/asciitobinary.sh' and `table/binarytoascii.sh' have failed. Alan Lefor also kindly tested it on his 32-bit Raspberry Pi and was able to reproduce this failure. The second depends on the first, so fixing the error in the first will probably fix the second failure too.

So I tried valgrind on `table/asciitobinary.sh' and was surprized to see multiple cases of invalid reads and writes (see below). These invalid reads and writes are probably causing this failure on 32-bit systems and are somehow not an issue on 64-bit systems. But we have to fix them any way.

I currently don't have time to look into them. But if anyone has the chance to look into this it would be great.

==9389== Memcheck, a memory error detector
==9389== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==9389== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
==9389== Command: ../bin/table/asttable /path/to/gnuastro/tests/table/asciitobinary.txt --output=asciitobinary.fits
==9389==
==9389== Invalid write of size 8
==9389==    at 0x409058: readinputcols (table.c:203)
==9389==    by 0x409CA6: table (table.c:431)
==9389==    by 0x405756: main (main.c:47)
==9389==  Address 0x89c7d70 is 0 bytes inside a block of size 2 alloc'd
==9389==    at 0x4C29BBE: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==9389==    by 0x408FC9: readinputcols (table.c:197)
==9389==    by 0x409CA6: table (table.c:431)
==9389==    by 0x405756: main (main.c:47)
==9389==
==9389== Conditional jump or move depends on uninitialised value(s)
==9389==    at 0x499833: ffpcnd (putcold.c:657)
==9389==    by 0x4949E3: ffpcn (putcol.c:829)
==9389==    by 0x40946C: saveouttofits (table.c:283)
==9389==    by 0x409CC0: table (table.c:434)
==9389==    by 0x405756: main (main.c:47)
==9389==
==9389== Invalid read of size 8
==9389==    at 0x49982C: ffpcnd (putcold.c:657)
==9389==    by 0x4949E3: ffpcn (putcol.c:829)
==9389==    by 0x40946C: saveouttofits (table.c:283)
==9389==    by 0x409CC0: table (table.c:434)
==9389==    by 0x405756: main (main.c:47)
==9389==  Address 0x89c7d78 is 6 bytes after a block of size 2 alloc'd
==9389==    at 0x4C29BBE: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==9389==    by 0x408FC9: readinputcols (table.c:197)
==9389==    by 0x409CA6: table (table.c:431)
==9389==    by 0x405756: main (main.c:47)
==9389==
==9389== Invalid read of size 8
==9389==    at 0x4C305F0: memmove (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==9389==    by 0x49882B: ffr8fr8 (putcold.c:1007)
==9389==    by 0x498C61: ffpcld.part.0 (putcold.c:441)
==9389==    by 0x499933: ffpcnd (putcold.c:711)
==9389==    by 0x4949E3: ffpcn (putcol.c:829)
==9389==    by 0x40946C: saveouttofits (table.c:283)
==9389==    by 0x409CC0: table (table.c:434)
==9389==    by 0x405756: main (main.c:47)
==9389==  Address 0x89c7d78 is 6 bytes after a block of size 2 alloc'd
==9389==    at 0x4C29BBE: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==9389==    by 0x408FC9: readinputcols (table.c:197)
==9389==    by 0x409CA6: table (table.c:431)
==9389==    by 0x405756: main (main.c:47)
==9389==
==9389== Syscall param write(buf) points to uninitialised byte(s)
==9389==    at 0x638E530: __write_nocancel (in /usr/lib/libc-2.24.so)
==9389==    by 0x6325866: _IO_file_write@@GLIBC_2.2.5 (in /usr/lib/libc-2.24.so)
==9389==    by 0x6324BB1: new_do_write (in /usr/lib/libc-2.24.so)
==9389==    by 0x6326938: _IO_do_write@@GLIBC_2.2.5 (in /usr/lib/libc-2.24.so)
==9389==    by 0x6325F05: _IO_file_xsputn@@GLIBC_2.2.5 (in /usr/lib/libc-2.24.so)
==9389==    by 0x631B0FA: fwrite (in /usr/lib/libc-2.24.so)
==9389==    by 0x426E74: file_write (drvrfile.c:669)
==9389==    by 0x4220C3: ffwrite (cfileio.c:7083)
==9389==    by 0x4E2BA4: ffbfwt (buffers.c:676)
==9389==    by 0x4E3EA9: ffflsh (buffers.c:608)
==9389==    by 0x42161F: ffclos (cfileio.c:6923)
==9389==    by 0x4094B3: saveouttofits (table.c:288)
==9389==  Address 0x8ef3310 is 1,664 bytes inside a block of size 4,096 alloc'd
==9389==    at 0x4C29BBE: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==9389==    by 0x6319AB1: _IO_file_doallocate (in /usr/lib/libc-2.24.so)
==9389==    by 0x6327AD5: _IO_doallocbuf (in /usr/lib/libc-2.24.so)
==9389==    by 0x6326E37: _IO_file_overflow@@GLIBC_2.2.5 (in /usr/lib/libc-2.24.so)
==9389==    by 0x6325F05: _IO_file_xsputn@@GLIBC_2.2.5 (in /usr/lib/libc-2.24.so)
==9389==    by 0x631B0FA: fwrite (in /usr/lib/libc-2.24.so)
==9389==    by 0x426E74: file_write (drvrfile.c:669)
==9389==    by 0x4220C3: ffwrite (cfileio.c:7083)
==9389==    by 0x4E2BA4: ffbfwt (buffers.c:676)
==9389==    by 0x4E3EA9: ffflsh (buffers.c:608)
==9389==    by 0x42161F: ffclos (cfileio.c:6923)
==9389==    by 0x4094B3: saveouttofits (table.c:288)
==9389==
==9389==
==9389== HEAP SUMMARY:
==9389==     in use at exit: 98,840 bytes in 3,272 blocks
==9389==   total heap usage: 38,220 allocs, 34,948 frees, 2,759,099 bytes allocated
==9389==
==9389== 48 bytes in 1 blocks are definitely lost in loss record 590 of 658
==9389==    at 0x4C29BBE: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==9389==    by 0x41A6F7: gal_txtarray_txt_to_array (txtarray.c:264)
==9389==    by 0x407C40: sanitycheck (ui.c:687)
==9389==    by 0x408789: setparams (ui.c:970)
==9389==    by 0x405747: main (main.c:44)
==9389==
==9389== LEAK SUMMARY:
==9389==    definitely lost: 48 bytes in 1 blocks
==9389==    indirectly lost: 0 bytes in 0 blocks
==9389==      possibly lost: 0 bytes in 0 blocks
==9389==    still reachable: 98,792 bytes in 3,271 blocks
==9389==         suppressed: 0 bytes in 0 blocks
==9389== Reachable blocks (those to which a pointer was found) are not shown.
==9389== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==9389==
==9389== For counts of detected and suppressed errors, rerun with: -v
==9389== Use --track-origins=yes to see where uninitialised values come from
==9389== ERROR SUMMARY: 17 errors from 6 contexts (suppressed: 0 from 0)
PASS table/asciitobinary.sh (exit status: 0)

Mohammad Akhlaghi <makhlaghi>
Project Administrator

 

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

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by vvm (Posted a comment)
  • -email is unavailable- added by makhlaghi (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.

     

     

     

    Follow 3 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-10-21 makhlaghi StatusConfirmed Fixed
        Assigned toNone makhlaghi
        Open/ClosedOpen Closed

    Back to the top


    Powered by Savane 3.9