[Lldb-commits] [PATCH] D71372: [lldb] Add additional validation on return address in 'thread step-out'

[Mark Mossberg via Phabricator via lldb-commits]

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());
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D71372.233419.patch
Type: text/x-patch
Size: 1121 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20191211/0ac93676/attachment-0001.bin>