search

cms-sw / cmssw

CMS Offline Software
http://cms-sw.github.io/
Apache License 2.0
1.07k stars 4.29k forks source link

EcalMultifitConditionsHostESProducer reads off the end of conditions #44275

Closed Dr15Jones closed 6 months ago

Dr15Jones commented 7 months ago

The ASAN report for CMSSW_14_1_ASAN_X_2024-02-28-2300 has the following item

==1999105==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6020020e61f4 at pc 0x1516ecd26b5b bp 0x7ffe98512670 sp 0x7ffe98511e20
READ of size 284 at 0x6020020e61f4 thread T0
    #0 0x1516ecd26b5a in __interceptor_memcpy ../../../../libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:827
    #1 0x1516b131e593 in alpaka_serial_sync::EcalMultifitConditionsHostESProducer::produce(EcalMultifitConditionsRcd const&) (/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02826/el8_amd64_gcc12/cms/cmssw/CMSSW_14_1_ASAN_X_2024-02-26-2300/lib/el8_amd64_gcc12/pluginRecoLocalCaloEcalRecProducersPluginsPortableSerialSync.so+0xb1593)

...

0x6020020e61f4 is located 0 bytes to the right of 4-byte region [0x6020020e61f0,0x6020020e61f4)
allocated by thread T3 here:
    #0 0x1516ecd976d8 in operator new(unsigned long) ../../../../libsanitizer/asan/asan_new_delete.cpp:95
    #1 0x1516c49e95c8 in std::vector<float, std::allocator<float> >::reserve(unsigned long) (/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02826/el8_amd64_gcc12/cms/cmssw/CMSSW_14_1_ASAN_X_2024-02-26-2300/lib/el8_amd64_gcc12/libDataFormatsTrackerCommon.so+0x175c8)
    #2 0x1516c3ccf54d in boost::archive::detail::iserializer<eos::portable_iarchive, std::vector<float, std::allocator<float> > >::load_object_data(boost::archive::detail::basic_iarchive&, void*, unsigned int) const (/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02826/el8_amd64_gcc12/cms/cmssw/CMSSW_14_1_ASAN_X_2024-02-26-2300/lib/el8_amd64_gcc12/libCondFormatsRunInfo.so+0x15554d)
    #3 0x1516c3fa6c06 in boost::archive::detail::basic_iarchive::load_object(void*, boost::archive::detail::basic_iserializer const&) (/cvmfs/cms-ib.cern.ch/sw/x86_64/nweek-02826/el8_amd64_gcc12/cms/cmssw-patch/CMSSW_14_1_ASAN_X_2024-02-28-2300/external/el8_amd64_gcc12/lib/libboost_serialization.so.1.80.0+0x16c06)

Thread T3 created by T0 here:
    #0 0x1516ecd28136 in __interceptor_pthread_create ../../../../libsanitizer/asan/asan_interceptors.cpp:207
    #1 0x1516ea25316f in tbb::detail::r1::rml::internal::thread_monitor::launch(void* (*)(void*), void*, unsigned long) /data/cmsbld/jenkins/workspace/auto-builds/CMSSW_14_0_0_pre2_SKYLAKEAVX512-el8_amd64_gcc12/build/CMSSW_14_0_0_pre2_SKYLAKEAVX512-build/BUILD/el8_amd64_gcc12/external/tbb/v2021.9.0-5109e373398eaf79a8268d6a86a2f6e2/tbb-v2021.9.0/src/tbb/rml_thread_monitor.h:208
    #2 0x1516ea25316f in tbb::detail::r1::rml::private_worker::wake_or_launch() /data/cmsbld/jenkins/workspace/auto-builds/CMSSW_14_0_0_pre2_SKYLAKEAVX512-el8_amd64_gcc12/build/CMSSW_14_0_0_pre2_SKYLAKEAVX512-build/BUILD/el8_amd64_gcc12/external/tbb/v2021.9.0-5109e373398eaf79a8268d6a86a2f6e2/tbb-v2021.9.0/src/tbb/private_server.cpp:305
    #3 0x1516ea25316f in tbb::detail::r1::rml::private_server::wake_some(int) /data/cmsbld/jenkins/workspace/auto-builds/CMSSW_14_0_0_pre2_SKYLAKEAVX512-el8_amd64_gcc12/build/CMSSW_14_0_0_pre2_SKYLAKEAVX512-build/BUILD/el8_amd64_gcc12/external/tbb/v2021.9.0-5109e373398eaf79a8268d6a86a2f6e2/tbb-v2021.9.0/src/tbb/private_server.cpp:412

