Skip to content

Commit

Permalink
[tree] Avoid heap-use-after-free in BulkApiSillyStruct test
Browse files Browse the repository at this point in the history 
The destructor of TTreeReader was making use of the TFile after its destruction.

```
480: ==2560128==ERROR: AddressSanitizer: heap-use-after-free on address 0x617000011648 at pc 0x7fb9d1ae089e bp 0x7ffd8e35b0a0 sp 0x7ffd8e35b098
480: READ of size 8 at 0x617000011648 thread T0
480:     #0 0x7fb9d1ae089d in TTree::GetNotify() const /home/vpadulan/Programs/rootproject/rootsrc/tree/tree/inc/TTree.h:503
480:     #1 0x7fb9d1ae089d in void TNotifyLinkBase::RemoveLink<TTree>(TTree&) /home/vpadulan/Programs/rootproject/rootsrc/core/base/inc/TNotifyLin
k.h:104
480:     #2 0x7fb9d1ae089d in TTreeReader::~TTreeReader() /home/vpadulan/Programs/rootproject/rootsrc/tree/treeplayer/src/TTreeReader.cxx:252
480:     #3 0x433968 in BulkApiSillyStructTest_stdReadSplitBranch_Test::TestBody() /home/vpadulan/Programs/rootproject/rootsrc/tree/tree/test/Bulk
ApiSillyStruct.cxx:104
480:     #4 0x470bac in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)()
, char const*) (/home/vpadulan/Programs/rootproject/rootbuild/bulksilly-heap-use-after-free-testing-asan/tree/tree/test/testBulkApiSillyStruct+0x4
70bac) (BuildId: 45d8d15c24ba2aae252398ec00943aba7665b034)
480:     #5 0x45a5f3 in testing::Test::Run() [clone .part.0] (/home/vpadulan/Programs/rootproject/rootbuild/bulksilly-heap-use-after-free-testing-
asan/tree/tree/test/testBulkApiSillyStruct+0x45a5f3) (BuildId: 45d8d15c24ba2aae252398ec00943aba7665b034)
480:     #6 0x45a969 in testing::TestInfo::Run() (/home/vpadulan/Programs/rootproject/rootbuild/bulksilly-heap-use-after-free-testing-asan/tree/tr
ee/test/testBulkApiSillyStruct+0x45a969) (BuildId: 45d8d15c24ba2aae252398ec00943aba7665b034)
480:     #7 0x45ab10 in testing::TestSuite::Run() [clone .part.0] (/home/vpadulan/Programs/rootproject/rootbuild/bulksilly-heap-use-after-free-tes
ting-asan/tree/tree/test/testBulkApiSillyStruct+0x45ab10) (BuildId: 45d8d15c24ba2aae252398ec00943aba7665b034)
480:     #8 0x4675be in testing::internal::UnitTestImpl::RunAllTests() (/home/vpadulan/Programs/rootproject/rootbuild/bulksilly-heap-use-after-fre
e-testing-asan/tree/tree/test/testBulkApiSillyStruct+0x4675be) (BuildId: 45d8d15c24ba2aae252398ec00943aba7665b034)
480:     #9 0x45af6c in testing::UnitTest::Run() (/home/vpadulan/Programs/rootproject/rootbuild/bulksilly-heap-use-after-free-testing-asan/tree/tr
ee/test/testBulkApiSillyStruct+0x45af6c) (BuildId: 45d8d15c24ba2aae252398ec00943aba7665b034)
480:     #10 0x425456 in main (/home/vpadulan/Programs/rootproject/rootbuild/bulksilly-heap-use-after-free-testing-asan/tree/tree/test/testBulkApi
SillyStruct+0x425456) (BuildId: 45d8d15c24ba2aae252398ec00943aba7665b034)
480:     #11 0x7fb9cd446149 in __libc_start_call_main (/lib64/libc.so.6+0x28149) (BuildId: 0d710e9d9dc10c500b8119c85da75004183618e2)
480:     #12 0x7fb9cd44620a in __libc_start_main_impl (/lib64/libc.so.6+0x2820a) (BuildId: 0d710e9d9dc10c500b8119c85da75004183618e2)
480:     #13 0x4257b4 in _start (/home/vpadulan/Programs/rootproject/rootbuild/bulksilly-heap-use-after-free-testing-asan/tree/tree/test/testBulkA
piSillyStruct+0x4257b4) (BuildId: 45d8d15c24ba2aae252398ec00943aba7665b034)

480: 0x617000011648 is located 328 bytes inside of 712-byte region [0x617000011500,0x6170000117c8)
480: freed by thread T0 here:
480:     #0 0x7fb9d1eda878 in operator delete(void*) (/lib64/libasan.so.8+0xda878) (BuildId: 2e1c50524ff1a2e7e73c4565b46f3f51892353ea)
480:     #1 0x7fb9cdfb4f25 in TCollection::GarbageCollect(TObject*) /home/vpadulan/Programs/rootproject/rootsrc/core/cont/src/TCollection.cxx:736
480:     #2 0x7fb9cdfe8a27 in TList::Delete(char const*) /home/vpadulan/Programs/rootproject/rootsrc/core/cont/src/TList.cxx:535
480:     #3 0x7fb9cdfc53d7 in THashList::Delete(char const*) /home/vpadulan/Programs/rootproject/rootsrc/core/cont/src/THashList.cxx:215
480:     #4 0x7fb9ce8d285d in TDirectoryFile::Close(char const*) /home/vpadulan/Programs/rootproject/rootsrc/io/io/src/TDirectoryFile.cxx:585
480:     #5 0x7fb9ce8d285d in TDirectoryFile::Close(char const*) /home/vpadulan/Programs/rootproject/rootsrc/io/io/src/TDirectoryFile.cxx:561
480:     #6 0x7fb9ce9468e4 in TFile::Close(char const*) /home/vpadulan/Programs/rootproject/rootsrc/io/io/src/TFile.cxx:989
480:     #7 0x7fb9ce9481fd in TFile::~TFile() /home/vpadulan/Programs/rootproject/rootsrc/io/io/src/TFile.cxx:566
480:     #8 0x7fb9ce948fd0 in TFile::~TFile() /home/vpadulan/Programs/rootproject/rootsrc/io/io/src/TFile.cxx:603
480:     #9 0x4338cd in BulkApiSillyStructTest_stdReadSplitBranch_Test::TestBody() /home/vpadulan/Programs/rootproject/rootsrc/tree/tree/test/Bulk
ApiSillyStruct.cxx:103
480:     #10 0x470bac in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(
), char const*) (/home/vpadulan/Programs/rootproject/rootbuild/bulksilly-heap-use-after-free-testing-asan/tree/tree/test/testBulkApiSillyStruct+0x
470bac) (BuildId: 45d8d15c24ba2aae252398ec00943aba7665b034)

480: previously allocated by thread T0 here:
480:     #0 0x7fb9d1ed9e38 in operator new(unsigned long) (/lib64/libasan.so.8+0xd9e38) (BuildId: 2e1c50524ff1a2e7e73c4565b46f3f51892353ea)
480:     #1 0x7fb9cdebbfd0 in TStorage::ObjectAlloc(unsigned long) /home/vpadulan/Programs/rootproject/rootsrc/core/base/src/TStorage.cxx:293
480:     #2 0x7fb9d12cc209 in TObject::operator new(unsigned long) /home/vpadulan/Programs/rootproject/rootbuild/bulksilly-heap-use-after-free-tes
ting-asan/include/TObject.h:181
480:     #3 0x7fb9d12cc209 in new_TTree /home/vpadulan/Programs/rootproject/rootbuild/bulksilly-heap-use-after-free-testing-asan/tree/tree/G__Tree
.cxx:4363
480:     #4 0x7fb9ce10a49c in TClass::NewObject(TClass::ENewType, bool) const /home/vpadulan/Programs/rootproject/rootsrc/core/meta/src/TClass.cxx
:5003
480:     #5 0x7fb9ce113c7d in TClass::New(TClass::ENewType, bool) const /home/vpadulan/Programs/rootproject/rootsrc/core/meta/src/TClass.cxx:4980
480:     #6 0x7fb9ce9cda38 in TKey::ReadObjectAny(TClass const*) /home/vpadulan/Programs/rootproject/rootsrc/io/io/src/TKey.cxx:1086
480:     #7 0x7fb9ce8d10a7 in TDirectoryFile::GetObjectChecked(char const*, TClass const*) /home/vpadulan/Programs/rootproject/rootsrc/io/io/src/T
DirectoryFile.cxx:1111
480:     #8 0x7fb9d1ade1d1 in void TDirectory::GetObject<TTree>(char const*, TTree*&) /home/vpadulan/Programs/rootproject/rootsrc/core/base/inc/TD
irectory.h:214
480:     #9 0x7fb9d1ade1d1 in TTreeReader::TTreeReader(char const*, TDirectory*, TEntryList*) /home/vpadulan/Programs/rootproject/rootsrc/tree/tre
eplayer/src/TTreeReader.cxx:232
480:     #10 0x433332 in BulkApiSillyStructTest_stdReadSplitBranch_Test::TestBody() /home/vpadulan/Programs/rootproject/rootsrc/tree/tree/test/Bu$
kApiSillyStruct.cxx:87
480:     #11 0x470bac in void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(
), char const*) (/home/vpadulan/Programs/rootproject/rootbuild/bulksilly-heap-use-after-free-testing-asan/tree/tree/test/testBulkApiSillyStruct+0x
470bac) (BuildId: 45d8d15c24ba2aae252398ec00943aba7665b034)
480:
480: SUMMARY: AddressSanitizer: heap-use-after-free /home/vpadulan/Programs/rootproject/rootsrc/tree/tree/inc/TTree.h:503 in TTree::GetNotify() co
nst
```
  • Loading branch information
