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.

Sign up for GitHub

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

Buffer overflow in HGCalCLUEAlgoT::computeThreshold #31940

Closed
Dr15Jones opened this issue Oct 26, 2020 · 21 comments
Closed

Buffer overflow in HGCalCLUEAlgoT::computeThreshold #31940

Dr15Jones opened this issue Oct 26, 2020 · 21 comments
Labels
fully-signed reconstruction-approved upgrade-approved

Comments

@Dr15Jones
Copy link
Contributor

In ASAN intergration builds are showing a buffer overflow in HGCalCLUEAlgoT<HGCalLayerTilesT<HGCalTilesConstants> >::computeThreshold()
@cmsbuild
Copy link
Contributor

A new Issue was created by @Dr15Jones Chris Jones.

@Dr15Jones, @dpiparo, @silviodonato, @smuzaffar, @makortel, @qliphy can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

@Dr15Jones
Copy link
Contributor Author

The full information is

==18194==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60300493f050 at pc 0x2b130c97eaf7 bp 0x7ffd3a094250 sp 0x7ffd3a094248
READ of size 8 at 0x60300493f050 thread T0
    #0 0x2b130c97eaf6 in HGCalCLUEAlgoT<HGCalLayerTilesT<HGCalTilesConstants> >::computeThreshold() (/cvmfs/cms-ib.cern.ch/nweek-02651/slc7_amd64_gcc820/cms/cmssw/CMSSW_11_2_ASAN_X_2020-10-23-2300/lib/slc7_amd64_gcc820/pluginRecoLocalCaloHGCalRecProducersPlugins.so+0x96af6)
    #1 0x2b130c97fee7 in HGCalCLUEAlgoT<HGCalLayerTilesT<HGCalTilesConstants> >::populate(edm::SortedCollection<HGCRecHit, edm::StrictWeakOrdering<HGCRecHit> > const&) (/cvmfs/cms-ib.cern.ch/nweek-02651/slc7_amd64_gcc820/cms/cmssw/CMSSW_11_2_ASAN_X_2020-10-23-2300/lib/slc7_amd64_gcc820/pluginRecoLocalCaloHGCalRecProducersPlugins.so+0x97ee7)
    #2 0x2b130c9d2e24 in HGCalLayerClusterProducer::produce(edm::Event&, edm::EventSetup const&) (/cvmfs/cms-ib.cern.ch/nweek-02651/slc7_amd64_gcc820/cms/cmssw/CMSSW_11_2_ASAN_X_2020-10-23-2300/lib/slc7_amd64_gcc820/pluginRecoLocalCaloHGCalRecProducersPlugins.so+0xeae24)
[cut]

