Hello Dan Burkert, Mike Percy,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/4638
to review the following change.
Change subject: log: address a heap overflow race during log roll
......................................................................
log: address a heap overflow race during log roll
This addresses the following heap overflow which I managed to produce in
TSAN builds of raft-consensus-itest:
==14633==ERROR: AddressSanitizer: heap-buffer-overflow on address
0x602000083b58 at pc 0x7f95761db463 bp 0x7f9532d1ad00 sp 0x7f9532d1acf8
READ of size 8 at 0x602000083b58 thread T115 (maintenance_sch)
#0 0x7f95761db462 in
scoped_refptr<kudu::log::ReadableLogSegment>::operator->() const
/home/todd/git/kudu/build/asan/../../src/kudu/gutil/ref_counted.h:273:5
#1 0x7f957577a012 in kudu::log::Log::GetMaxIndexesToSegmentSizeMap(long,
std::map<long, long, std::less<long>, std::allocator<std::pair<long const,
long> > >*) const
/home/todd/git/kudu/build/asan/../../src/kudu/consensus/log.cc:787:10
#2 0x7f9575c5995a in
kudu::tablet::TabletPeer::GetMaxIndexesToSegmentSizeMap(std::map<long, long,
std::less<long>, std::allocator<std::pair<long const, long> > >*) const
/home/todd/git/kudu/build/asan/../../src/kudu/tablet/tablet_peer.cc:480:9
The issue is the following race:
- we grab a snapshot of the current log segments
- the log rolls, such that the last segment is now closed and has a
footer
- FLAGS_log_min_segments_to_retain is higher than segments.size()
In this case, we triggered integer underflow in the following for loop:
for (int i = gc_prefix; i < segments.size() -
FLAGS_log_min_segments_to_retain; i++) {
since segments.size() is an unsigned integer. This made the loop never
terminate. In most cases, it would terminate due to the last segment
having no footer, but in the race described above, that would not be the
case. We'd then access invalid memory and probably crash.
The test modifications in this patch would trigger the issue reliably
when run with --stress_cpu_threads=5.
Change-Id: Ic6dd40cf9ac628f56f0c467bcd9eeb2191124836
---
M src/kudu/consensus/log.cc
M src/kudu/consensus/mt-log-test.cc
2 files changed, 28 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/38/4638/1
--
To view, visit http://gerrit.cloudera.org:8080/4638
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic6dd40cf9ac628f56f0c467bcd9eeb2191124836
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <[email protected]>
Gerrit-Reviewer: Dan Burkert <[email protected]>
Gerrit-Reviewer: Mike Percy <[email protected]>
