mossberg created this revision.
mossberg added reviewers: labath, jingham.
mossberg added a project: LLDB.
Herald added a subscriber: lldb-commits.

During the 'thread step-out' command, check that the memory we are about to 
place a breakpoint in is 1. at an Address with a valid Section and 2. in an 
executable Section. Previously, if the current function had a nonstandard stack 
layout/ABI, and had a valid data pointer in the location where the return 
address is usually located, data corruption would occur when the breakpoint was 
written. This could lead to an incorrectly reported crash or silent corruption 
of the program's state. Now, if either of the above checks fail, the command 
safely aborts.

Further discussion:

- This patch doesn't include a unit test -- I'd be happy to add one, but would 
appreciate guidance on how to do so. This is my first time working with the 
lldb codebase.
- I wasn't sure if it was necessary to first check the `log` pointer before 
using it. Some parts of the code do this, and some don't.
- Should we print out the return address in the log line?

Also, I don't have commit access, so I will need some help landing it when it's 
ready.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71372

Files:
  lldb/source/Target/ThreadPlanStepOut.cpp


Index: lldb/source/Target/ThreadPlanStepOut.cpp
===================================================================
--- lldb/source/Target/ThreadPlanStepOut.cpp
+++ lldb/source/Target/ThreadPlanStepOut.cpp
@@ -8,6 +8,7 @@
 
 #include "lldb/Target/ThreadPlanStepOut.h"
 #include "lldb/Breakpoint/Breakpoint.h"
+#include "lldb/Core/Section.h"
 #include "lldb/Core/Value.h"
 #include "lldb/Core/ValueObjectConstResult.h"
 #include "lldb/Symbol/Block.h"
@@ -120,6 +121,21 @@
         }
       }
     }
+
+    // Perform some additional validation on the return address.
+    const auto return_address_section = return_address.GetSection();
+    if (!return_address_section) {
+      LLDB_LOGF(log, "Return address had no section.");
+      return;
+    }
+
+    const auto return_address_section_perms =
+        return_address_section->GetPermissions();
+    if (!(return_address_section_perms & ePermissionsExecutable)) {
+      LLDB_LOGF(log, "Return address did not point to executable memory.");
+      return;
+    }
+
     m_return_addr =
         return_address.GetLoadAddress(&m_thread.GetProcess()->GetTarget());
 


Index: lldb/source/Target/ThreadPlanStepOut.cpp
===================================================================
--- lldb/source/Target/ThreadPlanStepOut.cpp
+++ lldb/source/Target/ThreadPlanStepOut.cpp
@@ -8,6 +8,7 @@
 
 #include "lldb/Target/ThreadPlanStepOut.h"
 #include "lldb/Breakpoint/Breakpoint.h"
+#include "lldb/Core/Section.h"
 #include "lldb/Core/Value.h"
 #include "lldb/Core/ValueObjectConstResult.h"
 #include "lldb/Symbol/Block.h"
@@ -120,6 +121,21 @@
         }
       }
     }
+
+    // Perform some additional validation on the return address.
+    const auto return_address_section = return_address.GetSection();
+    if (!return_address_section) {
+      LLDB_LOGF(log, "Return address had no section.");
+      return;
+    }
+
+    const auto return_address_section_perms =
+        return_address_section->GetPermissions();
+    if (!(return_address_section_perms & ePermissionsExecutable)) {
+      LLDB_LOGF(log, "Return address did not point to executable memory.");
+      return;
+    }
+
     m_return_addr =
         return_address.GetLoadAddress(&m_thread.GetProcess()->GetTarget());
 
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to