[Lldb-commits] [PATCH] D143012: Add a bit of error checking to SBProcess::ReadMemory to check if a nullptr destination buffer was provided, return error

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda created this revision.
jasonmolenda added a reviewer: JDevlieghere.
jasonmolenda added a project: LLDB.
Herald added a project: All.
jasonmolenda requested review of this revision.
Herald added a subscriber: lldb-commits.

I helped a developer using SB API yesterday who had a bug and tried to read 0xffffffffffffffe8 with SBProcess::ReadMemory.  swig would try to malloc a buffer for that, which would fail but was not checked, and hand the nullptr to ReadMemory.  lldb would continue on until it tried to copy bytes into the nullptr and crash.

Fixing the script was obvious, but having lldb crash out from under the script made it a little harder to debug.  I think we should check for a null buffer at the SBProcess API point and return an error there.

Added a quick test to the ProcessAPI test.  This call will cause a huge malloc which should safely fail on every platform I think?  We may need to revise if this ends up crashing lldb on some target I guess.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D143012

Files:
  lldb/source/API/SBProcess.cpp
  lldb/test/API/python_api/process/TestProcessAPI.py


Index: lldb/test/API/python_api/process/TestProcessAPI.py
===================================================================
--- lldb/test/API/python_api/process/TestProcessAPI.py
+++ lldb/test/API/python_api/process/TestProcessAPI.py
@@ -60,6 +60,20 @@
             exe=False,
             startstr=b'x')
 
+        # Try to read an impossibly large amount of memory; swig
+        # will try to malloc it and fail, we should get an error 
+        # result.
+        error = lldb.SBError()
+        content = process.ReadMemory(
+                val.AddressOf().GetValueAsUnsigned(), 
+                0xffffffffffffffe8, error)
+        if error.Success():
+            self.fail("SBProcessReadMemory claims to have "
+                      "successfully read 0xffffffffffffffe8 bytes")
+        if self.TraceOn():
+            print("Tried to read 0xffffffffffffffe8 bytes, got error message: ",
+                  error.GetCString())
+
         # Read (char *)my_char_ptr.
         val = frame.FindValue("my_char_ptr", lldb.eValueTypeVariableGlobal)
         self.DebugSBValue(val)
Index: lldb/source/API/SBProcess.cpp
===================================================================
--- lldb/source/API/SBProcess.cpp
+++ lldb/source/API/SBProcess.cpp
@@ -804,6 +804,12 @@
 
   size_t bytes_read = 0;
 
+  if (!dst) {
+    sb_error.SetErrorStringWithFormat(
+        "no buffer provided to read %zu bytes into", dst_len);
+    return 0;
+  }
+
   ProcessSP process_sp(GetSP());
 
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D143012.493722.patch
Type: text/x-patch
Size: 1491 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20230131/d0c48c57/attachment.bin>