Skip to content

Commit c93685e

Browse files
committed
[SharedCache] Don't crash if we have a different metadata version
Also we now initialize m_cacheInfo when calling `DeserializeFromRawView` the responsibility is on the caller to handle
1 parent e3060d1 commit c93685e

File tree

2 files changed

+34
-41
lines changed

2 files changed

+34
-41
lines changed

view/sharedcache/core/SharedCache.cpp

Lines changed: 33 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -461,10 +461,11 @@ void SharedCache::SetMemoryRegionHeaderInitialized(std::lock_guard<std::mutex>&,
461461

462462
void SharedCache::PerformInitialLoad(std::lock_guard<std::mutex>& lock)
463463
{
464-
m_logger->LogInfo("Performing initial load of Shared Cache");
464+
std::unique_lock viewOperationsLock(m_viewSpecificState->viewOperationsThatInfluenceMetadataMutex);
465465
auto path = m_dscView->GetFile()->GetOriginalFilename();
466466
auto baseFile = MapFileWithoutApplyingSlide(path);
467467

468+
m_logger->LogInfo("Loading caches...");
468469
m_viewSpecificState->progress = LoadProgressLoadingCaches;
469470

470471
CacheInfo initialState;
@@ -962,6 +963,7 @@ void SharedCache::PerformInitialLoad(std::lock_guard<std::mutex>& lock)
962963
}
963964
baseFile.reset();
964965

966+
m_logger->LogInfo("Loading images...");
965967
m_viewSpecificState->progress = LoadProgressLoadingImages;
966968

967969
// We have set up enough metadata to map VM now.
@@ -1061,8 +1063,7 @@ void SharedCache::PerformInitialLoad(std::lock_guard<std::mutex>& lock)
10611063
m_modifiedState->viewState = DSCViewStateLoaded;
10621064
SaveCacheInfoToDSCView(lock);
10631065

1064-
m_logger->LogDebug("Finished initial load of Shared Cache");
1065-
1066+
m_logger->LogInfo("Finished loading...");
10661067
m_viewSpecificState->progress = LoadProgressFinished;
10671068
}
10681069

@@ -1092,49 +1093,38 @@ std::shared_ptr<VM> SharedCache::GetVMMap(const CacheInfo& cacheInfo)
10921093
}
10931094

10941095

1095-
void SharedCache::DeserializeFromRawView(std::lock_guard<std::mutex>& lock)
1096+
bool SharedCache::DeserializeFromRawView(std::lock_guard<std::mutex>& lock)
10961097
{
10971098
std::lock_guard cacheInfoLock(m_viewSpecificState->cacheInfoMutex);
1099+
1100+
m_cacheInfo = std::make_shared<CacheInfo>();
1101+
m_modifiedState = std::make_unique<ModifiedState>();
1102+
m_modifiedState->viewState = DSCViewStateUnloaded;
1103+
1104+
// First try and load from the view itself.
10981105
if (m_viewSpecificState->cacheInfo)
10991106
{
11001107
m_cacheInfo = m_viewSpecificState->cacheInfo;
1101-
m_modifiedState = std::make_unique<ModifiedState>();
1102-
m_metadataValid = true;
1103-
return;
1108+
m_modifiedState->viewState = DSCViewStateLoaded;
1109+
return true;
11041110
}
11051111

1112+
// Second try and load from the view metadata.
11061113
if (SharedCacheMetadata::ViewHasMetadata(m_dscView))
11071114
{
11081115
auto metadata = SharedCacheMetadata::LoadFromView(m_dscView);
11091116
if (!metadata)
1110-
{
1111-
m_metadataValid = false;
1112-
m_logger->LogError("Failed to deserialize Shared Cache metadata");
1113-
return;
1114-
}
1117+
return false;
11151118

11161119
m_viewSpecificState->viewState = metadata->state->viewState.value_or(DSCViewStateUnloaded);
1117-
m_viewSpecificState->state = std::move(*metadata->state);
1120+
m_viewSpecificState->state = static_cast<State>(std::move(*metadata->state));
11181121
m_viewSpecificState->cacheInfo = std::move(metadata->cacheInfo);
11191122

11201123
m_cacheInfo = m_viewSpecificState->cacheInfo;
1121-
m_modifiedState = std::make_unique<ModifiedState>();
1122-
m_metadataValid = true;
1123-
return;
1124+
m_modifiedState->viewState = DSCViewStateLoaded;
11241125
}
11251126

1126-
m_cacheInfo = nullptr;
1127-
m_modifiedState = std::make_unique<ModifiedState>();
1128-
m_modifiedState->viewState = DSCViewStateUnloaded;
1129-
m_metadataValid = true;
1130-
}
1131-
1132-
1133-
std::string to_hex_string(uint64_t value)
1134-
{
1135-
std::stringstream ss;
1136-
ss << std::hex << value;
1137-
return ss.str();
1127+
return true;
11381128
}
11391129

