Skip to content

[SharedCache] Fix some memory leaks in shared cache loading #6124

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

Closed
bdash wants to merge 2 commits into from
+30 −32
Closed

[SharedCache] Fix some memory leaks in shared cache loading #6124

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d4fc229
Don't leak DataBuffers
bdash Nov 12, 2024
cc1b055
Append export and symbol info to correct arrays
bdash Nov 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 19 additions & 19 deletions view/sharedcache/core/SharedCache.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ void SharedCache::PerformInitialLoad()

m_baseFilePath = path;

DataBuffer sig = *baseFile->ReadBuffer(0, 4);
DataBuffer sig = baseFile->ReadBuffer(0, 4);
if (sig.GetLength() != 4)
abort();
const char* magic = (char*)sig.GetData();
Expand Down Expand Up @@ -1571,13 +1571,13 @@ bool SharedCache::LoadSectionAtAddress(uint64_t address)

auto name = stubIsland.prettyName;
m_dscView->GetParentView()->GetParentView()->WriteBuffer(
m_dscView->GetParentView()->GetParentView()->GetEnd(), *buff);
m_dscView->GetParentView()->GetParentView()->GetEnd(), buff);
m_dscView->GetParentView()->AddAutoSegment(rawViewEnd, stubIsland.size, rawViewEnd, stubIsland.size,
SegmentReadable | SegmentExecutable);
m_dscView->AddUserSegment(stubIsland.start, stubIsland.size, rawViewEnd, stubIsland.size,
SegmentReadable | SegmentExecutable);
m_dscView->AddUserSection(name, stubIsland.start, stubIsland.size, ReadOnlyCodeSectionSemantics);
m_dscView->WriteBuffer(stubIsland.start, *buff);
m_dscView->WriteBuffer(stubIsland.start, buff);

stubIsland.loaded = true;

Expand Down Expand Up @@ -1611,13 +1611,13 @@ bool SharedCache::LoadSectionAtAddress(uint64_t address)

auto name = dyldData.prettyName;
m_dscView->GetParentView()->GetParentView()->WriteBuffer(
m_dscView->GetParentView()->GetParentView()->GetEnd(), *buff);
m_dscView->GetParentView()->WriteBuffer(rawViewEnd, *buff);
m_dscView->GetParentView()->GetParentView()->GetEnd(), buff);
m_dscView->GetParentView()->WriteBuffer(rawViewEnd, buff);
m_dscView->GetParentView()->AddAutoSegment(rawViewEnd, dyldData.size, rawViewEnd, dyldData.size,
SegmentReadable);
m_dscView->AddUserSegment(dyldData.start, dyldData.size, rawViewEnd, dyldData.size, SegmentReadable);
m_dscView->AddUserSection(name, dyldData.start, dyldData.size, ReadOnlyDataSectionSemantics);
m_dscView->WriteBuffer(dyldData.start, *buff);
m_dscView->WriteBuffer(dyldData.start, buff);

dyldData.loaded = true;
dyldData.rawViewOffsetIfLoaded = rawViewEnd;
Expand Down Expand Up @@ -1650,12 +1650,12 @@ bool SharedCache::LoadSectionAtAddress(uint64_t address)

auto name = region.prettyName;
m_dscView->GetParentView()->GetParentView()->WriteBuffer(
m_dscView->GetParentView()->GetParentView()->GetEnd(), *buff);
m_dscView->GetParentView()->WriteBuffer(rawViewEnd, *buff);
m_dscView->GetParentView()->GetParentView()->GetEnd(), buff);
m_dscView->GetParentView()->WriteBuffer(rawViewEnd, buff);
m_dscView->GetParentView()->AddAutoSegment(rawViewEnd, region.size, rawViewEnd, region.size, region.flags);
m_dscView->AddUserSegment(region.start, region.size, rawViewEnd, region.size, region.flags);
m_dscView->AddUserSection(name, region.start, region.size, ReadOnlyCodeSectionSemantics);
m_dscView->WriteBuffer(region.start, *buff);
m_dscView->WriteBuffer(region.start, buff);

