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

Jump to bottom

Double buffer size for 'name1' and 'name2' #11987

Merged
merged 1 commit into from Oct 28, 2015
Merged

Double buffer size for 'name1' and 'name2' #11987

merged 1 commit into from Oct 28, 2015

Conversation

davidlt
Copy link
Contributor

@davidlt davidlt commented Oct 20, 2015

ASan detected stack-buffer-overflow (READ of 23 bytes) in
std::basic_string constructor which is called by
SimHitsValidationHcal::getHistogramTypes().

A quick test showed that we always overflow name2, it's usual size is
21-23 chars.

The patch doubles buffers size for name1 and name2 to 40 bytes and
uses snprintf to make sure it never overflows.

len(name1) = 6
len(name2) = 23

len(name1) = 6
len(name2) = 23

len(name1) = 6
len(name2) = 21

len(name1) = 6
len(name2) = 21

len(name1) = 6
len(name2) = 21

len(name1) = 6
len(name2) = 21

len(name1) = 6
len(name2) = 21

len(name1) = 6
len(name2) = 21

=================================================================
==14834==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fff0da5ef04 at pc 0x46b04a bp 0x7fff0da5ea90 sp 0x7fff0da5e9d8
READ of size 23 at 0x7fff0da5ef04 thread T0
   #0 0x46b049 in memcpy (/mnt/build/davidlt/asan2/a/slc6_amd64_gcc493/cms/cmssw/CMSSW_7_6_ASAN_X_2015-10-19-1100/bin/slc6_amd64_gcc493/cmsRun+0x46b049)
   #1 0x7f47b5a8fdc9 in std::char_traits<char>::copy(char*, char const*, unsigned long) (/mnt/build/davidlt/asan2/a/slc6_amd64_gcc493/external/gcc/4.9.3/lib64/libstdc++.so.6+0x63dc9)
   #2 0x7f47b5a9065f in _Y_ZNSs7_M_copyEPcPKcm (/mnt/build/davidlt/asan2/a/slc6_amd64_gcc493/external/gcc/4.9.3/lib64/libstdc++.so.6+0x6465f)
   #3 0x7f47b5b031d5 in std::string::_S_copy_chars(char*, char const*, char const*) (/mnt/build/davidlt/asan2/a/slc6_amd64_gcc493/external/gcc/4.9.3/lib64/libstdc++.so.6+0xd71d5)
   #4 0x7f47b7e92d66 in char* std::string::_S_construct<char const*>(char const*, char const*, std::allocator<char> const&, std::forward_iterator_tag) (/mnt/build/davidlt/asan2/CMSSW_7_6_ASAN_X_2015-10-19-1100/lib/slc6_amd64_gcc493/libFWCoreUtilities.so+0xcbd66)
   #5 0x7f47b7e9282b in char* std::string::_S_construct_aux<char const*>(char const*, char const*, std::allocator<char> const&, std::__false_type) /mnt/build/davidlt/asan2/a/slc6_amd64_gcc493/external/gcc/4.9.3/include/c++/4.9.3/bits/basic_string.h:1743
   #6 0x7f47b7e92711 in char* std::string::_S_construct<char const*>(char const*, char const*, std::allocator<char> const&) /mnt/build/davidlt/asan2/a/slc6_amd64_gcc493/external/gcc/4.9.3/include/c++/4.9.3/bits/basic_string.h:1764
   #7 0x7f47b5b03891 in std::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string(char const*, std::allocator<char> const&) (/mnt/build/davidlt/asan2/a/slc6_amd64_gcc493/external/gcc/4.9.3/lib64/libstdc++.so.6+0xd7891)
   #8 0x7f478afbf134 in SimHitsValidationHcal::getHistogramTypes() /mnt/build/davidlt/asan2/CMSSW_7_6_ASAN_X_2015-10-19-1100/src/Validation/HcalHits/src/SimHitsValidationHcal.cc:431
   #9 0x7f478afb9b46 in SimHitsValidationHcal::bookHistograms(DQMStore::IBooker&, edm::Run const&, edm::EventSetup const&) /mnt/build/davidlt/asan2/CMSSW_7_6_ASAN_X_2015-10-19-1100/src/Validation/HcalHits/src/SimHitsValidationHcal.cc:38
   #10 0x7f47a6757891 in DQMEDAnalyzer::beginRun(edm::Run const&, edm::EventSetup const&)::{lambda(DQMStore::IBooker&)#1}::operator()(DQMStore::IBooker&) const (/mnt/build/davidlt/asan2/CMSSW_7_6_ASAN_X_2015-10-19-1100/lib/slc6_amd64_gcc493/libDQMServicesCore.so+0x7f891)
   #11 0x7f47a675828c in bookTransaction<DQMEDAnalyzer::beginRun(const edm::Run&, const edm::EventSetup&)::<lambda(DQMStore::IBooker&)> > /mnt/build/davidlt/asan2/CMSSW_7_6_ASAN_X_2015-10-19-1100/src/DQMServices/Core/interface/DQMStore.h:254
   #12 0x7f47a6757997 in DQMEDAnalyzer::beginRun(edm::Run const&, edm::EventSetup const&) /mnt/build/davidlt/asan2/CMSSW_7_6_ASAN_X_2015-10-19-1100/src/DQMServices/Core/src/DQMEDAnalyzer.cc:26
   #13 0x7f47b887de5e in edm::stream::EDAnalyzerAdaptorBase::doStreamBeginRun(edm::StreamID, edm::RunPrincipal&, edm::EventSetup const&, edm::ModuleCallingContext const*) (/mnt/build/davidlt/asan2/a/slc6_amd64_gcc493/cms/cmssw/CMSSW_7_6_ASAN_X_2015-10-19-1100/lib/slc6_amd64_gcc493/libFWCoreFramework.so+0x3a9e5e)
   #14 0x7f47b8858965 in edm::WorkerT<edm::stream::EDAnalyzerAdaptorBase>::implDoStreamBegin(edm::StreamID, edm::RunPrincipal&, edm::EventSetup const&, edm::ModuleCallingContext const*) (/mnt/build/davidlt/asan2/a/slc6_amd64_gcc493/cms/cmssw/CMSSW_7_6_ASAN_X_2015-10-19-1100/lib/slc6_amd64_gcc493/libFWCoreFramework.so+0x384965)
   #15 0x7f47b866d2e2 in decltype ({parm#1}()) edm::convertException::wrap<bool edm::Worker::doWork<edm::OccurrenceTraits<edm::RunPrincipal, (edm::BranchActionType)1> >(edm::OccurrenceTraits<edm::RunPrincipal, (edm::BranchActionType)1>::MyPrincipal&, edm::EventSetup const&, edm::StreamID, edm::ParentContext const&, edm::OccurrenceTraits<edm::RunPrincipal, (edm::BranchActionType)1>::Context const*)::{lambda()#1}>(bool edm::Worker::doWork<edm::OccurrenceTraits<edm::RunPrincipal, (edm::BranchActionType)1> >(edm::OccurrenceTraits<edm::RunPrincipal, (edm::BranchActionType)1>::MyPrincipal&, edm::EventSetup const&, edm::StreamID, edm::ParentContext const&, edm::OccurrenceTraits<edm::RunPrincipal, (edm::BranchActionType)1>::Context const*)::{lambda()#1}) (/mnt/build/davidlt/asan2/a/slc6_amd64_gcc493/cms/cmssw/CMSSW_7_6_ASAN_X_2015-10-19-1100/lib/slc6_amd64_gcc493/libFWCoreFramework.so+0x1992e2)
   #16 0x7f47b866d76d in bool edm::Worker::doWork<edm::OccurrenceTraits<edm::RunPrincipal, (edm::BranchActionType)1> >(edm::OccurrenceTraits<edm::RunPrincipal, (edm::BranchActionType)1>::MyPrincipal&, edm::EventSetup const&, edm::StreamID, edm::ParentContext const&, edm::OccurrenceTraits<edm::RunPrincipal, (edm::BranchActionType)1>::Context const*) (/mnt/build/davidlt/asan2/a/slc6_amd64_gcc493/cms/cmssw/CMSSW_7_6_ASAN_X_2015-10-19-1100/lib/slc6_amd64_gcc493/libFWCoreFramework.so+0x19976d)
   #17 0x7f47b866dca3 in decltype ({parm#1}()) edm::convertException::wrap<void edm::Path::processOneOccurrence<edm::OccurrenceTraits<edm::RunPrincipal, (edm::BranchActionType)1> >(edm::OccurrenceTraits<edm::RunPrincipal, (edm::BranchActionType)1>::MyPrincipal&, edm::EventSetup const&, edm::StreamID const&, edm::OccurrenceTraits<edm::RunPrincipal, (edm::BranchActionType)1>::Context const*)::{lambda()#1}>(void edm::Path::processOneOccurrence<edm::OccurrenceTraits<edm::RunPrincipal, (edm::BranchActionType)1> >(edm::OccurrenceTraits<edm::RunPrincipal, (edm::BranchActionType)1>::MyPrincipal&, edm::EventSetup const&, edm::StreamID const&, edm::OccurrenceTraits<edm::RunPrincipal, (edm::BranchActionType)1>::Context const*)::{lambda()#1}) (/mnt/build/davidlt/asan2/a/slc6_amd64_gcc493/cms/cmssw/CMSSW_7_6_ASAN_X_2015-10-19-1100/lib/slc6_amd64_gcc493/libFWCoreFramework.so+0x199ca3)
   #18 0x7f47b866e078 in void edm::Path::processOneOccurrence<edm::OccurrenceTraits<edm::RunPrincipal, (edm::BranchActionType)1> >(edm::OccurrenceTraits<edm::RunPrincipal, (edm::BranchActionType)1>::MyPrincipal&, edm::EventSetup const&, edm::StreamID const&, edm::OccurrenceTraits<edm::RunPrincipal, (edm::BranchActionType)1>::Context const*) (/mnt/build/davidlt/asan2/a/slc6_amd64_gcc493/cms/cmssw/CMSSW_7_6_ASAN_X_2015-10-19-1100/lib/slc6_amd64_gcc493/libFWCoreFramework.so+0x19a078)
   #19 0x7f47b866e53c in decltype ({parm#1}()) edm::convertException::wrap<void edm::StreamSchedule::processOneStream<edm::OccurrenceTraits<edm::RunPrincipal, (edm::BranchActionType)1> >(edm::OccurrenceTraits<edm::RunPrincipal, (edm::BranchActionType)1>::MyPrincipal&, edm::EventSetup const&, bool)::{lambda()#1}>(void edm::StreamSchedule::processOneStream<edm::OccurrenceTraits<edm::RunPrincipal, (edm::BranchActionType)1> >(edm::OccurrenceTraits<edm::RunPrincipal, (edm::BranchActionType)1>::MyPrincipal&, edm::EventSetup const&, bool)::{lambda()#1}) (/mnt/build/davidlt/asan2/a/slc6_amd64_gcc493/cms/cmssw/CMSSW_7_6_ASAN_X_2015-10-19-1100/lib/slc6_amd64_gcc493/libFWCoreFramework.so+0x19a53c)
   #20 0x7f47b866f3a7 in void edm::StreamSchedule::processOneStream<edm::OccurrenceTraits<edm::RunPrincipal, (edm::BranchActionType)1> >(edm::OccurrenceTraits<edm::RunPrincipal, (edm::BranchActionType)1>::MyPrincipal&, edm::EventSetup const&, bool) (/mnt/build/davidlt/asan2/a/slc6_amd64_gcc493/cms/cmssw/CMSSW_7_6_ASAN_X_2015-10-19-1100/lib/slc6_amd64_gcc493/libFWCoreFramework.so+0x19b3a7)
   #21 0x7f47b864ec68 in edm::EventProcessor::beginRun(statemachine::Run const&) (/mnt/build/davidlt/asan2/a/slc6_amd64_gcc493/cms/cmssw/CMSSW_7_6_ASAN_X_2015-10-19-1100/lib/slc6_amd64_gcc493/libFWCoreFramework.so+0x17ac68)
   #22 0x7f47b85ef497 in statemachine::HandleRuns::beginRun(statemachine::Run const&) (/mnt/build/davidlt/asan2/a/slc6_amd64_gcc493/cms/cmssw/CMSSW_7_6_ASAN_X_2015-10-19-1100/lib/slc6_amd64_gcc493/libFWCoreFramework.so+0x11b497)
   #23 0x7f47b85ef676 in statemachine::HandleRuns::setupCurrentRun() (/mnt/build/davidlt/asan2/a/slc6_amd64_gcc493/cms/cmssw/CMSSW_7_6_ASAN_X_2015-10-19-1100/lib/slc6_amd64_gcc493/libFWCoreFramework.so+0x11b676)
   #24 0x7f47b85f5574 in statemachine::NewRun::NewRun(boost::statechart::state<statemachine::NewRun, statemachine::HandleRuns, boost::mpl::list<mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na>, (boost::statechart::history_mode)0>::my_context) (/mnt/build/davidlt/asan2/a/slc6_amd64_gcc493/cms/cmssw/CMSSW_7_6_ASAN_X_2015-10-19-1100/lib/slc6_amd64_gcc493/libFWCoreFramework.so+0x121574)
   #25 0x7f47b86141d3 in boost::statechart::state<statemachine::HandleRuns, statemachine::HandleFiles, statemachine::NewRun, (boost::statechart::history_mode)0>::deep_construct(boost::intrusive_ptr<statemachine::HandleFiles> const&, boost::statechart::state_machine<statemachine::Machine, statemachine::Starting, std::allocator<void>, boost::statechart::null_exception_translator>&) (/mnt/build/davidlt/asan2/a/slc6_amd64_gcc493/cms/cmssw/CMSSW_7_6_ASAN_X_2015-10-19-1100/lib/slc6_amd64_gcc493/libFWCoreFramework.so+0x1401d3)
   #26 0x7f47b8614970 in boost::statechart::simple_state<statemachine::FirstFile, statemachine::HandleFiles, boost::mpl::list<mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na, mpl_::na>, (boost::statechart::history_mode)0>::react_impl(boost::statechart::event_base const&, void const*) (/mnt/build/davidlt/asan2/a/slc6_amd64_gcc493/cms/cmssw/CMSSW_7_6_ASAN_X_2015-10-19-1100/lib/slc6_amd64_gcc493/libFWCoreFramework.so+0x140970)
   #27 0x7f47b866275d in boost::statechart::state_machine<statemachine::Machine, statemachine::Starting, std::allocator<void>, boost::statechart::null_exception_translator>::process_event(boost::statechart::event_base const&) (/mnt/build/davidlt/asan2/a/slc6_amd64_gcc493/cms/cmssw/CMSSW_7_6_ASAN_X_2015-10-19-1100/lib/slc6_amd64_gcc493/libFWCoreFramework.so+0x18e75d)
   #28 0x7f47b863bb29 in edm::EventProcessor::runToCompletion() (/mnt/build/davidlt/asan2/a/slc6_amd64_gcc493/cms/cmssw/CMSSW_7_6_ASAN_X_2015-10-19-1100/lib/slc6_amd64_gcc493/libFWCoreFramework.so+0x167b29)
   #29 0x4a97c7 in main::{lambda()#1}::operator()() const (/mnt/build/davidlt/asan2/a/slc6_amd64_gcc493/cms/cmssw/CMSSW_7_6_ASAN_X_2015-10-19-1100/bin/slc6_amd64_gcc493/cmsRun+0x4a97c7)
   #30 0x41f2ca in main (/mnt/build/davidlt/asan2/a/slc6_amd64_gcc493/cms/cmssw/CMSSW_7_6_ASAN_X_2015-10-19-1100/bin/slc6_amd64_gcc493/cmsRun+0x41f2ca)
   #31 0x7f47b51fcd5c in __libc_start_main (/lib64/libc.so.6+0x1ed5c)
   #32 0x41f7f4 (/mnt/build/davidlt/asan2/a/slc6_amd64_gcc493/cms/cmssw/CMSSW_7_6_ASAN_X_2015-10-19-1100/bin/slc6_amd64_gcc493/cmsRun+0x41f7f4)