11401130

@@ -1461,17 +1451,20 @@ SharedCache::SharedCache(BinaryNinja::Ref<BinaryNinja::BinaryView> dscView) :
14611451

14621452
sharedCacheReferences++;
14631453
INIT_SHAREDCACHE_API_OBJECT()
1464-
DeserializeFromRawView(lock);
1465-
if (!m_metadataValid)
1466-
return;
1454+
if (!DeserializeFromRawView(lock))
1455+
{
1456+
// TODO: We need a way to prompt the user and ask if they want to continue (like BNDB version upgrades)
1457+
// TODO: To do that we really need to consolidate _where_ SharedCache is called.
1458+
// TODO: Specifically the **first** call to this must originate in DSCView, which is NOT the case currently.
1459+
m_logger->LogError("Metadata was invalid, recreating initial load of shared cache information...");
1460+
}
14671461

14681462
if (m_modifiedState->viewState.value_or(m_viewSpecificState->viewState) != DSCViewStateUnloaded)
14691463
{
14701464
m_viewSpecificState->progress = LoadProgressFinished;
14711465
return;
14721466
}
14731467

1474-
std::unique_lock viewOperationsLock(m_viewSpecificState->viewOperationsThatInfluenceMetadataMutex);
14751468
try {
14761469
PerformInitialLoad(lock);
14771470
}
@@ -1492,7 +1485,6 @@ SharedCache::SharedCache(BinaryNinja::Ref<BinaryNinja::BinaryView> dscView) :
14921485
{
14931486
if (header.installName.find("libsystem_c.dylib") != std::string::npos)
14941487
{
1495-
viewOperationsLock.unlock();
14961488
m_logger->LogInfo("Loading core libsystem_c.dylib library");
14971489
LoadImageWithInstallName(lock, header.installName, false);
14981490
break;
@@ -3071,10 +3063,14 @@ bool SharedCache::SaveCacheInfoToDSCView(std::lock_guard<std::mutex>&)
30713063

30723064
{
30733065
std::lock_guard lock(m_viewSpecificState->cacheInfoMutex);
3074-
if (m_cacheInfo && !m_viewSpecificState->cacheInfo)
3075-
m_viewSpecificState->cacheInfo = m_cacheInfo;
3076-
else if (m_cacheInfo != m_viewSpecificState->cacheInfo)
3077-
abort();
3066+
if (m_cacheInfo)
3067+
{
3068+
if (!m_viewSpecificState->cacheInfo)
3069+
m_viewSpecificState->cacheInfo = m_cacheInfo;
3070+
3071+
// At this point we expect cache info and view state cache to be the same.
3072+
assert(m_viewSpecificState->cacheInfo == m_cacheInfo);
3073+
}
30783074
}
30793075

30803076
m_metadataValid = true;
@@ -3364,10 +3360,7 @@ std::optional<SharedCache::CacheInfo> SharedCache::CacheInfo::Load(Deserializati
33643360
}
33653361

33663362
if (context.doc["metadataVersion"].GetUint() != METADATA_VERSION)
3367-
{
3368-
LogError("Shared Cache metadata version mismatch");
33693363
return std::nullopt;
3370-
}
33713364

33723365
CacheInfo cacheInfo;
33733366
cacheInfo.MSL(backingCaches);

view/sharedcache/core/SharedCache.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -585,7 +585,7 @@ namespace SharedCacheCore {
585585

586586
private:
587587
void PerformInitialLoad(std::lock_guard<std::mutex>&);
588-
void DeserializeFromRawView(std::lock_guard<std::mutex>&);
588+
bool DeserializeFromRawView(std::lock_guard<std::mutex>&);
589589

590590
public:
591591
std::shared_ptr<VM> GetVMMap();

0 commit comments

Comments
 (0)