Skip to content

Commit 714cefb

Browse files
committed
Add missing error handling to ReflectionContext
We found crashes deep in TypeRefBuilder that could traced back to a likely nullptr RemoteRef<> section address. It is very plausible that this is connected to a failed MemoryReader::getBytes() call, which can fail but isn't checked. This patch adds missing error checks to every call to readBytes(). rdar://74445486
1 parent cbdb032 commit 714cefb

File tree

1 file changed

+44
-22
lines changed

1 file changed

+44
-22
lines changed

include/swift/Reflection/ReflectionContext.h

Lines changed: 44 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,8 @@ class ReflectionContext
153153
auto CmdBuf = this->getReader().readBytes(
154154
RemoteAddress(CmdStartAddress.getAddressData() + Offset),
155155
SegmentCmdHdrSize);
156+
if (!CmdBuf)
157+
return false;
156158
auto CmdHdr = reinterpret_cast<typename T::SegmentCmd *>(CmdBuf.get());
157159
if (strncmp(CmdHdr->segname, "__TEXT", sizeof(CmdHdr->segname)) == 0) {
158160
Command = CmdHdr;
@@ -173,6 +175,8 @@ class ReflectionContext
173175
auto LoadCmdAddress = reinterpret_cast<const char *>(loadCmdOffset);
174176
auto LoadCmdBuf = this->getReader().readBytes(
175177
RemoteAddress(LoadCmdAddress), sizeof(typename T::SegmentCmd));
178+
if (!LoadCmdBuf)
179+
return false;
176180
auto LoadCmd = reinterpret_cast<typename T::SegmentCmd *>(LoadCmdBuf.get());
177181

178182
// The sections start immediately after the load command.
@@ -181,6 +185,8 @@ class ReflectionContext
181185
sizeof(typename T::SegmentCmd);
182186
auto Sections = this->getReader().readBytes(
183187
RemoteAddress(SectAddress), NumSect * sizeof(typename T::Section));
188+
if (!Sections)
189+
return false;
184190

185191
auto Slide = ImageStart.getAddressData() - Command->vmaddr;
186192
std::string Prefix = "__swift5";
@@ -211,6 +217,8 @@ class ReflectionContext
211217

212218
auto SectBuf = this->getReader().readBytes(RemoteAddress(RangeStart),
213219
RangeEnd - RangeStart);
220+
if (!SectBuf)
221+
return false;
214222

215223
auto findMachOSectionByName = [&](llvm::StringRef Name)
216224
-> std::pair<RemoteRef<void>, uint64_t> {
@@ -267,6 +275,8 @@ class ReflectionContext
267275
auto CmdBuf = this->getReader().readBytes(
268276
RemoteAddress(CmdStartAddress.getAddressData() + Offset),
269277
SegmentCmdHdrSize);
278+
if (!CmdBuf)
279+
return false;
270280
auto CmdHdr = reinterpret_cast<typename T::SegmentCmd *>(CmdBuf.get());
271281
if (strncmp(CmdHdr->segname, "__DATA", sizeof(CmdHdr->segname)) == 0) {
272282
auto DataSegmentEnd =
@@ -289,6 +299,8 @@ class ReflectionContext
289299
bool readPECOFFSections(RemoteAddress ImageStart) {
290300
auto DOSHdrBuf = this->getReader().readBytes(
291301
ImageStart, sizeof(llvm::object::dos_header));
302+
if (!DOSHdrBuf)
303+
return false;
292304
auto DOSHdr =
293305
reinterpret_cast<const llvm::object::dos_header *>(DOSHdrBuf.get());
294306
auto COFFFileHdrAddr = ImageStart.getAddressData() +
@@ -297,6 +309,8 @@ class ReflectionContext
297309

298310
auto COFFFileHdrBuf = this->getReader().readBytes(
299311
RemoteAddress(COFFFileHdrAddr), sizeof(llvm::object::coff_file_header));
312+
if (!COFFFileHdrBuf)
313+
return false;
300314
auto COFFFileHdr = reinterpret_cast<const llvm::object::coff_file_header *>(
301315
COFFFileHdrBuf.get());
302316

@@ -306,9 +320,11 @@ class ReflectionContext
306320
auto SectionTableBuf = this->getReader().readBytes(
307321
RemoteAddress(SectionTableAddr),
308322
sizeof(llvm::object::coff_section) * COFFFileHdr->NumberOfSections);
323+
if (!SectionTableBuf)
324+
return false;
309325

310-
auto findCOFFSectionByName = [&](llvm::StringRef Name)
311-
-> std::pair<RemoteRef<void>, uint64_t> {
326+
auto findCOFFSectionByName =
327+
[&](llvm::StringRef Name) -> std::pair<RemoteRef<void>, uint64_t> {
312328
for (size_t i = 0; i < COFFFileHdr->NumberOfSections; ++i) {
313329
const llvm::object::coff_section *COFFSec =
314330
reinterpret_cast<const llvm::object::coff_section *>(
@@ -323,6 +339,8 @@ class ReflectionContext
323339
auto Addr = ImageStart.getAddressData() + COFFSec->VirtualAddress;
324340
auto Buf = this->getReader().readBytes(RemoteAddress(Addr),
325341
COFFSec->VirtualSize);
342+
if (!Buf)
343+
return {nullptr, 0};
326344
auto BufStart = Buf.get();
327345
savedBuffers.push_back(std::move(Buf));
328346

@@ -508,6 +526,8 @@ class ReflectionContext
508526
} else {
509527
SecBuf = this->getReader().readBytes(SecStart, SecSize);
510528
}
529+
if (!SecBuf)
530+
return {nullptr, 0};
511531
auto SecContents =
512532
RemoteRef<void>(SecStart.getAddressData(), SecBuf.get());
513533
savedBuffers.push_back(std::move(SecBuf));
@@ -576,6 +596,8 @@ class ReflectionContext
576596
bool readELF(RemoteAddress ImageStart, llvm::Optional<llvm::sys::MemoryBlock> FileBuffer) {
577597
auto Buf =
578598
this->getReader().readBytes(ImageStart, sizeof(llvm::ELF::Elf64_Ehdr));
599+
if (!Buf)
600+
return false;
579601

580602
// Read the header.
581603
auto Hdr = reinterpret_cast<const llvm::ELF::Elf64_Ehdr *>(Buf.get());
@@ -887,10 +909,10 @@ class ReflectionContext
887909
return;
888910
auto NodeBytes = getReader().readBytes(RemoteAddress(NodePtr),
889911
sizeof(ConformanceNode<Runtime>));
912+
if (!NodeBytes)
913+
return;
890914
auto NodeData =
891915
reinterpret_cast<const ConformanceNode<Runtime> *>(NodeBytes.get());
892-
if (!NodeData)
893-
return;
894916
Call(NodeData->Type, NodeData->Proto);
895917
iterateConformanceTree(NodeData->Left, Call);
896918
iterateConformanceTree(NodeData->Right, Call);
@@ -901,21 +923,21 @@ class ReflectionContext
901923
std::function<void(StoredPointer Type, StoredPointer Proto)> Call) {
902924
auto MapBytes = getReader().readBytes(RemoteAddress(ConformancesPtr),
903925
sizeof(ConcurrentHashMap<Runtime>));
926+
if (!MapBytes)
927+
return;
904928
auto MapData =
905929
reinterpret_cast<const ConcurrentHashMap<Runtime> *>(MapBytes.get());
906-
if (!MapData)
907-
return;
908930

909931
auto Count = MapData->ElementCount;
910932
auto Size = Count * sizeof(ConformanceCacheEntry<Runtime>);
911933

912934
auto ElementsBytes =
913935
getReader().readBytes(RemoteAddress(MapData->Elements), Size);
936+
if (!ElementsBytes)
937+
return;
914938
auto ElementsData =
915939
reinterpret_cast<const ConformanceCacheEntry<Runtime> *>(
916940
ElementsBytes.get());
917-
if (!ElementsData)
918-
return;
919941

920942
for (StoredSize i = 0; i < Count; i++) {
921943
auto &Element = ElementsData[i];
@@ -983,10 +1005,10 @@ class ReflectionContext
9831005
auto AllocationBytes =
9841006
getReader().readBytes(RemoteAddress(Allocation.Ptr),
9851007
Allocation.Size);
1008+
if (!AllocationBytes)
1009+
return 0;
9861010
auto Entry = reinterpret_cast<const GenericMetadataCacheEntry *>(
9871011
AllocationBytes.get());
988-
if (!Entry)
989-
return 0;
9901012
return Entry->Value;
9911013
}
9921014
return 0;
@@ -1023,10 +1045,10 @@ class ReflectionContext
10231045
case GenericWitnessTableCacheTag: {
10241046
auto NodeBytes = getReader().readBytes(
10251047
RemoteAddress(Allocation.Ptr), sizeof(MetadataCacheNode<Runtime>));
1048+
if (!NodeBytes)
1049+
return llvm::None;
10261050
auto Node =
10271051
reinterpret_cast<const MetadataCacheNode<Runtime> *>(NodeBytes.get());
1028-
if (!Node)
1029-
return llvm::None;
10301052
return *Node;
10311053
}
10321054
default:
@@ -1079,23 +1101,23 @@ class ReflectionContext
10791101

10801102
auto PoolBytes = getReader()
10811103
.readBytes(AllocationPoolAddr->getResolvedAddress(), sizeof(PoolRange));
1082-
auto Pool = reinterpret_cast<const PoolRange *>(PoolBytes.get());
1083-
if (!Pool)
1104+
if (!PoolBytes)
10841105
return std::string("failure reading allocation pool contents");
1106+
auto Pool = reinterpret_cast<const PoolRange *>(PoolBytes.get());
10851107

10861108
auto TrailerPtr = Pool->Begin + Pool->Remaining;
10871109
while (TrailerPtr) {
10881110
auto TrailerBytes = getReader()
10891111
.readBytes(RemoteAddress(TrailerPtr), sizeof(PoolTrailer));
1090-
auto Trailer = reinterpret_cast<const PoolTrailer *>(TrailerBytes.get());
1091-
if (!Trailer)
1112+
if (!TrailerBytes)
10921113
break;
1114+
auto Trailer = reinterpret_cast<const PoolTrailer *>(TrailerBytes.get());
10931115
auto PoolStart = TrailerPtr - Trailer->PoolSize;
10941116
auto PoolBytes = getReader()
10951117
.readBytes(RemoteAddress(PoolStart), Trailer->PoolSize);
1096-
auto PoolPtr = (const char *)PoolBytes.get();
1097-
if (!PoolPtr)
1118+
if (!PoolBytes)
10981119
break;
1120+
auto PoolPtr = (const char *)PoolBytes.get();
10991121

11001122
uintptr_t Offset = 0;
11011123
while (Offset < Trailer->PoolSize) {
@@ -1137,10 +1159,7 @@ class ReflectionContext
11371159
auto HeaderBytes = getReader().readBytes(
11381160
RemoteAddress(BacktraceListNext),
11391161
sizeof(MetadataAllocationBacktraceHeader<Runtime>));
1140-
auto HeaderPtr =
1141-
reinterpret_cast<const MetadataAllocationBacktraceHeader<Runtime> *>(
1142-
HeaderBytes.get());
1143-
if (HeaderPtr == nullptr) {
1162+
if (!HeaderBytes) {
11441163
// FIXME: std::stringstream would be better, but LLVM's standard library
11451164
// introduces a vtable and we don't want that.
11461165
char result[128];
@@ -1149,6 +1168,9 @@ class ReflectionContext
11491168
BacktraceListNext.getAddressData());
11501169
return std::string(result);
11511170
}
1171+
auto HeaderPtr =
1172+
reinterpret_cast<const MetadataAllocationBacktraceHeader<Runtime> *>(
1173+
HeaderBytes.get());
11521174
auto BacktraceAddrPtr =
11531175
BacktraceListNext +
11541176
sizeof(MetadataAllocationBacktraceHeader<Runtime>);

0 commit comments

Comments
 (0)