region.loaded = true;
region.rawViewOffsetIfLoaded = rawViewEnd;
Expand Down Expand Up @@ -1685,13 +1685,13 @@ bool SharedCache::LoadSectionAtAddress(uint64_t address)
ParseAndApplySlideInfoForFile(targetFile);
auto buff = reader.ReadBuffer(targetSegment->start, targetSegment->size);
m_dscView->GetParentView()->GetParentView()->WriteBuffer(
m_dscView->GetParentView()->GetParentView()->GetEnd(), *buff);
m_dscView->GetParentView()->WriteBuffer(rawViewEnd, *buff);
m_dscView->GetParentView()->GetParentView()->GetEnd(), buff);
m_dscView->GetParentView()->WriteBuffer(rawViewEnd, buff);
m_dscView->GetParentView()->AddAutoSegment(
rawViewEnd, targetSegment->size, rawViewEnd, targetSegment->size, SegmentReadable);
m_dscView->AddUserSegment(
targetSegment->start, targetSegment->size, rawViewEnd, targetSegment->size, targetSegment->flags);
m_dscView->WriteBuffer(targetSegment->start, *buff);
m_dscView->WriteBuffer(targetSegment->start, buff);

targetSegment->loaded = true;
targetSegment->rawViewOffsetIfLoaded = rawViewEnd;
Expand Down Expand Up @@ -1764,8 +1764,8 @@ bool SharedCache::LoadImageWithInstallName(std::string installName)
auto rawViewEnd = m_dscView->GetParentView()->GetEnd();

auto buff = reader.ReadBuffer(region.start, region.size);
m_dscView->GetParentView()->GetParentView()->WriteBuffer(rawViewEnd, *buff);
m_dscView->GetParentView()->WriteBuffer(rawViewEnd, *buff);
m_dscView->GetParentView()->GetParentView()->WriteBuffer(rawViewEnd, buff);
m_dscView->GetParentView()->WriteBuffer(rawViewEnd, buff);

region.loaded = true;
region.rawViewOffsetIfLoaded = rawViewEnd;
Expand All @@ -1774,7 +1774,7 @@ bool SharedCache::LoadImageWithInstallName(std::string installName)

m_dscView->GetParentView()->AddAutoSegment(rawViewEnd, region.size, rawViewEnd, region.size, region.flags);
m_dscView->AddUserSegment(region.start, region.size, rawViewEnd, region.size, region.flags);
m_dscView->WriteBuffer(region.start, *buff);
m_dscView->WriteBuffer(region.start, buff);

regionsToLoad.push_back(&region);
}
Expand Down Expand Up @@ -2530,7 +2530,7 @@ void SharedCache::InitializeHeader(

while (i < header.functionStarts.funcsize)
{
curOffset = readLEB128(*funcStarts, header.functionStarts.funcsize, i);
curOffset = readLEB128(funcStarts, header.functionStarts.funcsize, i);
bool addFunction = false;
for (const auto& region : regionsToLoad)
{
Expand Down Expand Up @@ -2569,7 +2569,7 @@ void SharedCache::InitializeHeader(
if (sym.n_strx >= header.symtab.strsize || ((sym.n_type & N_TYPE) == N_INDR))
continue;

std::string symbol((char*)strtab->GetDataAt(sym.n_strx));
std::string symbol((char*)strtab.GetDataAt(sym.n_strx));
// BNLogError("%s: 0x%llx", symbol.c_str(), sym.n_value);
if (symbol == "<redacted>")
continue;
Expand Down Expand Up @@ -2769,8 +2769,8 @@ std::vector<Ref<Symbol>> SharedCache::ParseExportTrie(std::shared_ptr<MMappedFil

std::vector<ExportNode> nodes;

DataBuffer* buffer = reader->ReadBuffer(header.exportTrie.dataoff, header.exportTrie.datasize);
ReadExportNode(symbols, header, *buffer, header.textBase, "", 0, header.exportTrie.datasize);
DataBuffer buffer = reader->ReadBuffer(header.exportTrie.dataoff, header.exportTrie.datasize);
ReadExportNode(symbols, header, buffer, header.textBase, "", 0, header.exportTrie.datasize);
}
catch (std::exception& e)
{
Expand Down
4 changes: 2 additions & 2 deletions view/sharedcache/core/SharedCache.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -967,7 +967,7 @@ namespace SharedCacheCore {
m_activeContext.allocator);
exportInfoArr.PushBack(exportInfoPairArr, m_activeContext.allocator);
}
exportInfoArr.PushBack(exportInfoArr, m_activeContext.allocator);
exportInfos.PushBack(exportInfoArr, m_activeContext.allocator);
}
m_activeContext.doc.AddMember("exportInfos", exportInfos, m_activeContext.allocator);
// std::vector<std::pair<uint64_t, std::vector<std::pair<uint64_t, std::pair<BNSymbolType, std::string>>>>> m_symbolInfos
Expand All @@ -985,7 +985,7 @@ namespace SharedCacheCore {
m_activeContext.allocator);
symbolInfoArr.PushBack(symbolInfoPairArr, m_activeContext.allocator);
}
symbolInfoArr.PushBack(symbolInfoArr, m_activeContext.allocator);
symbolInfos.PushBack(symbolInfoArr, m_activeContext.allocator);
}
m_activeContext.doc.AddMember("symbolInfos", symbolInfos, m_activeContext.allocator);

Expand Down
12 changes: 5 additions & 7 deletions view/sharedcache/core/VM.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -453,16 +453,14 @@ int64_t MMappedFileAccessor::ReadLong(size_t address)
return ((int64_t*)(&(((uint8_t*)m_mmap._mmap)[address])))[0];
}

BinaryNinja::DataBuffer* MMappedFileAccessor::ReadBuffer(size_t address, size_t length)
BinaryNinja::DataBuffer MMappedFileAccessor::ReadBuffer(size_t address, size_t length)
{
if (address > m_mmap.len)
throw MappingReadException();
if (address + length > m_mmap.len)
throw MappingReadException();
void* data = (void*)(&(((uint8_t*)m_mmap._mmap)[address]));
void* dataCopy = malloc(length);
memcpy(dataCopy, data, length);
return new BinaryNinja::DataBuffer(dataCopy, length);
return BinaryNinja::DataBuffer(data, length);
}

void MMappedFileAccessor::Read(void* dest, size_t address, size_t length)
Expand Down Expand Up @@ -656,7 +654,7 @@ int64_t VM::ReadLong(size_t address)
return mapping.first.fileAccessor->lock()->ReadLong(mapping.second);
}

BinaryNinja::DataBuffer* VM::ReadBuffer(size_t addr, size_t length)
BinaryNinja::DataBuffer VM::ReadBuffer(size_t addr, size_t length)
{
auto mapping = MappingAtAddress(addr);
return mapping.first.fileAccessor->lock()->ReadBuffer(mapping.second, length);
Expand Down Expand Up @@ -767,14 +765,14 @@ size_t VMReader::ReadPointer()
return 0;
}

BinaryNinja::DataBuffer* VMReader::ReadBuffer(size_t length)
BinaryNinja::DataBuffer VMReader::ReadBuffer(size_t length)
{
auto mapping = m_vm->MappingAtAddress(m_cursor);
m_cursor += length;
return mapping.first.fileAccessor->lock()->ReadBuffer(mapping.second, length);
}

BinaryNinja::DataBuffer* VMReader::ReadBuffer(size_t addr, size_t length)
BinaryNinja::DataBuffer VMReader::ReadBuffer(size_t addr, size_t length)
{
auto mapping = m_vm->MappingAtAddress(addr);
m_cursor = addr + length;
Expand Down
8 changes: 4 additions & 4 deletions view/sharedcache/core/VM.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ class MMappedFileAccessor {

int64_t ReadLong(size_t address);

BinaryNinja::DataBuffer *ReadBuffer(size_t addr, size_t length);
BinaryNinja::DataBuffer ReadBuffer(size_t addr, size_t length);

void Read(void *dest, size_t addr, size_t length);
};
Expand Down Expand Up @@ -252,7 +252,7 @@ class VM {

int64_t ReadLong(size_t address);

BinaryNinja::DataBuffer *ReadBuffer(size_t addr, size_t length);
BinaryNinja::DataBuffer ReadBuffer(size_t addr, size_t length);

void Read(void *dest, size_t addr, size_t length);
};
Expand Down Expand Up @@ -320,9 +320,9 @@ class VMReader {

size_t ReadPointer(size_t address);

BinaryNinja::DataBuffer *ReadBuffer(size_t length);
BinaryNinja::DataBuffer ReadBuffer(size_t length);

BinaryNinja::DataBuffer *ReadBuffer(size_t addr, size_t length);
BinaryNinja::DataBuffer ReadBuffer(size_t addr, size_t length);

void Read(void *dest, size_t length);

Expand Down