0x60300493f050 is located 0 bytes to the right of 32-byte region [0x60300493f030,0x60300493f050)
allocated by thread T0 here:
    #0 0x2b12a1e68db0 in operator new(unsigned long) ../../../../libsanitizer/asan/asan_new_delete.cc:90
    #1 0x2b12a397b745 in void std::vector<double, std::allocator<double> >::_M_realloc_insert<double const&>(__gnu_cxx::__normal_iterator<double*, std::vector<double, std::allocator<double> > >, double const&) (/cvmfs/cms-ib.cern.ch/nweek-02651/slc7_amd64_gcc820/cms/cmssw/CMSSW_11_2_ASAN_X_2020-10-23-2300/lib/slc7_amd64_gcc820/libFWCoreParameterSet.so+0x2a0745)
    #2 0x2b12a397121e in edm::decode(std::vector<double, std::allocator<double> >&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) (/cvmfs/cms-ib.cern.ch/nweek-02651/slc7_amd64_gcc820/cms/cmssw/CMSSW_11_2_ASAN_X_2020-10-23-2300/lib/slc7_amd64_gcc820/libFWCoreParameterSet.so+0x29621e)
    #3 0x2b12a37aac18 in edm::Entry::getVDouble() const (/cvmfs/cms-ib.cern.ch/nweek-02651/slc7_amd64_gcc820/cms/cmssw/CMSSW_11_2_ASAN_X_2020-10-23-2300/lib/slc7_amd64_gcc820/libFWCoreParameterSet.so+0xcfc18)
    #4 0x2b12a3887041 in std::vector<double, std::allocator<double> > edm::ParameterSet::getParameter<std::vector<double, std::allocator<double> > >(char const*) const (/cvmfs/cms-ib.cern.ch/nweek-02651/slc7_amd64_gcc820/cms/cmssw/CMSSW_11_2_ASAN_X_2020-10-23-2300/lib/slc7_amd64_gcc820/libFWCoreParameterSet.so+0x1ac041)
    #5 0x2b130c97b834 in HGCalCLUEAlgoT<HGCalLayerTilesT<HGCalTilesConstants> >::HGCalCLUEAlgoT(edm::ParameterSet const&) (/cvmfs/cms-ib.cern.ch/nweek-02651/slc7_amd64_gcc820/cms/cmssw/CMSSW_11_2_ASAN_X_2020-10-23-2300/lib/slc7_amd64_gcc820/pluginRecoLocalCaloHGCalRecProducersPlugins.so+0x93834)
    #6 0x2b130c9c055a in edmplugin::PluginFactory<HGCalClusteringAlgoBase* (edm::ParameterSet const&)>::PMaker<HGCalCLUEAlgoT<HGCalLayerTilesT<HGCalTilesConstants> > >::create(edm::ParameterSet const&) const (/cvmfs/cms-ib.cern.ch/nweek-02651/slc7_amd64_gcc820/cms/cmssw/CMSSW_11_2_ASAN_X_2020-10-23-2300/lib/slc7_amd64_gcc820/pluginRecoLocalCaloHGCalRecProducersPlugins.so+0xd855a)
    #7 0x2b130c9c79f7 in HGCalLayerClusterProducer::HGCalLayerClusterProducer(edm::ParameterSet const&) (/cvmfs/cms-ib.cern.ch/nweek-02651/slc7_amd64_gcc820/cms/cmssw/CMSSW_11_2_ASAN_X_2020-10-23-2300/lib/slc7_amd64_gcc820/pluginRecoLocalCaloHGCalRecProducersPlugins.so+0xdf9f7)
[cut]

SUMMARY: AddressSanitizer: heap-buffer-overflow (/cvmfs/cms-ib.cern.ch/nweek-02651/slc7_amd64_gcc820/cms/cmssw/CMSSW_11_2_ASAN_X_2020-10-23-2300/lib/slc7_amd64_gcc820/pluginRecoLocalCaloHGCalRecProducersPlugins.so+0x96af6) in HGCalCLUEAlgoT<HGCalLayerTilesT<HGCalTilesConstants> >::computeThreshold()
Shadow bytes around the buggy address:
  0x0c068091fdb0: fa fa fa fa fa fa fa fa 00 00 00 fa fa fa fa fa
  0x0c068091fdc0: fa fa fa fa 00 00 00 fa fa fa 00 00 01 fa fa fa
  0x0c068091fdd0: 00 00 01 fa fa fa 00 00 00 fa fa fa fa fa fa fa
  0x0c068091fde0: fa fa 00 00 00 fa fa fa 00 00 04 fa fa fa 00 00
  0x0c068091fdf0: 01 fa fa fa fa fa fa fa fa fa 00 00 01 fa fa fa
=>0x0c068091fe00: fa fa fa fa fa fa 00 00 00 00[fa]fa 00 00 00 07
  0x0c068091fe10: fa fa 00 00 01 fa fa fa fa fa fa fa fa fa 00 00
  0x0c068091fe20: 00 fa fa fa 00 00 07 fa fa fa 00 00 00 00 fa fa
  0x0c068091fe30: 00 00 00 fa fa fa 00 00 00 fa fa fa 00 00 00 fa
  0x0c068091fe40: fa fa 00 00 00 00 fa fa 00 00 01 fa fa fa 00 00
  0x0c068091fe50: 00 00 fa fa fa fa fa fa fa fa fa fa fa fa fa fa
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
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb

@Dr15Jones
Copy link
Contributor Author

assign reconstruction, upgrade

@cmsbuild
Copy link
Contributor

New categories assigned: upgrade,reconstruction

@slava77,@perrotta,@jpata,@kpedro88 you have been requested to review this Pull request/Issue and eventually sign? Thanks

@perrotta
Copy link
Contributor

@felicepantaleo @rovere

@rovere
Copy link
Contributor

rovere commented Oct 27, 2020

Thanks for reporting. After some further investigation, I think I've found the bug. We will submit a fix asap.

@rovere rovere mentioned this issue Nov 4, 2020
Merged
@perrotta
Copy link
Contributor

perrotta commented Nov 11, 2020 •
edited

@Dr15Jones
Copy link
Contributor Author

The problem still remains even after the pull request #32021 . See https://cmssdt.cern.ch/SDT/cgi-bin/logreader/slc7_amd64_gcc820/CMSSW_11_2_ASAN_X_2020-11-09-2300/pyRelValMatrixLogs/run/23234.9_TTbar_14TeV+2026D49_vectorHits+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14INPUT+DigiTrigger+RecoGlobal+HARVESTGlobal/step3_TTbar_14TeV+2026D49_vectorHits+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14INPUT+DigiTrigger+RecoGlobal+HARVESTGlobal.log#/

@kpedro88
Copy link
Contributor

@smuzaffar @mrodozov is there a way to launch PR tests with the ASAN IB, so (in the future) this can be checked before a PR intended to fix an ASAN problem is approved/merged?

@smuzaffar
Copy link
Contributor

currently you can requets the test by doing "please test with CMSSW_11_2_ASAN_X" to run additional PR tests for ASAN IBs

@perrotta
Copy link
Contributor

Removed the "+1" previously issued for reco, waiting for the actual fix
@felicepantaleo @rovere

@perrotta
Copy link
Contributor

currently you can requets the test by doing "please test with CMSSW_11_2_ASAN_X" to run additional PR tests for ASAN IBs

@smuzaffar this is indeed what I did for #32021, and the results (see #32021 (comment)) didn't point to any error related to HGCal. The only issue was in a AlCa related step of wf 1001 (MinBias2011A) and obviously unrelated to HGCal

@perrotta
Copy link
Contributor

@Dr15Jones your test was run with CMSSW_11_2_ASAN_X_2020-11-09-2300, while #32021 was merged for CMSSW_11_2_X_2020-11-10-2300, i.e. more recent.
I bet ASAN test will succeed for a more recent ASAN IB

@smuzaffar
Copy link
Contributor

I think workflow 23234.9 is not part of PR tests that is why it was not shown.

@smuzaffar
Copy link
Contributor

and indeed we do not have an ASAN IB with #32021 yet (there will be one tonight)

@perrotta
Copy link
Contributor

I think workflow 23234.9 is not part of PR tests that is why it was not shown.

The very same error appeared also on 23234.0, which is part of the PR tests

@rovere
Copy link
Contributor

rovere commented Nov 11, 2020

The problem still remains even after the pull request #32021 . See https://cmssdt.cern.ch/SDT/cgi-bin/logreader/slc7_amd64_gcc820/CMSSW_11_2_ASAN_X_2020-11-09-2300/pyRelValMatrixLogs/run/23234.9_TTbar_14TeV+2026D49_vectorHits+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14INPUT+DigiTrigger+RecoGlobal+HARVESTGlobal/step3_TTbar_14TeV+2026D49_vectorHits+TTbar_14TeV_TuneCP5_GenSimHLBeamSpot14INPUT+DigiTrigger+RecoGlobal+HARVESTGlobal.log#/

That IB, unless I'm wrong, does not have the fix merged.

@perrotta
Copy link
Contributor

+1

@rovere
Copy link
Contributor

rovere commented Nov 16, 2020

Dear all,
should we close this issue?

@kpedro88
Copy link
Contributor

+upgrade

@cmsbuild
Copy link
Contributor

This issue is fully signed and ready to be closed.

@qliphy qliphy closed this as completed Nov 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
fully-signed reconstruction-approved upgrade-approved
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@rovere @Dr15Jones @perrotta @smuzaffar @cmsbuild @kpedro88 @qliphy