[lldb] Fix a use-after-free in SymbolFileCTF #151586
Conversation
This fixes a use-after-free in SymbolFileCTF. Previously, we would remove the underlying CTF type as soon as we resolved it. However, it's possible that we're still holding onto the CTF type while we're parsing a dependent type, like a modifier, resulting in a use-after-free. This patch addresses the issue by delaying the removal of the CTF type until the type is fully resolved. I have a XNU kernel binary that reproduces the issue and confirmed that this solves the memory issue using ASan. However I haven't been able to craft types by hand that reproduce this issue for a test case. rdar://156660866
|
@llvm/pr-subscribers-lldb Author: Jonas Devlieghere (JDevlieghere) ChangesThis fixes a use-after-free in SymbolFileCTF. Previously, we would remove the underlying CTF type as soon as we resolved it. However, it's possible that we're still holding onto the CTF type while we're parsing a dependent type, like a modifier, resulting in a use-after-free. This patch addresses the issue by delaying the removal of the CTF type until the type is fully resolved. I have a XNU kernel binary that reproduces the issue and confirmed that this solves the memory issue using ASan. However I haven't been able to craft types by hand that reproduce this issue for a test case. rdar://156660866 Full diff: https://github.com/llvm/llvm-project/pull/151586.diff 1 Files Affected:
diff --git a/lldb/source/Plugins/SymbolFile/CTF/SymbolFileCTF.cpp b/lldb/source/Plugins/SymbolFile/CTF/SymbolFileCTF.cpp
index 81c6731cafcd1..591fdede70c26 100644
--- a/lldb/source/Plugins/SymbolFile/CTF/SymbolFileCTF.cpp
+++ b/lldb/source/Plugins/SymbolFile/CTF/SymbolFileCTF.cpp
@@ -738,9 +738,29 @@ size_t SymbolFileCTF::ParseTypes(CompileUnit &cu) {
LLDB_LOG(log, "Parsed {0} CTF types", m_ctf_types.size());
- for (lldb::user_id_t uid = 1; uid < type_uid; ++uid)
+ for (lldb::user_id_t uid = 1; uid < type_uid; ++uid) {
ResolveTypeUID(uid);
+ // Remove the CTF type because we don't need it anymore, except for record
+ // types which we may need to complete later.
+ auto ctf_type_it = m_ctf_types.find(uid);
+ if (ctf_type_it != m_ctf_types.end()) {
+ CTFType *ctf_type = ctf_type_it->second.get();
+ if (!llvm::isa<CTFRecord>(ctf_type))
+ m_ctf_types.erase(uid);
+ }
+ }
+
+#ifndef NDEBUG
+ // Verify that the only CTF types left at this point are record types.
+ for (auto &t : m_ctf_types) {
+ CTFType *ctf_type = t.second.get();
+ assert(ctf_type && "invalid type in m_ctf_types");
+ assert(llvm::isa<CTFRecord>(ctf_type) && "leaking non record type");
+ }
+
+#endif
+
LLDB_LOG(log, "Created {0} CTF types", m_types.size());
return m_types.size();
@@ -994,6 +1014,8 @@ lldb_private::Type *SymbolFileCTF::ResolveTypeUID(lldb::user_id_t type_uid) {
CTFType *ctf_type = ctf_type_it->second.get();
assert(ctf_type && "m_ctf_types should only contain valid CTF types");
+ assert(ctf_type->uid == type_uid &&
+ "CTF type UID doesn't match UID in m_ctf_types");
Log *log = GetLog(LLDBLog::Symbols);
@@ -1015,11 +1037,6 @@ lldb_private::Type *SymbolFileCTF::ResolveTypeUID(lldb::user_id_t type_uid) {
m_types[type_uid] = type_sp;
- // Except for record types which we'll need to complete later, we don't need
- // the CTF type anymore.
- if (!isa<CTFRecord>(ctf_type))
- m_ctf_types.erase(type_uid);
-
return type_sp.get();
}
|
bulbazord
left a comment
bulbazord
left a comment
There was a problem hiding this comment.
I think I'm missing something. Instead of removing it at the end of ResolveTypeUID, you remove it right after the call to ResolveTypeUID. What's the difference?
I should've mentioned that this function is (indirectly) recursive, because that's totally fine (e.g. a struct having a pointer to itself). Here's the stack trace from ASan that illustrates this. In the process of resolving the type, we have to create another type, which includes a type modifier of the original type. |
3 participants
This fixes a use-after-free in SymbolFileCTF. Previously, we would remove the underlying CTF type as soon as we resolved it. However, it's possible that we're still holding onto the CTF type while we're parsing a dependent type, like a modifier, resulting in a use-after-free. This patch addresses the issue by delaying the removal of the CTF type until the type is fully resolved.
I have a XNU kernel binary that reproduces the issue and confirmed that this solves the memory issue using ASan. However I haven't been able to craft types by hand that reproduce this issue for a test case.
rdar://156660866