Skip to content

[lldb] Don't use a vm addr range starting at 0 for local memory #100288

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

jasonmolenda
Copy link
Collaborator

When an inferior stub cannot allocate memory for lldb, and lldb needs to store the result of expressions, it will do it in lldb's own memory range ("host memory"). But it needs to find a virtual address range that is not used in the inferior process. It tries to use the qMemoryRegionInfo gdb remote serial protocol packet to find a range that is inaccessible, starting at address 0 and moving up the size of each region.

If the first region found at address 0 is inaccessible, lldb will use the address range starting at 0 to mean "read lldb's host memory, not the process memory", and programs that crash with a null dereference will have poor behavior.

This patch skips consideration of a memory region that starts at address 0.

I also clarified the documentation of qMemoryRegionInfo to make it clear that the stub is required to provide permissions for a memory range that is accessable, it is not an optional key in this response. This issue was originally found by a stub that did not list permissions in its response, and lldb treated the first region returned as the one it would use. (the stub also didn't support the memory-allocate packet)

When an inferior stub cannot allocate memory for lldb, and lldb
needs to store the result of expressions, it will do it in lldb's
own memory range ("host memory").  But it needs to find a virtual
address range that is not used in the inferior process.  It tries
to use the qMemoryRegionInfo gdb remote serial protocol packet to
find a range that is inaccessible, starting at address 0 and moving
up the size of each region.

If the first region found at address 0 is inaccessible, lldb will
use the address range starting at 0 to mean "read lldb's host memory,
not the process memory", and programs that crash with a null
dereference will have poor behavior.

This patch skips consideration of a memory region that starts at
address 0.

I also clarified the documentation of qMemoryRegionInfo to make it
clear that the stub is required to provide permissions for a memory
range that is accessable, it is not an optional key in this response.
This issue was originally found by a stub that did not list
permissions, and lldb treated the first region returned as the one
it would use.
@jasonmolenda
Copy link
Collaborator Author

I created this PR as an alternative approach to the issue reported in #99045

@llvmbot
Copy link
Member

llvmbot commented Jul 24, 2024

@llvm/pr-subscribers-lldb

Author: Jason Molenda (jasonmolenda)

Changes

When an inferior stub cannot allocate memory for lldb, and lldb needs to store the result of expressions, it will do it in lldb's own memory range ("host memory"). But it needs to find a virtual address range that is not used in the inferior process. It tries to use the qMemoryRegionInfo gdb remote serial protocol packet to find a range that is inaccessible, starting at address 0 and moving up the size of each region.

If the first region found at address 0 is inaccessible, lldb will use the address range starting at 0 to mean "read lldb's host memory, not the process memory", and programs that crash with a null dereference will have poor behavior.

This patch skips consideration of a memory region that starts at address 0.

I also clarified the documentation of qMemoryRegionInfo to make it clear that the stub is required to provide permissions for a memory range that is accessable, it is not an optional key in this response. This issue was originally found by a stub that did not list permissions in its response, and lldb treated the first region returned as the one it would use. (the stub also didn't support the memory-allocate packet)


Full diff: https://github.com/llvm/llvm-project/pull/100288.diff

2 Files Affected:

  • (modified) lldb/docs/resources/lldbgdbremote.md (+7-1)
  • (modified) lldb/source/Expression/IRMemoryMap.cpp (+12-5)
diff --git a/lldb/docs/resources/lldbgdbremote.md b/lldb/docs/resources/lldbgdbremote.md
index 7076a75032dae..5cac3736337a8 100644
--- a/lldb/docs/resources/lldbgdbremote.md
+++ b/lldb/docs/resources/lldbgdbremote.md
@@ -1403,6 +1403,12 @@ For instance, with a macOS process which has nothing mapped in the first
 The lack of `permissions:` indicates that none of read/write/execute are valid
 for this region.
 
+The stub must include `permissions:` key-value on all memory ranges
+that are valid to access in the inferior process -- the lack of
+`permissions:` means that this is an inaccessible (no page table
+entries exist, in a system using VM) memory range.  If a stub cannot
+determine actual permissions, return `rwx`.
+
 **Priority To Implement:** Medium
 
 This is nice to have, but it isn't necessary. It helps LLDB
@@ -2434,4 +2440,4 @@ The `0x` prefixes are optional - like most of the gdb-remote packets,
 omitting them will work fine; these numbers are always base 16.
 
 The length of the payload is not provided.  A reliable, 8-bit clean,
-transport layer is assumed.
\ No newline at end of file
+transport layer is assumed.
diff --git a/lldb/source/Expression/IRMemoryMap.cpp b/lldb/source/Expression/IRMemoryMap.cpp
index de631370bb048..bb3e4e0a9a9fc 100644
--- a/lldb/source/Expression/IRMemoryMap.cpp
+++ b/lldb/source/Expression/IRMemoryMap.cpp
@@ -84,7 +84,7 @@ lldb::addr_t IRMemoryMap::FindSpace(size_t size) {
   // any allocations.  Otherwise start at the beginning of memory.
 
   if (m_allocations.empty()) {
-    ret = 0x0;
+    ret = 0;
   } else {
     auto back = m_allocations.rbegin();
     lldb::addr_t addr = back->first;
@@ -116,10 +116,17 @@ lldb::addr_t IRMemoryMap::FindSpace(size_t size) {
     Status err = process_sp->GetMemoryRegionInfo(ret, region_info);
     if (err.Success()) {
       while (true) {
-        if (region_info.GetReadable() != MemoryRegionInfo::OptionalBool::eNo ||
-            region_info.GetWritable() != MemoryRegionInfo::OptionalBool::eNo ||
-            region_info.GetExecutable() !=
-                MemoryRegionInfo::OptionalBool::eNo) {
+        if (region_info.GetRange().GetRangeBase() == 0 &&
+            region_info.GetRange().GetRangeEnd() - 1 < end_of_memory) {
+          // Don't use a region that starts at address 0,
+          // that can mask null dereferences in the inferior.
+          ret = region_info.GetRange().GetRangeEnd();
+        } else if (region_info.GetReadable() !=
+                       MemoryRegionInfo::OptionalBool::eNo ||
+                   region_info.GetWritable() !=
+                       MemoryRegionInfo::OptionalBool::eNo ||
+                   region_info.GetExecutable() !=
+                       MemoryRegionInfo::OptionalBool::eNo) {
           if (region_info.GetRange().GetRangeEnd() - 1 >= end_of_memory) {
             ret = LLDB_INVALID_ADDRESS;
             break;

@jasonmolenda jasonmolenda merged commit 2ba1aee into llvm:main Jul 25, 2024
5 of 6 checks passed
@jasonmolenda jasonmolenda deleted the dont-use-address-range-starting-at-0 branch July 25, 2024 00:26
yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024
)

Summary:
When an inferior stub cannot allocate memory for lldb, and lldb needs to
store the result of expressions, it will do it in lldb's own memory
range ("host memory"). But it needs to find a virtual address range that
is not used in the inferior process. It tries to use the
qMemoryRegionInfo gdb remote serial protocol packet to find a range that
is inaccessible, starting at address 0 and moving up the size of each
region.

If the first region found at address 0 is inaccessible, lldb will use
the address range starting at 0 to mean "read lldb's host memory, not
the process memory", and programs that crash with a null dereference
will have poor behavior.

This patch skips consideration of a memory region that starts at address
0.

I also clarified the documentation of qMemoryRegionInfo to make it clear
that the stub is required to provide permissions for a memory range that
is accessable, it is not an optional key in this response. This issue
was originally found by a stub that did not list permissions in its
response, and lldb treated the first region returned as the one it would
use. (the stub also didn't support the memory-allocate packet)

Test Plan: 

Reviewers: 

Subscribers: 

Tasks: 

Tags: 


Differential Revision: https://phabricator.intern.facebook.com/D60250627
jasonmolenda added a commit to jasonmolenda/llvm-project that referenced this pull request Aug 27, 2024
…#100288)

When an inferior stub cannot allocate memory for lldb, and lldb needs to
store the result of expressions, it will do it in lldb's own memory
range ("host memory"). But it needs to find a virtual address range that
is not used in the inferior process. It tries to use the
qMemoryRegionInfo gdb remote serial protocol packet to find a range that
is inaccessible, starting at address 0 and moving up the size of each
region.

If the first region found at address 0 is inaccessible, lldb will use
the address range starting at 0 to mean "read lldb's host memory, not
the process memory", and programs that crash with a null dereference
will have poor behavior.

This patch skips consideration of a memory region that starts at address
0.

I also clarified the documentation of qMemoryRegionInfo to make it clear
that the stub is required to provide permissions for a memory range that
is accessable, it is not an optional key in this response. This issue
was originally found by a stub that did not list permissions in its
response, and lldb treated the first region returned as the one it would
use. (the stub also didn't support the memory-allocate packet)

(cherry picked from commit 2ba1aee)
jasonmolenda added a commit to swiftlang/llvm-project that referenced this pull request Aug 27, 2024
…-target-vmaddr-0

[lldb] Don't use a vm addr range starting at 0 for local memory (llvm#100288)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants