Skip to content

Commit b95f3c1

Browse files
zznopzznop
committed
[EFI Resolver] Stabilize workflow propagation and annotations
Run automatic resolution as persisted workflow continuations, completing analysis between dependent phases. Defer entry-point and callback type propagation until IL is refreshed, and accumulate pending callee signature edits across asynchronous updates. Correct interface pointer depth and require exact SSA provenance for PEI service recovery and output annotations. Preserve containing structures, reject ambiguous HLIL call mappings, and track visited wrapper states with cancellation checks. Make generated names deterministic and stable across reruns, using GUIDs to name interfaces when protocol types are unavailable. Fix GUID extraction, pointer reads, notify annotation validation, and traversal past unresolved entries in variadic protocol lists. Apply automatic annotations in short main-thread undo scopes while keeping inspection on the workflow thread. Preserve dirty state and user edits while discarding automatic undo records.
1 parent 828474f commit b95f3c1

11 files changed

Lines changed: 552 additions & 396 deletions

File tree

‎plugins/efi_resolver/include/AnalysisUpdates.h‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
#pragma once
2+
3+
#include "binaryninjaapi.h"
4+
#include <functional>
5+
6+
// Apply discovered annotations before the resolver continues inspecting the view.
7+
// Automatic analysis uses short main-thread undo scopes; manual commands retain
8+
// their normal mutation and undo behavior.
9+
class AnalysisUpdates
10+
{
11+
BinaryNinja::Ref<BinaryNinja::BinaryView> m_view;
12+
bool m_automatic;
13+
14+
public:
15+
AnalysisUpdates(BinaryNinja::BinaryView* view, bool automatic) : m_view(view), m_automatic(automatic) {}
16+
17+
// Only mutations belong in this callback. Compute types, names, and targets on
18+
// the calling thread, and never wait for analysis inside it. This is synchronous
19+
// so subsequent resolver reads see these writes and reference captures are safe.
20+
void Apply(const std::function<void()>& update) const;
21+
void CreateUserVariable(BinaryNinja::Ref<BinaryNinja::Function> func, const BinaryNinja::Variable& variable,
22+
const BinaryNinja::Confidence<BinaryNinja::Ref<BinaryNinja::Type>>& type, const std::string& name) const;
23+
};

‎plugins/efi_resolver/include/DxeResolver.h‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,20 +10,16 @@ class DxeResolver : public Resolver
1010
bool resolveProtocolInterfaces(Ref<Function> func, uint64_t addr, size_t guidParam, const vector<size_t>& interfaceParams);
1111
bool resolveProtocolInterfaceList(Ref<Function> func, uint64_t addr, size_t firstGuidParam);
1212

13+
public:
14+
// Individual phases require completed analysis between calls.
1315
bool resolveSmmTables(string serviceName, string tableName);
1416
bool resolveSmmServices();
1517
bool resolveSmiHandlers();
1618

17-
public:
1819
/*!
1920
resolve BootServices and RuntimeServices, define protocol types that loaded by BootServices
2021
*/
2122
bool resolveDxe();
2223

23-
/*!
24-
Define MMST/SMMST and resolve SMM related protocols
25-
*/
26-
bool resolveSmm();
27-
28-
DxeResolver(Ref<BinaryView> view, Ref<BackgroundTask> task);
24+
DxeResolver(Ref<BinaryView> view, Ref<BackgroundTask> task, TypePropagation& propagation);
2925
};

‎plugins/efi_resolver/include/PeiResolver.h‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,12 @@ class PeiResolver : public Resolver
77
bool resolvePeiIdt();
88
bool resolvePeiMrc();
99
bool resolvePeiMrs();
10+
public:
11+
// Individual phases require completed analysis between calls.
1012
bool resolvePlatformPointers();
13+
bool resolveServicePointers();
1114
bool resolvePeiDescriptors();
1215
bool resolvePeiServices();
1316

14-
public:
15-
/*!
16-
resolve Pei related types and PPIs, this function will also resolve processor-specific pointers
17-
and tried to define the EFI_PEI_DESCRIPTORS
18-
*/
19-
bool resolvePei();
20-
PeiResolver(Ref<BinaryView> view, Ref<BackgroundTask> task);
17+
PeiResolver(Ref<BinaryView> view, Ref<BackgroundTask> task, TypePropagation& propagation);
2118
};

‎plugins/efi_resolver/include/Resolver.h‎

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,22 @@ class Resolver
2525
string protocolName;
2626
string guidName;
2727
};
28+
struct GuidInterfaceCallsite
29+
{
30+
Ref<Function> func;
31+
uint64_t addr;
32+
int guidPos;
33+
int interfacePos;
34+
};
2835

2936
Ref<BinaryView> m_view;
3037
Ref<BackgroundTask> m_task;
38+
TypePropagation& m_propagation;
39+
const AnalysisUpdates& m_updates;
3140
size_t m_width;
3241
map<EFI_GUID, pair<string, string>> m_protocol;
3342
map<EFI_GUID, string> m_user_guids;
43+
map<Ref<Function>, map<Variable, string>> m_localNames;
3444

3545
vector<pair<uint64_t, string>> m_service_usages;
3646
vector<pair<uint64_t, string>> m_protocol_usages;
@@ -50,25 +60,28 @@ class Resolver
5060
*/
5161
Ref<Type> GetTypeFromViewAndPlatform(string type_name);
5262
optional<uint64_t> GetConstantDataAddress(const HighLevelILInstruction& expr);
53-
vector<HighLevelILInstruction> GetCallExprs(const vector<HighLevelILInstruction>& exprs, uint64_t addr);
54-
ProtocolGuidInfo resolveProtocolGuid(const EFI_GUID& guid, uint64_t addr);
63+
static vector<HighLevelILInstruction> GetCallExprs(const vector<HighLevelILInstruction>& exprs, uint64_t addr);
64+
ProtocolGuidInfo resolveProtocolGuid(const EFI_GUID& guid, uint64_t addr, optional<uint64_t> guidDataAddr);
5565
bool defineGuidDataVariable(uint64_t addr, const string& guidName);
5666
bool applyProtocolInterface(Ref<Function> func, const HighLevelILInstruction& interfaceParam,
5767
const ProtocolGuidInfo& info, bool outputInterface);
5868
void initProtocolMapping();
69+
bool resolveGuidInterfaceAtCallsite(const GuidInterfaceCallsite& callsite,
70+
vector<GuidInterfaceCallsite>& pending);
5971

6072
public:
73+
const AnalysisUpdates& GetUpdates() const { return m_updates; }
6174
bool setModuleEntry(EFIModuleType fileType);
6275
bool propagateEntryTypes();
6376
bool resolveGuidInterface(Ref<Function> func, uint64_t addr, int guid_pos, int interface_pos);
6477
bool defineOutputAtCallsite(Ref<Function> func, uint64_t addr, int paramIdx, string typeName, string name);
65-
Resolver(Ref<BinaryView> view, Ref<BackgroundTask> task);
78+
Resolver(Ref<BinaryView> view, Ref<BackgroundTask> task, TypePropagation& propagation);
6679

6780
pair<string, string> lookupGuid(EFI_GUID guidBytes);
6881
pair<string, string> defineAndLookupGuid(uint64_t addr);
6982

70-
string nonConflictingName(const string& basename);
71-
static string nonConflictingLocalName(Ref<Function> func, const string& basename);
83+
string nonConflictingName(const string& basename, optional<uint64_t>>nullopt);
84+
string nonConflictingLocalName(Ref<Function> func, const Variable& target, const string& basename);
7285

7386
/*!
7487
Define the structure used at the callsite with type \c typeName, propagate it to the data section. If it's a

‎plugins/efi_resolver/include/TypePropagation.h‎

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,30 @@
11
#pragma once
22

33
#include "Utils.h"
4+
#include "AnalysisUpdates.h"
45
#include "binaryninjaapi.h"
56

67
using namespace BinaryNinja;
78

89
class TypePropagation
910
{
1011
Ref<BinaryView> m_view;
11-
std::deque<uint64_t> m_queue;
12-
Ref<Platform> m_platform;
12+
AnalysisUpdates m_updates;
13+
using FunctionKey = std::pair<std::string, uint64_t>;
14+
using PendingFunctionTypes = std::map<FunctionKey, Ref<Type>>;
15+
std::deque<FunctionKey> m_queue;
16+
std::set<FunctionKey> m_processed;
1317

14-
public:
15-
TypePropagation(BinaryView* view);
1618
bool propagateFuncParamTypes(Function* func);
17-
bool propagateFuncParamTypes(Function* func, SSAVariable ssa_var);
19+
bool propagateFuncParamTypes(Function* func, SSAVariable ssa_var, PendingFunctionTypes& pendingTypes);
20+
21+
public:
22+
TypePropagation(BinaryView* view, bool automatic = false);
23+
const AnalysisUpdates& GetUpdates() const { return m_updates; }
24+
void QueueFunction(Function* func);
25+
bool HasPendingFunctions() const { return !m_queue.empty(); }
26+
// The caller must complete analysis before each call, including for newly queued roots.
27+
void ProcessNextFunction();
28+
Ref<Metadata> SaveState() const;
29+
void RestoreState(Ref<Metadata> metadata);
1830
};

‎plugins/efi_resolver/include/Utils.h‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,35 @@
11
#pragma once
22

33
#include "binaryninjaapi.h"
4+
#include <algorithm>
5+
#include <tuple>
46

57
using namespace BinaryNinja;
68

9+
static inline auto GetFunctionSemanticKey(Ref<Function> func)
10+
{
11+
return std::make_tuple(func ? func->GetStart() : 0,
12+
func && func->GetPlatform() ? func->GetPlatform()->GetName() : std::string());
13+
}
14+
15+
static inline void SortCodeReferences(std::vector<ReferenceSource>& refs)
16+
{
17+
// Core reference ordering includes object pointers. Resolve in semantic order before allocating names or
18+
// applying annotations, including when references from multiple service types have been merged.
19+
auto key = [](const ReferenceSource& ref) {
20+
return std::tuple_cat(GetFunctionSemanticKey(ref.func),
21+
std::make_tuple(ref.arch ? ref.arch->GetName() : std::string(), ref.addr));
22+
};
23+
std::sort(refs.begin(), refs.end(), [&](const auto& left, const auto& right) { return key(left) < key(right); });
24+
}
25+
26+
static inline void SortAnalysisFunctions(std::vector<Ref<Function>>& funcs)
27+
{
28+
std::sort(funcs.begin(), funcs.end(), [](const auto& left, const auto& right) {
29+
return GetFunctionSemanticKey(left) < GetFunctionSemanticKey(right);
30+
});
31+
}
32+
733
static inline std::string GetOriginalTypeName(Ref<Type> type)
834
{
935
std::string result;

‎plugins/efi_resolver/src/AnalysisUpdates.cpp‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
#include "AnalysisUpdates.h"
2+
#include <exception>
3+
4+
using namespace BinaryNinja;
5+
6+
void AnalysisUpdates::Apply(const std::function<void()>& update) const
7+
{
8+
if (!m_automatic)
9+
{
10+
update();
11+
return;
12+
}
13+
14+
std::exception_ptr error;
15+
ExecuteOnMainThreadAndWait([&]() {
16+
try
17+
{
18+
// Undo scopes are file-wide. Open and close this scope within a single
19+
// UI callback so UI edits cannot enter it between resolver writes.
20+
struct UndoScope
21+
{
22+
Ref<BinaryView> view;
23+
std::string id;
24+
~UndoScope() { view->ForgetUndoActions(id); }
25+
} undoScope {m_view, m_view->BeginUndoActions(false)};
26+
update();
27+
}
28+
catch (...)
29+
{
30+
// Report errors on the workflow thread after restoring the dirty flags.
31+
error = std::current_exception();
32+
}
33+
});
34+
if (error)
35+
std::rethrow_exception(error);
36+
}
37+
38+
void AnalysisUpdates::CreateUserVariable(Ref<Function> func, const Variable& variable,
39+
const Confidence<Ref<Type>>& type, const std::string& name) const
40+
{
41+
Apply([&]() { func->CreateUserVariable(variable, type, name); });
42+
}

‎plugins/efi_resolver/src/DxeResolver.cpp‎

Lines changed: 23 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ bool DxeResolver::resolveProtocolGuid(Ref<Function> func, uint64_t addr, size_t
1818
if (m_view->Read(&guid, *guidDataAddr, 16) < 16)
1919
continue;
2020

21-
auto info = Resolver::resolveProtocolGuid(guid, addr);
21+
auto info = Resolver::resolveProtocolGuid(guid, addr, guidDataAddr);
2222
if (defineGuidDataVariable(*guidDataAddr, info.guidName))
2323
changed = true;
2424
}
@@ -46,7 +46,7 @@ bool DxeResolver::resolveProtocolInterfaces(
4646
if (m_view->Read(&guid, *guidDataAddr, 16) < 16)
4747
continue;
4848

49-
auto info = Resolver::resolveProtocolGuid(guid, addr);
49+
auto info = Resolver::resolveProtocolGuid(guid, addr, guidDataAddr);
5050
if (defineGuidDataVariable(*guidDataAddr, info.guidName))
5151
changed = true;
5252

@@ -76,14 +76,17 @@ bool DxeResolver::resolveProtocolInterfaceList(Ref<Function> func, uint64_t addr
7676
for (size_t guidParam = firstGuidParam; guidParam + 1 < params.size(); guidParam += 2)
7777
{
7878
auto guidDataAddr = GetConstantDataAddress(params[guidParam]);
79-
if (!guidDataAddr || *guidDataAddr == 0)
79+
if (!guidDataAddr)
80+
continue;
81+
// Only a proven null GUID terminates the list; unresolved pairs can precede known ones.
82+
if (*guidDataAddr == 0)
8083
break;
8184

8285
EFI_GUID guid;
8386
if (m_view->Read(&guid, *guidDataAddr, 16) < 16)
8487
continue;
8588

86-
auto info = Resolver::resolveProtocolGuid(guid, addr);
89+
auto info = Resolver::resolveProtocolGuid(guid, addr, guidDataAddr);
8790
if (!defineGuidDataVariable(*guidDataAddr, info.guidName))
8891
continue;
8992
changed = true;
@@ -103,6 +106,7 @@ bool DxeResolver::resolveBootServices()
103106
auto refs = m_view->GetCodeReferencesForType(QualifiedName("EFI_BOOT_SERVICES"));
104107
// search reference of `EFI_BOOT_SERVICES` so that we can easily parse different services
105108

109+
SortCodeReferences(refs);
106110
for (auto& ref : refs)
107111
{
108112
if (IsCancelled())
@@ -188,6 +192,7 @@ bool DxeResolver::resolveRuntimeServices()
188192
SetProgressText("Resolving Runtime Services...");
189193
auto refs = m_view->GetCodeReferencesForType(QualifiedName("EFI_RUNTIME_SERVICES"));
190194

195+
SortCodeReferences(refs);
191196
for (auto& ref : refs)
192197
{
193198
if (IsCancelled())
@@ -230,6 +235,7 @@ bool DxeResolver::resolveSmmTables(string serviceName, string tableName)
230235
SetProgressText("Defining MM tables...");
231236
auto refs = m_view->GetCodeReferencesForType(QualifiedName(serviceName));
232237
// both versions use the same type, so we only need to search for this one
238+
SortCodeReferences(refs);
233239
for (auto& ref : refs)
234240
{
235241
if (IsCancelled())
@@ -274,8 +280,11 @@ bool DxeResolver::resolveSmmTables(string serviceName, string tableName)
274280
bool ok = m_view->ParseTypeString(tableName, result, errors);
275281
if (!ok)
276282
return false;
277-
m_view->DefineDataVariable(smstAddr.GetValue().value, result.type);
278-
m_view->DefineUserSymbol(new Symbol(DataSymbol, "gMmst", smstAddr.GetValue().value));
283+
auto address = smstAddr.GetValue().value;
284+
m_updates.Apply([&]() {
285+
m_view->DefineDataVariable(address, result.type);
286+
m_view->DefineUserSymbol(new Symbol(DataSymbol, "gMmst", address));
287+
});
279288
m_view->UpdateAnalysis();
280289
}
281290
return true;
@@ -289,6 +298,7 @@ bool DxeResolver::resolveSmmServices()
289298
// These tables have same type information, we can just iterate once
290299
refs.insert(refs.end(), refs_smm.begin(), refs_smm.end());
291300

301+
SortCodeReferences(refs);
292302
for (auto& ref : refs)
293303
{
294304
if (IsCancelled())
@@ -345,6 +355,7 @@ bool DxeResolver::resolveSmiHandlers()
345355
refs.insert(refs.end(), refs_smm_sx.begin(), refs_smm_sx.end());
346356
refs.insert(refs.end(), refs_mm_sx.begin(), refs_mm_sx.end());
347357

358+
SortCodeReferences(refs);
348359
for (auto& ref : refs)
349360
{
350361
if (IsCancelled())
@@ -409,13 +420,14 @@ bool DxeResolver::resolveSmiHandlers()
409420
bool ok = m_view->ParseTypeString(handleTypeStr, result, errors);
410421
if (!ok)
411422
return false;
412-
targetFunc->SetUserType(result.type);
413-
m_view->DefineUserSymbol(new Symbol(FunctionSymbol, funcName, funcAddr));
423+
m_updates.Apply([&]() {
424+
targetFunc->SetUserType(result.type);
425+
m_view->DefineUserSymbol(new Symbol(FunctionSymbol, funcName, funcAddr));
426+
});
414427
m_view->UpdateAnalysis();
415428

416429
// After setting the type, we want to propagate the parameters' type
417-
TypePropagation propagator(m_view);
418-
propagator.propagateFuncParamTypes(targetFunc);
430+
m_propagation.QueueFunction(targetFunc);
419431
}
420432
}
421433
}
@@ -431,20 +443,7 @@ bool DxeResolver::resolveDxe()
431443
return true;
432444
}
433445

434-
bool DxeResolver::resolveSmm()
435-
{
436-
if (!resolveSmmTables("EFI_SMM_GET_SMST_LOCATION2", "EFI_SMM_SYSTEM_TABLE2*"))
437-
return false;
438-
if (!resolveSmmTables("EFI_MM_GET_MMST_LOCATION", "EFI_MM_SYSTEM_TABLE*"))
439-
return false;
440-
if (!resolveSmmServices())
441-
return false;
442-
if (!resolveSmiHandlers())
443-
return false;
444-
return true;
445-
}
446-
447-
DxeResolver::DxeResolver(Ref<BinaryView> view, Ref<BackgroundTask> task) : Resolver(view, task)
446+
DxeResolver::DxeResolver(Ref<BinaryView> view, Ref<BackgroundTask> task, TypePropagation& propagation) : Resolver(view, task, propagation)
448447
{
449448
initProtocolMapping();
450449
}

0 commit comments

Comments
 (0)