1
0
Fork 0

DBG: fix signature verification for PDB v2.0 and improve symbol log

closes #2193
This commit is contained in:
Duncan Ogilvie 2019-08-09 20:55:59 +02:00
parent f57d69f91e
commit 60e1dff147
No known key found for this signature in database
GPG Key ID: FC89E0AAA0C1AAD8
3 changed files with 48 additions and 42 deletions

View File

@ -577,6 +577,7 @@ static void ReadDebugDirectory(MODINFO & Info, ULONG_PTR FileMapVA)
cv->Age);
Info.pdbFile = String((const char*)cv->PdbFileName, entry->SizeOfData - FIELD_OFFSET(CV_INFO_PDB70, PdbFileName));
memcpy(&Info.pdbValidation.guid, &cv->Signature, sizeof(GUID));
Info.pdbValidation.signature = 0;
Info.pdbValidation.age = cv->Age;
}

View File

@ -293,51 +293,52 @@ bool PDBDiaFile::open(const wchar_t* file, uint64_t loadAddress, DiaValidationDa
return false;
}
// NOTE: For some reason this never matches, commented for now.
// ^ 99% sure this should only be used for PDB v2.0 ('NB10' ones). v7.0 PDBs should be checked using (age+guid) only
/*
DWORD signature = 0;
hr = globalSym->get_signature(&signature);
if (!testError(hr) && validationData->signature != signature)
if(validationData->signature != 0)
{
close();
GuiSymbolLogAdd("PDB is not matching.\n");
return false;
}
*/
GUID guid = {0};
hr = globalSym->get_guid(&guid);
if(!testError(hr) && memcmp(&guid, &validationData->guid, sizeof(GUID)) != 0)
{
globalSym->Release();
close();
auto guidStr = [](const GUID & guid) -> String
// PDB v2.0 ('NB10' ones) do not have a GUID and they use a signature and age
DWORD signature = 0;
hr = globalSym->get_signature(&signature);
if(!testError(hr) && validationData->signature != signature)
{
// https://stackoverflow.com/a/22848342/1806760
char guid_string[37]; // 32 hex chars + 4 hyphens + null terminator
sprintf_s(guid_string,
"%08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x",
guid.Data1, guid.Data2, guid.Data3,
guid.Data4[0], guid.Data4[1], guid.Data4[2],
guid.Data4[3], guid.Data4[4], guid.Data4[5],
guid.Data4[6], guid.Data4[7]);
return guid_string;
};
globalSym->Release();
close();
GuiSymbolLogAdd(StringUtils::sprintf("Validation error: PDB guid is not matching (expected: %s, actual: %s).\n",
guidStr(validationData->guid).c_str(), guidStr(guid).c_str()).c_str());
return false;
GuiSymbolLogAdd(StringUtils::sprintf("Validation error: PDB signature is not matching (expected: %08X, actual: %08X).\n",
signature, validationData->signature).c_str());
return false;
}
}
else
{
// v7.0 PDBs should be checked using (age+guid) only
GUID guid = { 0 };
hr = globalSym->get_guid(&guid);
if(!testError(hr) && memcmp(&guid, &validationData->guid, sizeof(GUID)) != 0)
{
globalSym->Release();
close();
auto guidStr = [](const GUID & guid) -> String
{
// https://stackoverflow.com/a/22848342/1806760
char guid_string[37]; // 32 hex chars + 4 hyphens + null terminator
sprintf_s(guid_string,
"%08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x",
guid.Data1, guid.Data2, guid.Data3,
guid.Data4[0], guid.Data4[1], guid.Data4[2],
guid.Data4[3], guid.Data4[4], guid.Data4[5],
guid.Data4[6], guid.Data4[7]);
return guid_string;
};
GuiSymbolLogAdd(StringUtils::sprintf("Validation error: PDB guid is not matching (expected: %s, actual: %s).\n",
guidStr(validationData->guid).c_str(), guidStr(guid).c_str()).c_str());
return false;
}
}
}
globalSym->Release();
}
else
{
GuiSymbolLogAdd("Skipping PDB validation, expect invalid results!\n");
}
if(loadAddress != 0)
{

View File

@ -44,6 +44,10 @@ DWORD WINAPI SymbolSourceDIA::SourceLinesThread(void* parameter)
bool SymbolSourceDIA::loadPDB(const std::string & path, const std::string & modname, duint imageBase, duint imageSize, DiaValidationData_t* validationData)
{
if(!validationData)
{
GuiSymbolLogAdd(StringUtils::sprintf("[%p, %s] Skipping PDB validation, expect invalid results!\n", imageBase, modname.c_str()).c_str());
}
PDBDiaFile pdb; // Instance used for validation only.
_isOpen = pdb.open(path.c_str(), 0, validationData);
#if 1 // Async loading.
@ -148,9 +152,9 @@ bool SymbolSourceDIA::loadSymbolsAsync()
SymbolInfo symInfo;
symInfo.decoratedName = sym.name;
symInfo.undecoratedName = sym.undecoratedName;
symInfo.size = sym.size;
symInfo.size = (duint)sym.size;
symInfo.disp = sym.disp;
symInfo.rva = sym.virtualAddress;
symInfo.rva = (duint)sym.virtualAddress;
symInfo.publicSymbol = sym.publicSymbol;
// Check if we already have it inside, private symbols have priority over public symbols.
@ -158,7 +162,7 @@ bool SymbolSourceDIA::loadSymbolsAsync()
{
ScopedSpinLock lock(_lockSymbols);
auto it = _symAddrs.find(sym.virtualAddress);
auto it = _symAddrs.find((duint)sym.virtualAddress);
if(it != _symAddrs.end())
{
if(_symData[it->second].publicSymbol == true && symInfo.publicSymbol == false)
@ -170,7 +174,7 @@ bool SymbolSourceDIA::loadSymbolsAsync()
else
{
_symData.push_back(symInfo);
_symAddrs.insert({ sym.virtualAddress, _symData.size() - 1 });
_symAddrs.insert({ (duint)sym.virtualAddress, _symData.size() - 1 });
}
}