Address 0x7fff0da5ef04 is located in stack of thread T0 at offset 372 in frame
   #0 0x7f478afbe1b5 in SimHitsValidationHcal::getHistogramTypes() /mnt/build/davidlt/asan2/CMSSW_7_6_ASAN_X_2015-10-19-1100/src/Validation/HcalHits/src/SimHitsValidationHcal.cc:370

 This frame has 8 object(s):
   [32, 36) 'maxDepth'
   [96, 112) 'names'
   [160, 176) 'type'
   [224, 240) 'dept0'
   [288, 308) 'name1'
   [352, 372) 'name2' <== Memory access at offset 372 overflows this variable
   [416, 448) 'hfty1'
   [480, 512) 'hfty2'
HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext
     (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow ??:0 memcpy
Shadow bytes around the buggy address:
 0x100061b43d90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 0x100061b43da0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
 0x100061b43db0: 00 00 f1 f1 f1 f1 04 f4 f4 f4 f2 f2 f2 f2 00 00
 0x100061b43dc0: f4 f4 f2 f2 f2 f2 00 00 f4 f4 f2 f2 f2 f2 00 00
 0x100061b43dd0: f4 f4 f2 f2 f2 f2 00 00 04 f4 f2 f2 f2 f2 00 00
=>0x100061b43de0:[04]f4 f2 f2 f2 f2 00 00 00 00 f2 f2 f2 f2 00 00
 0x100061b43df0: 00 00 f3 f3 f3 f3 00 00 00 00 00 00 00 00 00 00
 0x100061b43e00: 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1
 0x100061b43e10: 00 00 00 f4 f2 f2 f2 f2 00 00 00 f4 f2 f2 f2 f2
 0x100061b43e20: 00 00 00 00 f2 f2 f2 f2 00 00 00 00 00 00 00 00
 0x100061b43e30: 00 00 00 00 04 f4 f4 f4 f2 f2 f2 f2 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
 Addressable:           00
 Partially addressable: 01 02 03 04 05 06 07 
 Heap left redzone:       fa
 Heap right redzone:      fb
 Freed heap region:       fd
 Stack left redzone:      f1
 Stack mid redzone:       f2
 Stack right redzone:     f3
 Stack partial redzone:   f4
 Stack after return:      f5
 Stack use after scope:   f8
 Global redzone:          f9
 Global init order:       f6
 Poisoned by user:        f7
 Contiguous container OOB:fc
 ASan internal:           fe
==14834==ABORTING

Signed-off-by: David Abdurachmanov David.Abdurachmanov@cern.ch

Double buffer size for 'name1' and 'name2'
b134384 
ASan detected stack-buffer-overflow (READ of 23 bytes) in
std::basic_string constructor which is called by
`SimHitsValidationHcal::getHistogramTypes()`.

A quick test showed that we always overflow `name2`, it's usual size is
21-23 chars.

The patch doubles buffers size for `name1` and `name2` to 40 bytes and
uses `snprintf` to make sure it never overflows.

Signed-off-by: David Abdurachmanov <David.Abdurachmanov@cern.ch>
@davidlt
Copy link
Contributor Author

davidlt commented Oct 20, 2015

This was detected while running 101.0 workflow step1.

@cmsbuild cmsbuild added this to the Next CMSSW_7_6_X milestone Oct 20, 2015
@cmsbuild
Copy link
Contributor

A new Pull Request was created by @davidlt for CMSSW_7_6_X.

Double buffer size for 'name1' and 'name2'

It involves the following packages:

Validation/HcalHits

@cmsbuild, @danduggan, @vanbesien, @deguio can you please review it and eventually sign? Thanks.
You can sign-off by replying to this message having '+1' in the first line of your reply.
You can reject by replying to this message having '-1' in the first line of your reply.
If you are a L2 or a release manager you can ask for tests by saying 'please test' in the first line of a comment.
@Degano you are the release manager for this.
You can merge this pull request by typing 'merge' in the first line of your comment.

@cmsbuild
Copy link
Contributor

+1
Tested at: b134384
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-11987/9077/summary.html

@cmsbuild
Copy link
Contributor

Comparison is ready
https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-11987/9077/summary.html

@davidlt
Copy link
Contributor Author

davidlt commented Oct 26, 2015

ping^1

@davidlt
Copy link
Contributor Author

davidlt commented Oct 28, 2015

@davidlange6 this one is a trivial bug fix for out-of-bounds write/read.

davidlange6 added a commit that referenced this pull request Oct 28, 2015
@davidlange6
Merge pull request #11987 from davidlt/fix-stack-overflow-val
50895f5 
Double buffer size for 'name1' and 'name2'
@davidlange6 davidlange6 merged commit 50895f5 into cms-sw:CMSSW_7_6_X Oct 28, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
comparison-available dqm-pending orp-pending pending-signatures tests-approved
Projects
None yet
Milestone
CMSSW_7_6_X
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@davidlt @cmsbuild @davidlange6