[debugserver] Support for qMemTags packet
#160952
Conversation
Extend handling of `qMemoryRegionInfo` packet to add flags (`flags:<space-separated-flags>;`) including whether or not the region is mapped with taggable memory (`mt`).
Add support for reading memory tags (`qMemTags` packet) to debugserver. rdar://152169151
Add support for determining if processes can run with MTE enabled (`memory-tagging+` feature in `qSupported` packet).
Indicate whether a process instance is running with MTE enabled in the response to the `qProcessInfo` packet.
Ensure we can keep building debugserver with `memory tag read <addr-expr>` support with older SDKs.
Add test for Darwin MTE support which covers support for custom tag fault message and reading memory tags via `memory tag read`.
yln
requested review from
DanBlackwell,
DavidSpickett,
devincoughlin,
jasonmolenda and
jimingham
|
@llvm/pr-subscribers-lldb Author: Julian Lettner (yln) ChangesSupport for Patch is 26.64 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/160952.diff 16 Files Affected:
diff --git a/lldb/packages/Python/lldbsuite/test/cpu_feature.py b/lldb/packages/Python/lldbsuite/test/cpu_feature.py
index b46a5acc596f0..d7668c1884e40 100644
--- a/lldb/packages/Python/lldbsuite/test/cpu_feature.py
+++ b/lldb/packages/Python/lldbsuite/test/cpu_feature.py
@@ -62,7 +62,7 @@ def _is_supported_darwin(self, cmd_runner):
class AArch64:
FPMR = CPUFeature("fpmr")
GCS = CPUFeature("gcs")
- MTE = CPUFeature("mte")
+ MTE = CPUFeature("mte", "hw.optional.arm.FEAT_MTE4")
MTE_STORE_ONLY = CPUFeature("mtestoreonly")
PTR_AUTH = CPUFeature("paca", "hw.optional.arm.FEAT_PAuth2")
SME = CPUFeature("sme", "hw.optional.arm.FEAT_SME")
diff --git a/lldb/test/API/macosx/mte/Makefile b/lldb/test/API/macosx/mte/Makefile
new file mode 100644
index 0000000000000..cb20942805e2a
--- /dev/null
+++ b/lldb/test/API/macosx/mte/Makefile
@@ -0,0 +1,12 @@
+C_SOURCES := main.c
+
+EXE := uaf_mte
+
+all: uaf_mte sign
+
+include Makefile.rules
+
+sign: mte-entitlements.plist uaf_mte
+ifeq ($(OS),Darwin)
+ codesign -s - -f --entitlements $^
+endif
diff --git a/lldb/test/API/macosx/mte/TestDarwinMTE.py b/lldb/test/API/macosx/mte/TestDarwinMTE.py
new file mode 100644
index 0000000000000..787f29e3530cd
--- /dev/null
+++ b/lldb/test/API/macosx/mte/TestDarwinMTE.py
@@ -0,0 +1,90 @@
+"""Test MTE Memory Tagging on Apple platforms"""
+
+import lldb
+import re
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+import lldbsuite.test.cpu_feature as cpu_feature
+
+exe_name = "uaf_mte" # Must match Makefile
+
+
+class TestDarwinMTE(TestBase):
+ NO_DEBUG_INFO_TESTCASE = True
+
+ @skipUnlessFeature(cpu_feature.AArch64.MTE)
+ def test_tag_fault(self):
+ self.build()
+ exe = self.getBuildArtifact(exe_name)
+
+ target = self.dbg.CreateTarget(exe)
+ self.assertTrue(target, VALID_TARGET)
+
+ process = target.LaunchSimple(None, None, None)
+ self.assertState(process.GetState(), lldb.eStateStopped, PROCESS_STOPPED)
+
+ self.expect(
+ "thread info",
+ substrs=["stop reason = EXC_ARM_MTE_TAG_FAULT", "MTE tag mismatch detected"],
+ )
+
+ @skipUnlessFeature(cpu_feature.AArch64.MTE)
+ def test_memory_read_with_tags(self):
+ self.build()
+ lldbutil.run_to_source_breakpoint(
+ self, "// before free", lldb.SBFileSpec("main.c"), exe_name=exe_name
+ )
+
+ # (lldb) memory read ptr-16 ptr+48 --show-tags
+ # 0x7d2c00930: 00 00 00 00 00 00 00 00 d0 e3 a5 0a 02 00 00 00 ................ (tag: 0x3)
+ # 0x7d2c00940: 48 65 6c 6c 6f 00 00 00 00 00 00 00 00 00 00 00 Hello........... (tag: 0xb)
+ # 0x7d2c00950: 57 6f 72 6c 64 00 00 00 00 00 00 00 00 00 00 00 World........... (tag: 0xb)
+ # 0x7d2c00960: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ (tag: 0x9)
+ self.expect(
+ "memory read ptr-16 ptr+48 --show-tags",
+ substrs=[" Hello...........", " World..........."],
+ patterns=[r"(.*\(tag: 0x[0-9a-f]\)\n){4}"])
+
+ def _parse_pointer_tag(self):
+ return re.search(r"Logical tag: (0x[0-9a-f])", self.res.GetOutput()).group(1)
+
+ def _parse_memory_tags(self, expected_tag_count):
+ tags = re.findall(r"\): (0x[0-9a-f])", self.res.GetOutput())
+ self.assertEqual(len(tags), expected_tag_count)
+ return tags
+
+ @skipUnlessFeature(cpu_feature.AArch64.MTE)
+ def test_memory_tag_read(self):
+ self.build()
+ lldbutil.run_to_source_breakpoint(
+ self, "// before free", lldb.SBFileSpec("main.c"), exe_name=exe_name
+ )
+
+ # (lldb) memory tag read ptr-1 ptr+33
+ # Logical tag: 0x5
+ # Allocation tags:
+ # [0x100a65a40, 0x100a65a50): 0xf (mismatch)
+ # [0x100a65a50, 0x100a65a60): 0x5
+ # [0x100a65a60, 0x100a65a70): 0x5
+ # [0x100a65a70, 0x100a65a80): 0x2 (mismatch)
+ self.expect(
+ "memory tag read ptr-1 ptr+33",
+ substrs=["Logical tag: 0x", "Allocation tags:", "(mismatch)"],
+ patterns=[r"(\[.*\): 0x[0-9a-f].*\n){4}"]
+ )
+ self.assertEqual(self.res.GetOutput().count("(mismatch)"), 2)
+ ptr_tag = self._parse_pointer_tag()
+ tags = self._parse_memory_tags(4)
+ self.assertEqual(tags[1], ptr_tag)
+ self.assertEqual(tags[2], ptr_tag)
+ self.assertNotEqual(tags[0], ptr_tag)
+ self.assertNotEqual(tags[3], ptr_tag)
+
+ # Continue running until MTE fault
+ self.runCmd("process continue")
+
+ self.runCmd("memory tag read ptr-1 ptr+33")
+ self.assertEqual(self.res.GetOutput().count("(mismatch)"), 4)
+ tags = self._parse_memory_tags(4)
+ self.assertTrue(all(t != ptr_tag for t in tags))
diff --git a/lldb/test/API/macosx/mte/main.c b/lldb/test/API/macosx/mte/main.c
new file mode 100644
index 0000000000000..ca61c05b19a38
--- /dev/null
+++ b/lldb/test/API/macosx/mte/main.c
@@ -0,0 +1,29 @@
+#include <malloc/malloc.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+// Produce some names on the trace
+const size_t tag_granule = 16;
+uint8_t *my_malloc(void) { return malloc(2 * tag_granule); }
+uint8_t *allocate(void) { return my_malloc(); }
+
+void my_free(void *ptr) { free(ptr); }
+void deallocate(void *ptr) { my_free(ptr); }
+
+void touch_memory(uint8_t *ptr) { ptr[7] = 1; } // invalid access
+void modify(uint8_t *ptr) { touch_memory(ptr); }
+
+int main() {
+ uint8_t *ptr = allocate();
+ printf("ptr: %p\n", ptr);
+
+ strcpy((char *)ptr, "Hello");
+ strcpy((char *)ptr+16, "World");
+
+ deallocate(ptr); // before free
+
+ modify(ptr); // use-after-free
+
+ return 0;
+}
diff --git a/lldb/test/API/macosx/mte/mte-entitlements.plist b/lldb/test/API/macosx/mte/mte-entitlements.plist
new file mode 100644
index 0000000000000..6de5d5634d878
--- /dev/null
+++ b/lldb/test/API/macosx/mte/mte-entitlements.plist
@@ -0,0 +1,10 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
+<plist version="1.0">
+<dict>
+ <key>com.apple.security.hardened-process</key>
+ <true/>
+ <key>com.apple.security.hardened-process.checked-allocations</key>
+ <true/>
+</dict>
+</plist>
diff --git a/lldb/tools/debugserver/source/DNB.cpp b/lldb/tools/debugserver/source/DNB.cpp
index f541134b43a1b..0cd48d91a682a 100644
--- a/lldb/tools/debugserver/source/DNB.cpp
+++ b/lldb/tools/debugserver/source/DNB.cpp
@@ -1386,6 +1386,16 @@ int DNBProcessMemoryRegionInfo(nub_process_t pid, nub_addr_t addr,
return -1;
}
+nub_bool_t DNBProcessGetMemoryTags(nub_process_t pid, nub_addr_t addr,
+ nub_size_t size,
+ std::vector<uint8_t> &tags) {
+ MachProcessSP procSP;
+ if (GetProcessSP(pid, procSP))
+ return procSP->Task().GetMemoryTags(addr, size, tags);
+
+ return false;
+}
+
std::string DNBProcessGetProfileData(nub_process_t pid,
DNBProfileDataScanType scanType) {
MachProcessSP procSP;
diff --git a/lldb/tools/debugserver/source/DNB.h b/lldb/tools/debugserver/source/DNB.h
index 10d1f68794355..1f3d5392c588f 100644
--- a/lldb/tools/debugserver/source/DNB.h
+++ b/lldb/tools/debugserver/source/DNB.h
@@ -105,6 +105,9 @@ nub_bool_t DNBProcessMemoryDeallocate(nub_process_t pid,
nub_addr_t addr) DNB_EXPORT;
int DNBProcessMemoryRegionInfo(nub_process_t pid, nub_addr_t addr,
DNBRegionInfo *region_info) DNB_EXPORT;
+nub_bool_t DNBProcessGetMemoryTags(nub_process_t pid, nub_addr_t addr,
+ nub_size_t size,
+ std::vector<uint8_t> &tags) DNB_EXPORT;
std::string
DNBProcessGetProfileData(nub_process_t pid,
DNBProfileDataScanType scanType) DNB_EXPORT;
diff --git a/lldb/tools/debugserver/source/DNBDefs.h b/lldb/tools/debugserver/source/DNBDefs.h
index df8ca809d412c..d98399aed5e19 100644
--- a/lldb/tools/debugserver/source/DNBDefs.h
+++ b/lldb/tools/debugserver/source/DNBDefs.h
@@ -358,10 +358,11 @@ struct DNBExecutableImageInfo {
struct DNBRegionInfo {
public:
DNBRegionInfo()
- : addr(0), size(0), permissions(0), dirty_pages(), vm_types() {}
+ : addr(0), size(0), permissions(0), flags(), dirty_pages(), vm_types() {}
nub_addr_t addr;
nub_addr_t size;
uint32_t permissions;
+ std::vector<std::string> flags;
std::vector<nub_addr_t> dirty_pages;
std::vector<std::string> vm_types;
};
diff --git a/lldb/tools/debugserver/source/MacOSX/MachTask.h b/lldb/tools/debugserver/source/MacOSX/MachTask.h
index 2284f6b99de91..c4a20b80fda95 100644
--- a/lldb/tools/debugserver/source/MacOSX/MachTask.h
+++ b/lldb/tools/debugserver/source/MacOSX/MachTask.h
@@ -56,6 +56,8 @@ class MachTask {
nub_size_t ReadMemory(nub_addr_t addr, nub_size_t size, void *buf);
nub_size_t WriteMemory(nub_addr_t addr, nub_size_t size, const void *buf);
int GetMemoryRegionInfo(nub_addr_t addr, DNBRegionInfo *region_info);
+ nub_bool_t GetMemoryTags(nub_addr_t addr, nub_size_t size,
+ std::vector<uint8_t> &tags);
std::string GetProfileData(DNBProfileDataScanType scanType);
nub_addr_t AllocateMemory(nub_size_t size, uint32_t permissions);
diff --git a/lldb/tools/debugserver/source/MacOSX/MachTask.mm b/lldb/tools/debugserver/source/MacOSX/MachTask.mm
index 8ae9d4df99657..f25cfac7b030a 100644
--- a/lldb/tools/debugserver/source/MacOSX/MachTask.mm
+++ b/lldb/tools/debugserver/source/MacOSX/MachTask.mm
@@ -213,7 +213,7 @@
}
//----------------------------------------------------------------------
-// MachTask::MemoryRegionInfo
+// MachTask::GetMemoryRegionInfo
//----------------------------------------------------------------------
int MachTask::GetMemoryRegionInfo(nub_addr_t addr, DNBRegionInfo *region_info) {
task_t task = TaskPort();
@@ -221,7 +221,7 @@
return -1;
int ret = m_vm_memory.GetMemoryRegionInfo(task, addr, region_info);
- DNBLogThreadedIf(LOG_MEMORY, "MachTask::MemoryRegionInfo ( addr = 0x%8.8llx "
+ DNBLogThreadedIf(LOG_MEMORY, "MachTask::GetMemoryRegionInfo ( addr = 0x%8.8llx "
") => %i (start = 0x%8.8llx, size = 0x%8.8llx, "
"permissions = %u)",
(uint64_t)addr, ret, (uint64_t)region_info->addr,
@@ -229,6 +229,23 @@
return ret;
}
+//----------------------------------------------------------------------
+// MachTask::GetMemoryTags
+//----------------------------------------------------------------------
+nub_bool_t MachTask::GetMemoryTags(nub_addr_t addr, nub_size_t size,
+ std::vector<uint8_t> &tags) {
+ task_t task = TaskPort();
+ if (task == TASK_NULL)
+ return false;
+
+ bool ok = m_vm_memory.GetMemoryTags(task, addr, size, tags);
+ DNBLogThreadedIf(LOG_MEMORY, "MachTask::GetMemoryTags ( addr = 0x%8.8llx, "
+ "size = 0x%8.8llx ) => %s ( tag count = %llu)",
+ (uint64_t)addr, (uint64_t)size, (ok ? "ok" : "err"),
+ (uint64_t)tags.size());
+ return ok;
+}
+
#define TIME_VALUE_TO_TIMEVAL(a, r) \
do { \
(r)->tv_sec = (a)->seconds; \
diff --git a/lldb/tools/debugserver/source/MacOSX/MachVMMemory.cpp b/lldb/tools/debugserver/source/MacOSX/MachVMMemory.cpp
index f3aa4d7d980fd..7ecd57e7bbfeb 100644
--- a/lldb/tools/debugserver/source/MacOSX/MachVMMemory.cpp
+++ b/lldb/tools/debugserver/source/MacOSX/MachVMMemory.cpp
@@ -13,6 +13,7 @@
#include "MachVMMemory.h"
#include "DNBLog.h"
#include "MachVMRegion.h"
+#include <cassert>
#include <dlfcn.h>
#include <mach/mach_vm.h>
#include <mach/shared_region.h>
@@ -123,6 +124,7 @@ nub_bool_t MachVMMemory::GetMemoryRegionInfo(task_t task, nub_addr_t address,
region_info->addr = vmRegion.StartAddress();
region_info->size = vmRegion.GetByteSize();
region_info->permissions = vmRegion.GetDNBPermissions();
+ region_info->flags = vmRegion.GetFlags();
region_info->dirty_pages =
get_dirty_pages(task, vmRegion.StartAddress(), vmRegion.GetByteSize());
region_info->vm_types = vmRegion.GetMemoryTypes();
@@ -150,6 +152,63 @@ nub_bool_t MachVMMemory::GetMemoryRegionInfo(task_t task, nub_addr_t address,
return true;
}
+// API availability:
+// mach_vm_update_pointers_with_remote_tags() - 26.0
+// VM_OFFSET_LIST_MAX macro - 26.1
+#ifndef VM_OFFSET_LIST_MAX
+#define VM_OFFSET_LIST_MAX 512
+#endif
+using mach_vm_offset_list_t = mach_vm_offset_t *;
+using mach_vm_update_pointers_with_remote_tags_t = kern_return_t(
+ mach_port_name_t target, mach_vm_offset_list_t in_pointer_list,
+ mach_msg_type_number_t in_pointer_listCnt,
+ mach_vm_offset_list_t out_pointer_list,
+ mach_msg_type_number_t *out_pointer_listCnt);
+
+nub_bool_t MachVMMemory::GetMemoryTags(task_t task, nub_addr_t address,
+ nub_size_t size,
+ std::vector<uint8_t> &tags) {
+ static auto mach_vm_update_pointers_with_remote_tags =
+ (mach_vm_update_pointers_with_remote_tags_t *)dlsym(
+ RTLD_DEFAULT, "mach_vm_update_pointers_with_remote_tags");
+ assert(mach_vm_update_pointers_with_remote_tags);
+
+ // Max batch size supported by mach_vm_update_pointers_with_remote_tags()
+ constexpr uint32_t max_ptr_count = VM_OFFSET_LIST_MAX;
+ constexpr uint32_t tag_shift = 56;
+ constexpr nub_addr_t tag_mask =
+ ((nub_addr_t)0x0f << tag_shift); // Lower half of top byte
+ constexpr uint32_t tag_granule = 16;
+
+ mach_msg_type_number_t ptr_count =
+ (size / tag_granule) + ((size % tag_granule > 0) ? 1 : 0);
+ ptr_count = std::min(ptr_count, max_ptr_count);
+
+ auto ptr_arr = std::make_unique<mach_vm_offset_t[]>(ptr_count);
+ for (size_t i = 0; i < ptr_count; i++)
+ ptr_arr[i] = (address + i * tag_granule);
+
+ mach_msg_type_number_t ptr_count_out = ptr_count;
+ m_err = mach_vm_update_pointers_with_remote_tags(
+ task, ptr_arr.get(), ptr_count, ptr_arr.get(), &ptr_count_out);
+
+ const bool failed = (m_err.Fail() || (ptr_count != ptr_count_out));
+ if (failed || DNBLogCheckLogBit(LOG_MEMORY))
+ m_err.LogThreaded("::mach_vm_update_pointers_with_remote_tags ( task = "
+ "0x%4.4x, ptr_count = %d ) => %i ( ptr_count_out = %d)",
+ task, ptr_count, m_err.Status(), ptr_count_out);
+ if (failed)
+ return false;
+
+ tags.reserve(ptr_count);
+ for (size_t i = 0; i < ptr_count; i++) {
+ nub_addr_t tag = (ptr_arr[i] & tag_mask) >> tag_shift;
+ tags.push_back(tag);
+ }
+
+ return true;
+}
+
static uint64_t GetPhysicalMemory() {
// This doesn't change often at all. No need to poll each time.
static uint64_t physical_memory = 0;
diff --git a/lldb/tools/debugserver/source/MacOSX/MachVMMemory.h b/lldb/tools/debugserver/source/MacOSX/MachVMMemory.h
index 05d2c029b9980..8a7616091fbb3 100644
--- a/lldb/tools/debugserver/source/MacOSX/MachVMMemory.h
+++ b/lldb/tools/debugserver/source/MacOSX/MachVMMemory.h
@@ -28,6 +28,8 @@ class MachVMMemory {
nub_size_t PageSize(task_t task);
nub_bool_t GetMemoryRegionInfo(task_t task, nub_addr_t address,
DNBRegionInfo *region_info);
+ nub_bool_t GetMemoryTags(task_t task, nub_addr_t address, nub_size_t size,
+ std::vector<uint8_t> &tags);
nub_bool_t GetMemoryProfile(DNBProfileDataScanType scanType, task_t task,
struct task_basic_info ti, cpu_type_t cputype,
nub_process_t pid, vm_statistics64_data_t &vminfo,
diff --git a/lldb/tools/debugserver/source/MacOSX/MachVMRegion.cpp b/lldb/tools/debugserver/source/MacOSX/MachVMRegion.cpp
index 97908b4acaf28..9d0d60fdaaed9 100644
--- a/lldb/tools/debugserver/source/MacOSX/MachVMRegion.cpp
+++ b/lldb/tools/debugserver/source/MacOSX/MachVMRegion.cpp
@@ -114,6 +114,11 @@ bool MachVMRegion::RestoreProtections() {
return false;
}
+#ifdef VM_REGION_FLAG_JIT_ENABLED
+#define VM_REGION_HAS_FLAGS 1
+#else
+#define VM_REGION_HAS_FLAGS 0
+#endif
bool MachVMRegion::GetRegionForAddress(nub_addr_t addr) {
// Restore any original protections and clear our vars
Clear();
@@ -140,6 +145,30 @@ bool MachVMRegion::GetRegionForAddress(nub_addr_t addr) {
if (failed)
return false;
if (log_protections) {
+#if VM_REGION_HAS_FLAGS
+ DNBLogThreaded("info = { prot = %u, "
+ "max_prot = %u, "
+ "inheritance = 0x%8.8x, "
+ "offset = 0x%8.8llx, "
+ "user_tag = 0x%8.8x, "
+ "ref_count = %u, "
+ "shadow_depth = %u, "
+ "ext_pager = %u, "
+ "share_mode = %u, "
+ "is_submap = %d, "
+ "behavior = %d, "
+ "object_id = 0x%8.8x, "
+ "user_wired_count = 0x%4.4x, "
+ "flags = %d }",
+ m_data.protection, m_data.max_protection, m_data.inheritance,
+ (uint64_t)m_data.offset, m_data.user_tag, m_data.ref_count,
+ m_data.shadow_depth, m_data.external_pager,
+ m_data.share_mode, m_data.is_submap, m_data.behavior,
+ m_data.object_id, m_data.user_wired_count, m_data.flags);
+#else
+ // Duplicate log call instead of #if-defing printing of flags to avoid
+ // compiler warning: 'embedding a directive within macro arguments has
+ // undefined behavior'
DNBLogThreaded("info = { prot = %u, "
"max_prot = %u, "
"inheritance = 0x%8.8x, "
@@ -158,6 +187,7 @@ bool MachVMRegion::GetRegionForAddress(nub_addr_t addr) {
m_data.shadow_depth, m_data.external_pager,
m_data.share_mode, m_data.is_submap, m_data.behavior,
m_data.object_id, m_data.user_wired_count);
+#endif
}
m_curr_protection = m_data.protection;
@@ -183,6 +213,22 @@ uint32_t MachVMRegion::GetDNBPermissions() const {
return dnb_permissions;
}
+#ifndef VM_REGION_FLAG_MTE_ENABLED
+#define VM_REGION_FLAG_MTE_ENABLED 0x4
+#endif
+std::vector<std::string> MachVMRegion::GetFlags() const {
+ std::vector<std::string> flags;
+#if VM_REGION_HAS_FLAGS
+ if (m_data.flags & VM_REGION_FLAG_JIT_ENABLED)
+ flags.push_back("jit");
+ if (m_data.flags & VM_REGION_FLAG_TPRO_ENABLED)
+ flags.push_back("tpro");
+ if (m_data.flags & VM_REGION_FLAG_MTE_ENABLED)
+ flags.push_back("mt");
+#endif
+ return flags;
+}
+
std::vector<std::string> MachVMRegion::GetMemoryTypes() const {
std::vector<std::string> types;
if (m_data.user_tag == VM_MEMORY_STACK) {
diff --git a/lldb/tools/debugserver/source/MacOSX/MachVMRegion.h b/lldb/tools/debugserver/source/MacOSX/MachVMRegion.h
index cb7705893c7ed..ba6e1f3bfa70e 100644
--- a/lldb/tools/debugserver/source/MacOSX/MachVMRegion.h
+++ b/lldb/tools/debugserver/source/MacOSX/MachVMRegion.h
@@ -40,9 +40,10 @@ class MachVMRegion {
vm_prot_t prot);
bool RestoreProtections();
bool GetRegionForAddress(nub_addr_t addr);
- std::vector<std::string> GetMemoryTypes() const;
uint32_t GetDNBPermissions() const;
+ std::vector<std::string> GetFlags() const;
+ std::vector<std::string> GetMemoryTypes() const;
const DNBError &GetError() { return m_err; }
diff --git a/lldb/tools/debugserver/source/RNBRemote.cpp b/lldb/tools/debugserver/source/RNBRemote.cpp
index d9fb22c6a1c06..be7108f633517 100644
--- a/lldb/tools/debugserver/source/RNBRemote.cpp
+++ b/lldb/tools/debugserver/source/RNBRemote.cpp
@@ -22,6 +22,9 @@
#include <mach/mach_vm.h>
#include <mach/task_info.h>
#include <memory>
+#if __has_include(<os/security_config.h>)
+#include <os/security_config.h>
+#endif
#include <pwd.h>
#include <string>
#include <sys/stat.h>
@@ -502,6 +505,8 @@ void RNBRemote::CreatePacketTable() {
memory_region_info, &RNBRemote::HandlePacket_MemoryRegionInfo, NULL,
"qMemoryRegionInfo", "Return size and attributes of a memory region that...
[truncated]
|
There was a problem hiding this comment.
Though it's not explicitly mentioned, I think the testing does cover the over/under alignment to fit to granules. That's what I spent the most time on for Linux.
No testing on a packet level though. For lldb-server those live in lldb/test/API/tools/lldb-server/memory-tagging/TestGdbRemoteMemoryTagging.py. Mostly it covers the invalid packet forms.
No tests for truncated reads or skipping untagged regions. I suspect you can't have untagged memory as your MTE seems to be per process. So that would mean you can't fail to read tags either?
Even if untagged regions are possible, I'd be ok with you leaning on the Linux tests to cover it since it requires making dodgy assumptions about mmap's behaviour.
I do think that truncated reads are worth testing if they are in fact possible. For Linux we just fail in this case, under the assumption that the memory tag manager should have looked for the gaps before it issued the reads.
And any question of code quality in the debugserver parts is up to your Apple colleagues of course.
| rep << "vendor:apple;"; | ||
|
|
||
| if (ProcessRunningWithMemoryTagging(pid)) | ||
| rep << "mte:enabled;"; |
There was a problem hiding this comment.
Is this something new or does it exist for Linux too? It's been a while since I wrote the Linux side, I think we just looked at specific memory because MTE is a per region thing on Linux. Rather than being on for the entire process.
There was a problem hiding this comment.
I don't know if there is a conceptual counterpart for "MTE enabled in process?" on Linux.
mte:enabled is new for the qProcessInfo packet in debugserver per request from @jasonmolenda. We don't use it yet, but I think it will be useful to have.
There was a problem hiding this comment.
Sorry for the delay. Yes, I asked Julian to add this, it's purely for me reading packet logs, trying to understand what is going on in a debug session. It's not used in lldb, and I'm not sure it will ever be needed. But from a debugging-via-packet-logs perspective, I think it will be helpful to have explicitly in the log.
|
Also is now a good time to add a test that the |
Yes!
I agree that this is a deficiency in debugserver testing. Do we have any plans of addressing this? @JDevlieghere @jasonmolenda
My understanding is that all of the tag interpretation and error handling happens on the LLDB client side, so these aspects are already tested by the Linux tests. I essentially only re-implemented the existing
Thank you for your diligent reviews! :) |
Yes, it should just work. I will look into adding one more integration test for |
|
I've added an integration test for the @DavidSpickett |
Co-authored-by: Jonas Devlieghere <jonas@devlieghere.com>
I asked the question expecting the answer to be no, just to be clear :) In an ideal world we'd have this but I'm not going to block this review because of it. |
Ok but for my own education can you tell me whether the following situation can actually happen on your systems? With this layout on Linux I may end up reading across boundaries so parts of my memory read will have tags and parts won't. I wonder if you have a more "per process" rather than "per page" model. Also I bring this up because I'm not running those Linux tests with any regularity. If/when we have MTE hardware to hand, it'll happen but I'm not pushing for it myself. If the above scenario cannot happen in your systems, or you have your own integration testing anyway, this is not a problem for you. But of course make your own judgements here. If you did want to test that stuff, you might be able to port the Linux tests but I wouldn't try too hard to do so. There's going to be enough differences that mean the tests would be clearer staying separate. |
|
|
||
| deallocate(ptr); // before free | ||
|
|
||
| modify(ptr); // use-after-free |
There was a problem hiding this comment.
Ok I'm guessing since you don't have to PROT_MTE like we do on Linux, that tagging is a per process property?
Which makes your job significantly easier.
| <dict> | ||
| <key>com.apple.security.hardened-process</key> | ||
| <true/> | ||
| <key>com.apple.security.hardened-process.checked-allocations</key> |
There was a problem hiding this comment.
So this opts the entire process into tagging, correct?
There was a problem hiding this comment.
This enables MTE in processes spawned from this binary in the production config (for security), but it does not mean that all memory of this process has tags.
Only memory regions mapped with vm_map(..., flags=VM_FLAGS_MTE, ...) will be taggable.
The system allocator and other OS components automatically do this, so most allocations are protected.
So the scenario you mentioned above (intermingled tagged and untagged pages) can happen.
Support for `qMemTags` packet in debugserver which allows usage of LLDB's `memory tag read` on Darwin.
5 participants
Support for
qMemTagspacket in debugserver which allows usage of LLDB'smemory tag readon Darwin.