@vepadulano @guitargeek
vepadulano authored and guitargeek committed Jun 13, 2024
1 parent 703351a commit 5296450
Showing 1 changed file with 5 additions and 7 deletions.
12 changes: 5 additions & 7 deletions tree/tree/test/BulkApiSillyStruct.cxx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,9 @@ constexpr Long64_t BulkApiSillyStructTest::fEventCount;

TEST_F(BulkApiSillyStructTest, stdReadStruct)
{
auto hfile = TFile::Open(fFileName.c_str());
std::unique_ptr<TFile> hfile{TFile::Open(fFileName.c_str())};

TTreeReader myReader("T", hfile);
TTreeReader myReader("T", hfile.get());
TTreeReaderValue<SillyStruct> ss(myReader, "myEvent");

int evI = 0;
Expand All @@ -77,14 +77,13 @@ TEST_F(BulkApiSillyStructTest, stdReadStruct)
evF++;
evD++;
}
delete hfile;
}

TEST_F(BulkApiSillyStructTest, stdReadSplitBranch)
{
auto hfile = TFile::Open(fFileName.c_str());
std::unique_ptr<TFile> hfile{TFile::Open(fFileName.c_str())};

TTreeReader myReader("T", hfile);
TTreeReader myReader("T", hfile.get());
TTreeReaderValue<float> myF(myReader, "f");
TTreeReaderValue<int> myI(myReader, "i");
TTreeReaderValue<double> myD(myReader, "d");
Expand All @@ -100,15 +99,14 @@ TEST_F(BulkApiSillyStructTest, stdReadSplitBranch)
evF++;
evD++;
}
delete hfile;
}

TEST_F(BulkApiSillyStructTest, fastRead)
{
TBufferFile bufF(TBuffer::kWrite, 10000);
TBufferFile bufI(TBuffer::kWrite, 10000);
TBufferFile bufD(TBuffer::kWrite, 10000);
auto hfile = TFile::Open(fFileName.c_str());
std::unique_ptr<TFile> hfile{TFile::Open(fFileName.c_str())};
auto tree = dynamic_cast<TTree*>(hfile->Get("T"));
ASSERT_TRUE(tree);
TBranch *branchF = tree->GetBranch("f");
Expand Down

0 comments on commit 5296450

Please sign in to comment.