gvsafronov/futriix
1
0
Fork 0
Code Issues Pull Requests Actions 3 Packages Projects Releases Wiki Activity

Fix false positive leak reported by GCC ASAN (#9816)

Browse Source 
Leak found by the corrupt-dump-fuzzer when using GCC ASAN, which seems
to falsely report leaks on pointers kept only on the stack when calling exit.
Instead we now use _exit on panic / assert to skip these leak checks.

Additionally, check for sanitizer warnings in the corrupt-dump-fuzzer between iterations,
so that when something is found we know which test to relate it too (and it prints reproduction command list)
This commit is contained in:
Oran Agra 2021-11-21 18:47:10 +02:00 committed by GitHub
parent a68b71ac02
commit 183b90a625
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 37 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
src/debug.c
View File

@ -1975,8 +1975,12 @@ void bugReportEnd(int killViaSignal, int sig) {
if (server.daemonize && server.supervised == 0 && server.pidfile) unlink(server.pidfile);
if (!killViaSignal) {
if (server.use_exit_on_panic)
exit(1);
/* To avoid issues with valgrind, we may wanna exit rahter than generate a signal */
if (server.use_exit_on_panic) {
/* Using _exit to bypass false leak reports by gcc ASAN */
fflush(stdout);
_exit(1);
}
abort();
}

2
src/t_hash.c
View File

@ -477,6 +477,8 @@ void hashTypeConvertListpack(robj *o, int enc) {
value = hashTypeCurrentObjectNewSds(hi,OBJ_HASH_VALUE);
ret = dictAdd(dict, key, value);
if (ret != DICT_OK) {
sdsfree(key); sdsfree(value); /* Needed for gcc ASAN */
hashTypeReleaseIterator(hi); /* Needed for gcc ASAN */
serverLogHexDump(LL_WARNING,"listpack with dup elements dump",
o->ptr,lpBytes(o->ptr));
serverPanic("Listpack corruption detected");

5
tests/integration/corrupt-dump-fuzzer.tcl
View File

@ -166,8 +166,9 @@ foreach sanitize_dump {no yes} {
# check valgrind report for invalid reads after each RESTORE
# payload so that we have a report that is easier to reproduce
set valgrind_errors [find_valgrind_errors [srv 0 stderr] false]
if {$valgrind_errors != ""} {
puts "valgrind found an issue for payload: $printable_dump"
set asan_errors [sanitizer_errors_from_file [srv 0 stderr]]
if {$valgrind_errors != "" || $asan_errors != ""} {
puts "valgrind or asan found an issue for payload: $printable_dump"
set report_and_restart true
set print_commands true
}

29
tests/integration/corrupt-dump.tcl
View File

@ -261,10 +261,22 @@ test {corrupt payload: hash listpack with duplicate records} {
}
}
test {corrupt payload: hash ziplist uneven record count} {
# when we do perform full sanitization, we expect duplicate records to fail the restore
test {corrupt payload: hash listpack with duplicate records - convert} {
# when we do NOT perform full sanitization, but we convert to hash, we expect duplicate records panic
start_server [list overrides [list loglevel verbose use-exit-on-panic yes crash-memcheck-enabled no] ] {
r config set sanitize-dump-payload yes
r config set sanitize-dump-payload no
r config set hash-max-listpack-entries 1
r debug set-skip-checksum-validation 1
catch { r RESTORE _hash 0 "\x10\x17\x17\x00\x00\x00\x04\x00\x82a\x00\x03\x82b\x00\x03\x82a\x00\x03\x82d\x00\x03\xff\n\x00\xc0\xcf\xa6\x87\xe5\xa7\xc5\xbe" } err
assert_equal [count_log_message 0 "crashed by signal"] 0
assert_equal [count_log_message 0 "listpack with dup elements"] 1
}
}
test {corrupt payload: hash ziplist uneven record count} {
# when we do NOT perform full sanitization, but shallow sanitization can detect uneven count
start_server [list overrides [list loglevel verbose use-exit-on-panic yes crash-memcheck-enabled no] ] {
r config set sanitize-dump-payload no
r debug set-skip-checksum-validation 1
catch { r RESTORE _hash 0 "\r\x1b\x1b\x00\x00\x00\x16\x00\x00\x00\x04\x00\x00\x02a\x00\x04\x02b\x00\x04\x02a\x00\x04\x02d\x00\xff\t\x00\xa1\x98\x36x\xcc\x8e\x93\x2e" } err
assert_match "*Bad data format*" $err
@ -741,5 +753,16 @@ test {corrupt payload: fuzzer findings - LCS OOM} {
}
}
test {corrupt payload: fuzzer findings - gcc asan reports false leak on assert} {
start_server [list overrides [list loglevel verbose use-exit-on-panic yes crash-memcheck-enabled no] ] {
r debug set-skip-checksum-validation 1
r config set sanitize-dump-payload no
catch { r restore _list 0 "\x12\x01\x02\x13\x13\x00\x00\x00\x10\x00\x00\x00\x03\x00\x00\xF3\xFE\x02\x5F\x31\x04\xF1\xFF\x0A\x00\x19\x8D\x3D\x74\x85\x94\x29\xBD" }
catch { r LPOP _list } err
assert_equal [count_log_message 0 "crashed by signal"] 0
assert_equal [count_log_message 0 "ASSERTION FAILED"] 1
}
}
} ;# tags