SUMMARY: AddressSanitizer: heap-buffer-overflow ../../../../libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:827 in __interceptor_memcpy
Shadow bytes around the buggy address:
  0x0c0480414be0: fa fa fd fa fa fa fd fa fa fa fd fa fa fa fd fd
  0x0c0480414bf0: fa fa fd fa fa fa fd fa fa fa fd fd fa fa fd fa
  0x0c0480414c00: fa fa fd fd fa fa fd fa fa fa fd fd fa fa fd fd
  0x0c0480414c10: fa fa fd fa fa fa fd fd fa fa fd fa fa fa fd fd
  0x0c0480414c20: fa fa fd fd fa fa 00 00 fa fa 00 00 fa fa fd fa
=>0x0c0480414c30: fa fa 04 fa fa fa 04 fa fa fa 00 00 fa fa[04]fa
  0x0c0480414c40: fa fa fd fd fa fa fd fa fa fa 00 fa fa fa 04 fa
  0x0c0480414c50: fa fa 00 fa fa fa 00 fa fa fa 00 fa fa fa 00 fa
  0x0c0480414c60: fa fa 00 fa fa fa fd fd fa fa 00 00 fa fa 00 fa
  0x0c0480414c70: fa fa 00 fa fa fa fd fa fa fa fd fa fa fa 00 fa
  0x0c0480414c80: fa fa 00 fa fa fa 00 fa fa fa 00 fa fa fa 00 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

It appears that at least one of the calls to std::memcpy is reading pass the end of the conditions for which it is copying.

cmsbuild commented 7 months ago

cms-bot internal usage

cmsbuild commented 7 months ago

A new Issue was created by @Dr15Jones.

@rappoccio, @smuzaffar, @sextonkennedy, @Dr15Jones, @antoniovilela, @makortel can you please review it and eventually sign/assign? Thanks.

cms-bot commands are listed here

Dr15Jones commented 7 months ago

assign RecoLocalCalo/EcalRecProducers

cmsbuild commented 7 months ago

New categories assigned: reconstruction

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

makortel commented 7 months ago

assign heterogeneous

makortel commented 7 months ago

FYI @cms-sw/ecal-dpg-l2 @thomreis

cmsbuild commented 7 months ago

New categories assigned: heterogeneous

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

fwyzard commented 7 months ago

type ecal

thomreis commented 7 months ago

Should be fixed with #44301 I will also make a backport PR for 140X.

thomreis commented 7 months ago

Backport to 140x: #44304

fwyzard commented 7 months ago

Just to understand - the fix is just

-                  samplesCorrelationData.EBG6SamplesCorrelation.data(),
+                  samplesCorrelationData.EEG6SamplesCorrelation.data(),

while the others are overall improvements ?

thomreis commented 7 months ago

Just to understand - the fix is just

-                  samplesCorrelationData.EBG6SamplesCorrelation.data(),
+                  samplesCorrelationData.EEG6SamplesCorrelation.data(),

while the others are overall improvements ?

No that is the typo that got fixed as well.

The real issue was that the std::memcpy did use the maximum possible input size instead of the actual size of the vectors. So more bytes were copied than the input provided. This was not a problem downstream since those extra bytes were never accessed because the actual size was passed as well, but nevertheless they should not have been copied.

But if the PR changes the output compared to the current version it is likely from the fix of the typo (EB -> EE).

fwyzard commented 7 months ago

Understood, thanks.

antoniovilela commented 7 months ago

https://github.com/cms-sw/cmssw/pull/44301 is merged.

makortel commented 6 months ago

+heterogeneous

jfernan2 commented 6 months ago

+1 Fixed by https://github.com/cms-sw/cmssw/pull/44301

cmsbuild commented 6 months ago

This issue is fully signed and ready to be closed.

makortel commented 6 months ago

@cmsbuild, please close