search

thegenemyers / DAZZ_DB

The Dazzler Data Base
Other
35 stars 33 forks source link

Invalid read, DBstats crashes with glibc 2.32 #41

Open rbalint opened 4 years ago

rbalint commented 4 years ago

While testing glibc 2.32 we discovered dazzdb crasing in Ubuntu CI: https://autopkgtest.ubuntu.com/packages/d/dazzdb/groovy/amd64

The root cause seems to be an invalid read, which can be detected even with glibc 2.31, but for some reason it does not end up in a crash. I've replaced the crashing command with valgrind ...:

==2386== Memcheck, a memory error detector
==2386== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==2386== Using Valgrind-3.16.1 and LibVEX; rerun with -h for copyright info
==2386== Command: DBstats -mdust G
==2386== 
==2386== Invalid read of size 8
==2386==    at 0x10B146: main (DBstats.c:269)
==2386==  Address 0x4ba9e20 is 0 bytes inside a block of size 64 free'd
==2386==    at 0x483DA3F: free (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==2386==    by 0x10B140: main (DBstats.c:350)
==2386==  Block was alloc'd at
==2386==    at 0x483C7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==2386==    by 0x1108C5: UnknownInlinedFun (DB.c:67)
==2386==    by 0x1108C5: Open_Track (DB.c:1952)
==2386==    by 0x10B19E: main (DBstats.c:108)
==2386== 
==2386== 
==2386== HEAP SUMMARY:
==2386==     in use at exit: 196 bytes in 3 blocks
==2386==   total heap usage: 30 allocs, 27 frees, 153,650 bytes allocated
==2386== 
==2386== LEAK SUMMARY:
==2386==    definitely lost: 80 bytes in 1 blocks
==2386==    indirectly lost: 0 bytes in 0 blocks
==2386==      possibly lost: 0 bytes in 0 blocks
==2386==    still reachable: 116 bytes in 2 blocks
==2386==         suppressed: 0 bytes in 0 blocks
==2386== Rerun with --leak-check=full to see details of leaked memory
==2386== 
==2386== For lists of detected and suppressed errors, rerun with: -s
==2386== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
jscott0 commented 3 years ago

I bisected the issue to commit 51e14c12b671a624a6b0a08aaadab053220a4c59.

Although the changes to DBstats.c are small, the change cannot be reverted just for that file to narrow the issue down, as the commit also removed the Load_Track function the code previously depended on. Unfortunately, this seems to be a symptom of deeper issues. When building that snapshot with ASan, I get a buffer overflow in a totally different place:

    #0 0x7ffff76a4858 in __interceptor_realloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:164
    #1 0x5555555aefda in Realloc /root/DAZZ_DB/DB.c:79
    #2 0x5555555b7c4d in Trim_DB /root/DAZZ_DB/DB.c:800
    #3 0x5555555ab6f6 in main /root/DAZZ_DB/DBstats.c:118
    #4 0x7ffff69a5e49 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x27e49)
aurel32 commented 3 years ago

The DBstats code iterate through a linked list of tracks. Following the rework introduced in commit 51e14c12b671a624a6b0a08aaadab053220a4c59, the tracks are closed after being processed. After that the record stored in track is invalid because it has been freed, so accessing track->next is undefined. The problem is only visible with glibc 2.32 and later, due to the Safe-Linking mechanism added to fastbins and tcache (https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=a1a486d70ebcc47a686ff5846875eacad0940e41) which in that case prevent access to memory that has been freed.

I would recommend drop the Close_Track(db,track) entry because: