Skip to content

Commit 38148be

Browse files
adrian-prantladrian-prantl
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 (cherry picked from commit 714cefb)
1 parent 44ed14e commit 38148be

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
@@ -169,6 +169,8 @@ class ReflectionContext
169169
auto CmdBuf = this->getReader().readBytes(
170170
RemoteAddress(CmdStartAddress.getAddressData() + Offset),
171171
SegmentCmdHdrSize);
172+
if (!CmdBuf)
173+
return false;
172174
auto CmdHdr = reinterpret_cast<typename T::SegmentCmd *>(CmdBuf.get());
173175
if (strncmp(CmdHdr->segname, "__TEXT", sizeof(CmdHdr->segname)) == 0) {
174176
Command = CmdHdr;
@@ -189,6 +191,8 @@ class ReflectionContext
189191
auto LoadCmdAddress = reinterpret_cast<const char *>(loadCmdOffset);
190192
auto LoadCmdBuf = this->getReader().readBytes(
191193
RemoteAddress(LoadCmdAddress), sizeof(typename T::SegmentCmd));
194+
if (!LoadCmdBuf)
195+
return false;
192196
auto LoadCmd = reinterpret_cast<typename T::SegmentCmd *>(LoadCmdBuf.get());
193197

194198
// The sections start immediately after the load command.
@@ -197,6 +201,8 @@ class ReflectionContext
197201
sizeof(typename T::SegmentCmd);
198202
auto Sections = this->getReader().readBytes(
199203
RemoteAddress(SectAddress), NumSect * sizeof(typename T::Section));
204+
if (!Sections)
205+
return false;
200206

201207
auto Slide = ImageStart.getAddressData() - Command->vmaddr;
202208
std::string Prefix = "__swift5";
@@ -227,6 +233,8 @@ class ReflectionContext
227233

228234
auto SectBuf = this->getReader().readBytes(RemoteAddress(RangeStart),
229235
RangeEnd - RangeStart);
236+
if (!SectBuf)
237+
return false;
230238

231239
auto findMachOSectionByName = [&](llvm::StringRef Name)
232240
-> std::pair<RemoteRef<void>, uint64_t> {
@@ -283,6 +291,8 @@ class ReflectionContext
283291
auto CmdBuf = this->getReader().readBytes(
284292
RemoteAddress(CmdStartAddress.getAddressData() + Offset),
285293
SegmentCmdHdrSize);
294+
if (!CmdBuf)
295+
return false;
286296
auto CmdHdr = reinterpret_cast<typename T::SegmentCmd *>(CmdBuf.get());
287297
if (strncmp(CmdHdr->segname, "__DATA", sizeof(CmdHdr->segname)) == 0) {
288298
auto DataSegmentEnd =
@@ -305,6 +315,8 @@ class ReflectionContext
305315
bool readPECOFFSections(RemoteAddress ImageStart) {
306316
auto DOSHdrBuf = this->getReader().readBytes(
307317
ImageStart, sizeof(llvm::object::dos_header));
318+
if (!DOSHdrBuf)
319+
return false;
308320
auto DOSHdr =
309321
reinterpret_cast<const llvm::object::dos_header *>(DOSHdrBuf.get());
310322
auto COFFFileHdrAddr = ImageStart.getAddressData() +
@@ -313,6 +325,8 @@ class ReflectionContext
313325

314326
auto COFFFileHdrBuf = this->getReader().readBytes(
315327
RemoteAddress(COFFFileHdrAddr), sizeof(llvm::object::coff_file_header));
328+
if (!COFFFileHdrBuf)
329+
return false;
316330
auto COFFFileHdr = reinterpret_cast<const llvm::object::coff_file_header *>(
317331
COFFFileHdrBuf.get());
318332

@@ -322,9 +336,11 @@ class ReflectionContext
322336
auto SectionTableBuf = this->getReader().readBytes(
323337
RemoteAddress(SectionTableAddr),
324338
sizeof(llvm::object::coff_section) * COFFFileHdr->NumberOfSections);
339+
if (!SectionTableBuf)
340+
return false;
325341

326-
auto findCOFFSectionByName = [&](llvm::StringRef Name)
327-
-> std::pair<RemoteRef<void>, uint64_t> {
342+
auto findCOFFSectionByName =
343+
[&](llvm::StringRef Name) -> std::pair<RemoteRef<void>, uint64_t> {
328344
for (size_t i = 0; i < COFFFileHdr->NumberOfSections; ++i) {
329345
const llvm::object::coff_section *COFFSec =
330346
reinterpret_cast<const llvm::object::coff_section *>(
@@ -339,6 +355,8 @@ class ReflectionContext
339355
auto Addr = ImageStart.getAddressData() + COFFSec->VirtualAddress;
340356
auto Buf = this->getReader().readBytes(RemoteAddress(Addr),
341357
COFFSec->VirtualSize);
358+
if (!Buf)
359+
return {nullptr, 0};
342360
auto BufStart = Buf.get();
343361
savedBuffers.push_back(std::move(Buf));
344362

@@ -524,6 +542,8 @@ class ReflectionContext
524542
} else {
525543
SecBuf = this->getReader().readBytes(SecStart, SecSize);
526544
}
545+
if (!SecBuf)
546+
return {nullptr, 0};
527547
auto SecContents =
528548
RemoteRef<void>(SecStart.getAddressData(), SecBuf.get());
529549
savedBuffers.push_back(std::move(SecBuf));
@@ -592,6 +612,8 @@ class ReflectionContext
592612
bool readELF(RemoteAddress ImageStart, llvm::Optional<llvm::sys::MemoryBlock> FileBuffer) {
593613
auto Buf =
594614
this->getReader().readBytes(ImageStart, sizeof(llvm::ELF::Elf64_Ehdr));
615+
if (!Buf)
616+
return false;
595617

596618
// Read the header.
597619
auto Hdr = reinterpret_cast<const llvm::ELF::Elf64_Ehdr *>(Buf.get());
@@ -903,10 +925,10 @@ class ReflectionContext
903925
return;
904926
auto NodeBytes = getReader().readBytes(RemoteAddress(NodePtr),
905927
sizeof(ConformanceNode<Runtime>));
928+
if (!NodeBytes)
929+
return;
906930
auto NodeData =
907931
reinterpret_cast<const ConformanceNode<Runtime> *>(NodeBytes.get());
908-
if (!NodeData)
909-
return;
910932
Call(NodeData->Type, NodeData->Proto);
911933
iterateConformanceTree(NodeData->Left, Call);
912934
iterateConformanceTree(NodeData->Right, Call);
@@ -917,21 +939,21 @@ class ReflectionContext
917939
std::function<void(StoredPointer Type, StoredPointer Proto)> Call) {
918940
auto MapBytes = getReader().readBytes(RemoteAddress(ConformancesPtr),
919941
sizeof(ConcurrentHashMap<Runtime>));
942+
if (!MapBytes)
943+
return;
920944
auto MapData =
921945
reinterpret_cast<const ConcurrentHashMap<Runtime> *>(MapBytes.get());
922-
if (!MapData)
923-
return;
924946

925947
auto Count = MapData->ElementCount;
926948
auto Size = Count * sizeof(ConformanceCacheEntry<Runtime>);
927949

928950
auto ElementsBytes =
929951
getReader().readBytes(RemoteAddress(MapData->Elements), Size);
952+
if (!ElementsBytes)
953+
return;
930954
auto ElementsData =
931955
reinterpret_cast<const ConformanceCacheEntry<Runtime> *>(
932956
ElementsBytes.get());
933-
if (!ElementsData)
934-
return;
935957

936958
for (StoredSize i = 0; i < Count; i++) {
937959
auto &Element = ElementsData[i];
@@ -999,10 +1021,10 @@ class ReflectionContext
9991021
auto AllocationBytes =
10001022
getReader().readBytes(RemoteAddress(Allocation.Ptr),
10011023
Allocation.Size);
1024+
if (!AllocationBytes)
1025+
return 0;
10021026
auto Entry = reinterpret_cast<const GenericMetadataCacheEntry *>(
10031027
AllocationBytes.get());
1004-
if (!Entry)
1005-
return 0;
10061028
return Entry->Value;
10071029
}
10081030
return 0;
@@ -1039,10 +1061,10 @@ class ReflectionContext
10391061
case GenericWitnessTableCacheTag: {
10401062
auto NodeBytes = getReader().readBytes(
10411063
RemoteAddress(Allocation.Ptr), sizeof(MetadataCacheNode<Runtime>));
1064+
if (!NodeBytes)
1065+
return llvm::None;
10421066
auto Node =
10431067
reinterpret_cast<const MetadataCacheNode<Runtime> *>(NodeBytes.get());
1044-
if (!Node)
1045-
return llvm::None;
10461068
return *Node;
10471069
}
10481070
default:
@@ -1095,23 +1117,23 @@ class ReflectionContext
10951117

10961118
auto PoolBytes = getReader()
10971119
.readBytes(AllocationPoolAddr->getResolvedAddress(), sizeof(PoolRange));
1098-
auto Pool = reinterpret_cast<const PoolRange *>(PoolBytes.get());
1099-
if (!Pool)
1120+
if (!PoolBytes)
11001121
return std::string("failure reading allocation pool contents");
1122+
auto Pool = reinterpret_cast<const PoolRange *>(PoolBytes.get());
11011123

11021124
auto TrailerPtr = Pool->Begin + Pool->Remaining;
11031125
while (TrailerPtr) {
11041126
auto TrailerBytes = getReader()
11051127
.readBytes(RemoteAddress(TrailerPtr), sizeof(PoolTrailer));
1106-
auto Trailer = reinterpret_cast<const PoolTrailer *>(TrailerBytes.get());
1107-
if (!Trailer)
1128+
if (!TrailerBytes)
11081129
break;
1130+
auto Trailer = reinterpret_cast<const PoolTrailer *>(TrailerBytes.get());
11091131
auto PoolStart = TrailerPtr - Trailer->PoolSize;
11101132
auto PoolBytes = getReader()
11111133
.readBytes(RemoteAddress(PoolStart), Trailer->PoolSize);
1112-
auto PoolPtr = (const char *)PoolBytes.get();
1113-
if (!PoolPtr)
1134+
if (!PoolBytes)
11141135
break;
1136+
auto PoolPtr = (const char *)PoolBytes.get();
11151137

11161138
uintptr_t Offset = 0;
11171139
while (Offset < Trailer->PoolSize) {
@@ -1153,10 +1175,7 @@ class ReflectionContext
11531175
auto HeaderBytes = getReader().readBytes(
11541176
RemoteAddress(BacktraceListNext),
11551177
sizeof(MetadataAllocationBacktraceHeader<Runtime>));
1156-
auto HeaderPtr =
1157-
reinterpret_cast<const MetadataAllocationBacktraceHeader<Runtime> *>(
1158-
HeaderBytes.get());
1159-
if (HeaderPtr == nullptr) {
1178+
if (!HeaderBytes) {
11601179
// FIXME: std::stringstream would be better, but LLVM's standard library
11611180
// introduces a vtable and we don't want that.
11621181
char result[128];
@@ -1165,6 +1184,9 @@ class ReflectionContext
11651184
BacktraceListNext.getAddressData());
11661185
return std::string(result);
11671186
}
1187+
auto HeaderPtr =
1188+
reinterpret_cast<const MetadataAllocationBacktraceHeader<Runtime> *>(
1189+
HeaderBytes.get());
11681190
auto BacktraceAddrPtr =
11691191
BacktraceListNext +
11701192
sizeof(MetadataAllocationBacktraceHeader<Runtime>);

0 commit comments

Comments
 (0)