[compiler-rt] 056769c - Revert "[Sanitizers] Fix read buffer overrun in scanning loader commands"
Douglas Yung via llvm-commits
llvm-commits at lists.llvm.org
Sat Feb 4 10:20:20 PST 2023
Author: Douglas Yung
Date: 2023-02-04T10:19:57-08:00
New Revision: 056769cdbc60569b01c2f1ff67507f64ad83771f
URL: https://github.com/llvm/llvm-project/commit/056769cdbc60569b01c2f1ff67507f64ad83771f
DIFF: https://github.com/llvm/llvm-project/commit/056769cdbc60569b01c2f1ff67507f64ad83771f.diff
LOG: Revert "[Sanitizers] Fix read buffer overrun in scanning loader commands"
This reverts commit abbd4da2043856f443e3d1c8d2c7627cac93a6ac.
This change is breaking many bots including:
- http://45.33.8.238/linux/98629/step_10.txt
- https://buildkite.com/llvm-project/llvm-main/builds/6461#01861c4f-9d9c-4781-88f7-d6ccddcb4b06/919-8848
- https://lab.llvm.org/buildbot/#/builders/94/builds/13196
- https://lab.llvm.org/buildbot/#/builders/45/builds/10633
- https://lab.llvm.org/buildbot/#/builders/247/builds/1238
- https://lab.llvm.org/buildbot/#/builders/70/builds/33424
- https://lab.llvm.org/buildbot/#/builders/168/builds/11693
- https://lab.llvm.org/buildbot/#/builders/74/builds/17006
- https://lab.llvm.org/buildbot/#/builders/85/builds/14120
Added:
Modified:
compiler-rt/lib/sanitizer_common/sanitizer_procmaps.h
compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp
compiler-rt/lib/sanitizer_common/tests/CMakeLists.txt
Removed:
compiler-rt/lib/sanitizer_common/tests/sanitizer_procmaps_mac_test.cpp
################################################################################
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_procmaps.h b/compiler-rt/lib/sanitizer_common/sanitizer_procmaps.h
index a3887eaac9408..19bad158387c5 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_procmaps.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_procmaps.h
@@ -65,8 +65,6 @@ class MemoryMappedSegment {
MemoryMappedSegmentData *data_;
};
-struct ImageHeader;
-
class MemoryMappingLayoutBase {
public:
virtual bool Next(MemoryMappedSegment *segment) { UNIMPLEMENTED(); }
@@ -77,10 +75,10 @@ class MemoryMappingLayoutBase {
~MemoryMappingLayoutBase() {}
};
-class MemoryMappingLayout : public MemoryMappingLayoutBase {
+class MemoryMappingLayout final : public MemoryMappingLayoutBase {
public:
explicit MemoryMappingLayout(bool cache_enabled);
- virtual ~MemoryMappingLayout();
+ ~MemoryMappingLayout();
virtual bool Next(MemoryMappedSegment *segment) override;
virtual bool Error() const override;
virtual void Reset() override;
@@ -92,14 +90,10 @@ class MemoryMappingLayout : public MemoryMappingLayoutBase {
// Adds all mapped objects into a vector.
void DumpListOfModules(InternalMmapVectorNoCtor<LoadedModule> *modules);
- protected:
-#if SANITIZER_APPLE
- virtual const ImageHeader *CurrentImageHeader();
-#endif
- MemoryMappingLayoutData data_;
-
private:
void LoadFromCache();
+
+ MemoryMappingLayoutData data_;
};
// Returns code range for the specified module.
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp
index b44e016a0e5bc..4b0e678197614 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp
@@ -250,9 +250,7 @@ static bool NextSegmentLoad(MemoryMappedSegment *segment,
MemoryMappedSegmentData *seg_data,
MemoryMappingLayoutData *layout_data) {
const char *lc = layout_data->current_load_cmd_addr;
-
layout_data->current_load_cmd_addr += ((const load_command *)lc)->cmdsize;
- layout_data->current_load_cmd_count--;
if (((const load_command *)lc)->cmd == kLCSegment) {
const SegmentCommand* sc = (const SegmentCommand *)lc;
uptr base_virt_addr, addr_mask;
@@ -360,16 +358,11 @@ static bool IsModuleInstrumented(const load_command *first_lc) {
return false;
}
-const ImageHeader *MemoryMappingLayout::CurrentImageHeader() {
- const mach_header *hdr = (data_.current_image == kDyldImageIdx)
- ? get_dyld_hdr()
- : _dyld_get_image_header(data_.current_image);
- return (const ImageHeader *)hdr;
-}
-
bool MemoryMappingLayout::Next(MemoryMappedSegment *segment) {
for (; data_.current_image >= kDyldImageIdx; data_.current_image--) {
- const mach_header *hdr = (const mach_header *)CurrentImageHeader();
+ const mach_header *hdr = (data_.current_image == kDyldImageIdx)
+ ? get_dyld_hdr()
+ : _dyld_get_image_header(data_.current_image);
if (!hdr) continue;
if (data_.current_load_cmd_count < 0) {
// Set up for this image;
@@ -399,7 +392,7 @@ bool MemoryMappingLayout::Next(MemoryMappedSegment *segment) {
(const load_command *)data_.current_load_cmd_addr);
}
- while (data_.current_load_cmd_count > 0) {
+ for (; data_.current_load_cmd_count >= 0; data_.current_load_cmd_count--) {
switch (data_.current_magic) {
// data_.current_magic may be only one of MH_MAGIC, MH_MAGIC_64.
#ifdef MH_MAGIC_64
@@ -420,7 +413,6 @@ bool MemoryMappingLayout::Next(MemoryMappedSegment *segment) {
}
// If we get here, no more load_cmd's in this image talk about
// segments. Go on to the next image.
- data_.current_load_cmd_count = -1; // This will trigger loading next image
}
return false;
}
diff --git a/compiler-rt/lib/sanitizer_common/tests/CMakeLists.txt b/compiler-rt/lib/sanitizer_common/tests/CMakeLists.txt
index 2ff6d46ee10ee..0c729ecee7152 100644
--- a/compiler-rt/lib/sanitizer_common/tests/CMakeLists.txt
+++ b/compiler-rt/lib/sanitizer_common/tests/CMakeLists.txt
@@ -35,7 +35,6 @@ set(SANITIZER_UNITTESTS
sanitizer_posix_test.cpp
sanitizer_printf_test.cpp
sanitizer_procmaps_test.cpp
- sanitizer_procmaps_mac_test.cpp
sanitizer_ring_buffer_test.cpp
sanitizer_quarantine_test.cpp
sanitizer_stack_store_test.cpp
diff --git a/compiler-rt/lib/sanitizer_common/tests/sanitizer_procmaps_mac_test.cpp b/compiler-rt/lib/sanitizer_common/tests/sanitizer_procmaps_mac_test.cpp
deleted file mode 100644
index f622bba246309..0000000000000
--- a/compiler-rt/lib/sanitizer_common/tests/sanitizer_procmaps_mac_test.cpp
+++ /dev/null
@@ -1,137 +0,0 @@
-//===-- sanitizer_procmaps_mac_test.cpp ---------------------------------------===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-//
-// This file is a part of ThreadSanitizer/AddressSanitizer runtime.
-//
-//===----------------------------------------------------------------------===//
-
-# include "sanitizer_common/sanitizer_platform.h"
-
-# if SANITIZER_APPLE
-
-# include <stdlib.h>
-# include <string.h>
-# include <stdint.h>
-# include <stdio.h>
-
-# include <vector>
-# include <mach-o/dyld.h>
-# include <mach-o/loader.h>
-
-# include "gtest/gtest.h"
-
-# include "sanitizer_common/sanitizer_procmaps.h"
-
-namespace __sanitizer {
-
-class MemoryMappingLayoutMock final : public MemoryMappingLayout {
-private:
- static constexpr uuid_command mock_uuid_command = {
- .cmd = LC_UUID,
- .cmdsize = sizeof(uuid_command),
- .uuid = {}
- };
-
- static constexpr char dylib_name[] = "libclang_rt.\0\0\0"; // 8 bytes aligned, padded with zeros per loader.h
- static constexpr dylib_command mock_dylib_command = {
- .cmd = LC_LOAD_DYLIB,
- .cmdsize = sizeof(dylib_command) + sizeof(dylib_name),
- .dylib = {
- .name = {
- .offset = sizeof(dylib_command)
- }
- }
- };
-
- static constexpr uuid_command mock_trap_command = {
- .cmd = LC_UUID,
- .cmdsize = 0x10000,
- .uuid = {}
- };
-
- const char *start_load_cmd_addr;
- size_t sizeofcmds;
- std::vector<unsigned char> mock_header;
-
-public:
- MemoryMappingLayoutMock(): MemoryMappingLayout(false) {
- EXPECT_EQ(mock_uuid_command.cmdsize % 8, 0u);
- EXPECT_EQ(mock_dylib_command.cmdsize % 8, 0u);
-
- Reset();
-
-#ifdef MH_MAGIC_64
- const struct mach_header_64 *header = (mach_header_64 *)_dyld_get_image_header(0); // Any header will do
- const size_t header_size = sizeof(mach_header_64);
-#else
- const struct mach_header *header = _dyld_get_image_header(0);
- const size_t header_size = sizeof(mach_header);
-#endif
- const size_t mock_header_size_with_extras = header_size + header->sizeofcmds +
- mock_uuid_command.cmdsize + mock_dylib_command.cmdsize + sizeof(uuid_command);
-
- mock_header.reserve(mock_header_size_with_extras);
- // Copy the original header
- copy((unsigned char *)header,
- (unsigned char *)header + header_size + header->sizeofcmds,
- back_inserter(mock_header));
- // The following commands are not supposed to be processed
- // by the (correct) ::Next method at all, since they're not
- // accounted for in header->ncmds .
- copy((unsigned char *)&mock_uuid_command,
- ((unsigned char *)&mock_uuid_command) + mock_uuid_command.cmdsize,
- back_inserter(mock_header));
- copy((unsigned char *)&mock_dylib_command,
- ((unsigned char *)&mock_dylib_command) + sizeof(dylib_command), // as mock_dylib_command.cmdsize contains the following string
- back_inserter(mock_header));
- copy((unsigned char *)dylib_name,
- ((unsigned char *)dylib_name) + sizeof(dylib_name),
- back_inserter(mock_header));
-
- // Append a command w. huge size to have the test detect the read overrun
- copy((unsigned char *)&mock_trap_command,
- ((unsigned char *)&mock_trap_command) + sizeof(uuid_command),
- back_inserter(mock_header));
-
- start_load_cmd_addr = (const char *)(mock_header.data() + header_size);
- sizeofcmds = header->sizeofcmds;
-
- const char *last_byte_load_cmd_addr = (start_load_cmd_addr+sizeofcmds-1);
- data_.current_image = -1; // So the loop in ::Next runs just once
- }
-
- size_t SizeOfLoadCommands() {
- return sizeofcmds;
- }
-
- size_t CurrentLoadCommandOffset() {
- size_t offset = data_.current_load_cmd_addr - start_load_cmd_addr;
- return offset;
- }
-
-protected:
- virtual ImageHeader *CurrentImageHeader() override {
- return (ImageHeader *)mock_header.data();
- }
-};
-
-TEST(MemoryMappingLayout, Next) {
- __sanitizer::MemoryMappingLayoutMock memory_mapping;
- __sanitizer::MemoryMappedSegment segment;
- size_t size = memory_mapping.SizeOfLoadCommands();
- while (memory_mapping.Next(&segment)) {
- size_t offset = memory_mapping.CurrentLoadCommandOffset();
- EXPECT_LE(offset, size);
- }
- size_t final_offset = memory_mapping.CurrentLoadCommandOffset();
- EXPECT_EQ(final_offset, size); // All commands processed, no more, no less
-}
-
-} // namespace __sanitizer
-
-# endif // SANITIZER_APPLE
More information about the llvm-commits
mailing list