Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

csv formatter: free of invalid pointer caused by incorrect use of g_slice_free1 #496

Closed
cebtenzzre opened this issue Apr 26, 2021 · 3 comments

Comments

@cebtenzzre
Copy link
Collaborator

Steps to reproduce

$ mkdir repr
$ echo a >repr/a.txt
$ echo bb >repr/b.txt
$ rmlint -T df -o csv:stdout -c csv:unique repr

Actual behavior

rmlint crashes with this output:

type,path,size,checksum
unique_file,"/tmp/repr/b.txt",3,
free(): invalid pointer
ERROR: Aborting due to a fatal error. (signal received: Aborted)
ERROR: Please file a bug report (See rmlint -h)

Or, with AddressSanitizer:

type,path,size,checksum
unique_file,"/tmp/repr/b.txt",3,
=================================================================
==1462093==ERROR: AddressSanitizer: attempting free on address which was not malloc()-ed: 0x6150000015c0 in thread T3 (pool-rmlint)
    #0 0x7f7108a7d0e9 in __interceptor_free /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:123
    #1 0x557f73dfeccf in rm_fmt_elem lib/formats/csv.c:102
    #2 0x557f73dcb657 in rm_fmt_write_impl lib/formats.c:348
    #3 0x557f73dcdf9d in rm_fmt_write lib/formats.c:469
    #4 0x557f73de883e in rm_shred_forward_to_output lib/shredder.c:1308
    #5 0x557f73de8f08 in rm_shred_group_postprocess lib/shredder.c:1495
    #6 0x557f73de94ae in rm_shred_result_factory lib/shredder.c:1553
    #7 0x7f7108687c86  (/usr/lib/libglib-2.0.so.0+0x84c86)
    #8 0x7f71086850c0  (/usr/lib/libglib-2.0.so.0+0x820c0)
    #9 0x7f710848a298 in start_thread (/usr/lib/libpthread.so.0+0x9298)
    #10 0x7f71083b3052 in __GI___clone (/usr/lib/libc.so.6+0xff052)

0x6150000015c0 is located 192 bytes inside of 240-byte region [0x615000001500,0x6150000015f0)
allocated by thread T0 here:
    #0 0x7f7108a7e1c6 in __interceptor_posix_memalign /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:226
    #1 0x7f7108675499  (/usr/lib/libglib-2.0.so.0+0x72499)

Thread T3 (pool-rmlint) created by T0 here:
    #0 0x7f7108a231c7 in __interceptor_pthread_create /build/gcc/src/gcc/libsanitizer/asan/asan_interceptors.cpp:214
    #1 0x7f71086a9de4  (/usr/lib/libglib-2.0.so.0+0xa6de4)

SUMMARY: AddressSanitizer: bad-free /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:123 in __interceptor_free
==1462093==ABORTING

Expected behavior

rmlint prints this output:

type,path,size,checksum
unique_file,"/tmp/repr/b.txt",3,
unique_file,"/tmp/repr/a.txt",2,

Analysis

Here, checksum_str and checksum_size are initialized.

rmlint/lib/formats/csv.c

Lines 73 to 74 in 427791c

char *checksum_str = NULL;
size_t checksum_size = 0;

Here, if file->digest is NULL indicating a unique file, a block of size one is allocated with g_slice_alloc0 and stored in checksum_str, without changing checksum_size. This behavior was introduced by commit 1b06eb3.

rmlint/lib/formats/csv.c

Lines 81 to 84 in 427791c

} else {
checksum_str = g_slice_alloc0(1);
*checksum_str = 0;
}

Here, g_slice_free1 is called. checksum_size is still zero from when it was initialized, even though checksum_str points to a block of size one.

rmlint/lib/formats/csv.c

Lines 101 to 103 in 427791c

if(checksum_str != NULL) {
g_slice_free1(checksum_size, checksum_str);
}

From the glib docs, regarding g_slice_free1:

...the block_size has to match the size specified upon allocation.

@SeeSpotRun
Copy link
Collaborator

Thanks for reporting and particularly for the diagnostics.

Was fixed as of f4c821c in the develop branch and will be merged into master branch at next release. Meanwhile you can compile from develop branch.

@cebtenzzre
Copy link
Collaborator Author

Whoops, I saw recent commits in master and figured it wasn't too far from develop. Perhaps I should make an AUR package for that branch so I don't run into old bugs as often.

@SeeSpotRun
Copy link
Collaborator

SeeSpotRun commented Apr 26, 2021

Perhaps I should make an AUR package for that branch

You're welcome to, but really we should be more pro-active at back-porting clear-cut bugs so that it's not necessary. I still have my L-plates on as a package maintainer and @sahib has gotten busy with other projects and had to step back a bit.

I'm gearing up for a new point release soon, I feel that #492 is a good foundation for that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants