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

stack buffer overflow in GsfElectronEcalDrivenProducer #26238

Closed
Dr15Jones opened this issue Mar 24, 2019 · 8 comments
Closed

stack buffer overflow in GsfElectronEcalDrivenProducer #26238

Dr15Jones opened this issue Mar 24, 2019 · 8 comments
Labels
fully-signed reconstruction-approved

Comments

@Dr15Jones
Copy link
Contributor

The ASAN build has found a stack buffer overflow error in EnergyUncertaintyElectronSpecific::computeElectronEnergyUncertainty_cracks which is indirectly called by GsfElectronEcalDrivenProducer.
@Dr15Jones
Copy link
Contributor Author

assign reconstruction

@cmsbuild
Copy link
Contributor

New categories assigned: reconstruction

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

@cmsbuild
Copy link
Contributor

A new Issue was created by @Dr15Jones Chris Jones.

@davidlange6, @Dr15Jones, @smuzaffar, @fabiocos, @kpedro88 can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

@Dr15Jones
Copy link
Contributor Author

The relevant report is
https://cmssdt.cern.ch/SDT/cgi-bin/buildlogs/raw/slc7_amd64_gcc700/CMSSW_10_6_ASAN_X_2019-03-22-2300/pyRelValMatrixLogs/run/11024.2_TTbar_13UP18HEfailINPUT+TTbar_13UP18HEfailINPUT+DigiFullHEfail+RecoFullHEfail+HARVESTFullHEfail+NanoFullHEfail/step3_TTbar_13UP18HEfailINPUT+TTbar_13UP18HEfailINPUT+DigiFullHEfail+RecoFullHEfail+HARVESTFullHEfail+NanoFullHEfail.log

with the information

=================================================================
==16172==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7faea1cf2f78 at pc 0x7fae6d700dad bp 0x7faea1cf2e80 sp 0x7faea1cf2e78
READ of size 4 at 0x7faea1cf2f78 thread T3
%MSG-w SiPixelPhase1TrackClusters:   SiPixelPhase1TrackClusters:hltSiPixelPhase1TrackClustersAnalyzer  23-Mar-2019 16:32:15 CET Run: 1 Event: 6058
PixelClusterShapeCache collection is not valid
%MSG
%MSG-w SiPixelPhase1TrackClusters:   SiPixelPhase1TrackClusters:hltSiPixelPhase1TrackClustersAnalyzer  23-Mar-2019 16:32:17 CET Run: 1 Event: 6057
PixelClusterShapeCache collection is not valid
%MSG
    #0 0x7fae6d700dac in EnergyUncertaintyElectronSpecific::computeElectronEnergyUncertainty_cracks(double, double, double) (/cvmfs/cms-ib.cern.ch/nweek-02568/slc7_amd64_gcc700/cms/cmssw/CMSSW_10_6_ASAN_X_2019-03-22-2300/lib/slc7_amd64_gcc700/libRecoEgammaEgammaElectronAlgos.so+0xacdac)
    #1 0x7fae6d700eba in EnergyUncertaintyElectronSpecific::computeElectronEnergyUncertainty(reco::GsfElectron::Classification, double, double, double) (/cvmfs/cms-ib.cern.ch/nweek-02568/slc7_amd64_gcc700/cms/cmssw/CMSSW_10_6_ASAN_X_2019-03-22-2300/lib/slc7_amd64_gcc700/libRecoEgammaEgammaElectronAlgos.so+0xaceba)
    #2 0x7fae6d75914a in ElectronEnergyCorrector::classBasedParameterizationUncertainty(reco::GsfElectron&) (/cvmfs/cms-ib.cern.ch/nweek-02568/slc7_amd64_gcc700/cms/cmssw/CMSSW_10_6_ASAN_X_2019-03-22-2300/lib/slc7_amd64_gcc700/libRecoEgammaEgammaElectronAlgos.so+0x10514a)
    #3 0x7fae6d7a740e in GsfElectronAlgo::createElectron(std::vector<reco::GsfElectron, std::allocator<reco::GsfElectron> >&, GsfElectronAlgo::ElectronData&, GsfElectronAlgo::EventData&, gsfAlgoHelpers::HeavyObjectCache const*) (/cvmfs/cms-ib.cern.ch/nweek-02568/slc7_amd64_gcc700/cms/cmssw/CMSSW_10_6_ASAN_X_2019-03-22-2300/lib/slc7_amd64_gcc700/libRecoEgammaEgammaElectronAlgos.so+0x15340e)
    #4 0x7fae6d7a8c86 in GsfElectronAlgo::completeElectrons(std::vector<reco::GsfElectron, std::allocator<reco::GsfElectron> >&, edm::Event const&, edm::EventSetup const&, gsfAlgoHelpers::HeavyObjectCache const*) (/cvmfs/cms-ib.cern.ch/nweek-02568/slc7_amd64_gcc700/cms/cmssw/CMSSW_10_6_ASAN_X_2019-03-22-2300/lib/slc7_amd64_gcc700/libRecoEgammaEgammaElectronAlgos.so+0x154c86)
    #5 0x7fae6dba8622 in GsfElectronEcalDrivenProducer::produce(edm::Event&, edm::EventSetup const&) (/cvmfs/cms-ib.cern.ch/nweek-02568/slc7_amd64_gcc700/cms/cmssw/CMSSW_10_6_ASAN_X_2019-03-22-2300/lib/slc7_amd64_gcc700/pluginRecoEgammaEgammaElectronProducersPlugins.so+0x2cf622)
[stack trimmed]

Address 0x7faea1cf2f78 is located in stack of thread T3 at offset 200 in frame
    #0 0x7fae6d6ff31f in EnergyUncertaintyElectronSpecific::computeElectronEnergyUncertainty_cracks(double, double, double) (/cvmfs/cms-ib.cern.ch/nweek-02568/slc7_amd64_gcc700/cms/cmssw/CMSSW_10_6_ASAN_X_2019-03-22-2300/lib/slc7_amd64_gcc700/libRecoEgammaEgammaElectronAlgos.so+0xab31f)

  This frame has 5 object(s):
    [32, 80) 'EtaBins'
    [128, 184) 'BremBins' <== Memory access at offset 200 overflows this variable
    [224, 344) 'par0'
    [384, 504) 'par1'
    [544, 664) 'par2'

[stuff cut]

SUMMARY: AddressSanitizer: stack-buffer-overflow (/cvmfs/cms-ib.cern.ch/nweek-02568/slc7_amd64_gcc700/cms/cmssw/CMSSW_10_6_ASAN_X_2019-03-22-2300/lib/slc7_amd64_gcc700/libRecoEgammaEgammaElectronAlgos.so+0xacdac) in EnergyUncertaintyElectronSpecific::computeElectronEnergyUncertainty_cracks(double, double, double)
Shadow bytes around the buggy address:
  0x0ff654396590: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ff6543965a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ff6543965b0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ff6543965c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ff6543965d0: 00 00 00 00 00 00 f1 f1 f1 f1 00 00 00 00 00 00
=>0x0ff6543965e0: f2 f2 f2 f2 f2 f2 00 00 00 00 00 00 00 f2 f2[f2]
  0x0ff6543965f0: f2 f2 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0ff654396600: 00 f2 f2 f2 f2 f2 00 00 00 00 00 00 00 00 00 00
  0x0ff654396610: 00 00 00 00 00 f2 f2 f2 f2 f2 00 00 00 00 00 00
  0x0ff654396620: 00 00 00 00 00 00 00 00 00 f2 f3 f3 f3 f3 00 00
  0x0ff654396630: 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1 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
  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
==16172==ABORTING

@Dr15Jones
Copy link
Contributor Author

This problem could be explained if iEtaSl were -1 during these lookups (since that would be -24 bytes from the beginning of par0 which corresponds to memory location 200 in the stack).

cmssw/RecoEgamma/EgammaElectronAlgos/src/EnergyUncertaintyElectronSpecific.cc

Lines 180 to 183 in 02d4198

if (et<5) uncertainty = par0[iEtaSl][iBremSl] + par1[iEtaSl][iBremSl]/(5-par2[iEtaSl][iBremSl]);
if (et>100) uncertainty = par0[iEtaSl][iBremSl] + par1[iEtaSl][iBremSl]/(100-par2[iEtaSl][iBremSl]);
if (et>5 && et<100) uncertainty = par0[iEtaSl][iBremSl] + par1[iEtaSl][iBremSl]/(et-par2[iEtaSl][iBremSl]);

I believe such a value could happen if eta were a NaN.

@Dr15Jones
Copy link
Contributor Author

addressed in #26241

@perrotta perrotta mentioned this issue Mar 28, 2019
Merged
@slava77
Copy link
Contributor

slava77 commented May 24, 2019

+1

it seems like magic words in #26241 (comment) did not make this closed

@cmsbuild
Copy link
Contributor

This issue is fully signed and ready to be closed.

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
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@Dr15Jones @smuzaffar @cmsbuild @slava77