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

This patch replaces all the ScriptedInterface object instance shared
pointer by a raw pointer. The reason behind the change is that when the
smart pointer gets re-assigned, that triggers calling the default
deleter to the previously pointer object.

However, in this case, the pointed memory was allocated in Python, so
when another object tries to read it, it causes a heap-use-after-free.

By switching to a raw pointer, it prevents lldb from decrementing the
reference counting to 0 and calling the deleter for that object.

rdar://87425859

Signed-off-by: Med Ismail Bennani <medismail.benn...@gmail.com>


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117065

Files:
  lldb/include/lldb/Interpreter/ScriptedInterface.h
  lldb/include/lldb/Interpreter/ScriptedProcessInterface.h
  lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
  lldb/source/Plugins/Process/scripted/ScriptedProcess.h
  lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
  lldb/source/Plugins/Process/scripted/ScriptedThread.h
  
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.cpp
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.h

Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.h
===================================================================
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.h
@@ -22,7 +22,7 @@
 public:
   ScriptedThreadPythonInterface(ScriptInterpreterPythonImpl &interpreter);
 
-  StructuredData::GenericSP
+  StructuredData::Generic *
   CreatePluginObject(llvm::StringRef class_name, ExecutionContext &exe_ctx,
                      StructuredData::DictionarySP args_sp) override;
 
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.cpp
===================================================================
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.cpp
@@ -29,7 +29,7 @@
     ScriptInterpreterPythonImpl &interpreter)
     : ScriptedThreadInterface(), ScriptedPythonInterface(interpreter) {}
 
-StructuredData::GenericSP ScriptedThreadPythonInterface::CreatePluginObject(
+StructuredData::Generic *ScriptedThreadPythonInterface::CreatePluginObject(
     const llvm::StringRef class_name, ExecutionContext &exe_ctx,
     StructuredData::DictionarySP args_sp) {
 
@@ -50,10 +50,10 @@
   if (!ret_val)
     return {};
 
-  m_object_instance_sp =
-      StructuredData::GenericSP(new StructuredPythonObject(ret_val));
+  m_object_instance = static_cast<StructuredData::Generic *>(
+      new StructuredPythonObject(ret_val));
 
-  return m_object_instance_sp;
+  return m_object_instance;
 }
 
 lldb::tid_t ScriptedThreadPythonInterface::GetThreadID() {
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h
===================================================================
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h
@@ -42,15 +42,16 @@
         llvm::Twine(LLVM_PRETTY_FUNCTION + llvm::Twine(" (") +
                     llvm::Twine(method_name) + llvm::Twine(")"))
             .str();
-    if (!m_object_instance_sp)
-      return ErrorWithMessage<T>(caller_signature, "Python object ill-formed",
-                                 error);
 
     Locker py_lock(&m_interpreter, Locker::AcquireLock | Locker::NoSTDIN,
                    Locker::FreeLock);
 
+    if (!m_object_instance && m_object_instance->IsValid())
+      return ErrorWithMessage<T>(caller_signature, "Python object ill-formed",
+                                 error);
+
     PythonObject implementor(PyRefType::Borrowed,
-                             (PyObject *)m_object_instance_sp->GetValue());
+                             (PyObject *)m_object_instance->GetValue());
 
     if (!implementor.IsAllocated())
       return ErrorWithMessage<T>(caller_signature,
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.h
===================================================================
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.h
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.h
@@ -22,7 +22,7 @@
 public:
   ScriptedProcessPythonInterface(ScriptInterpreterPythonImpl &interpreter);
 
-  StructuredData::GenericSP
+  StructuredData::Generic *
   CreatePluginObject(const llvm::StringRef class_name,
                      ExecutionContext &exe_ctx,
                      StructuredData::DictionarySP args_sp) override;
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
===================================================================
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp
@@ -30,7 +30,7 @@
     ScriptInterpreterPythonImpl &interpreter)
     : ScriptedProcessInterface(), ScriptedPythonInterface(interpreter) {}
 
-StructuredData::GenericSP ScriptedProcessPythonInterface::CreatePluginObject(
+StructuredData::Generic *ScriptedProcessPythonInterface::CreatePluginObject(
     llvm::StringRef class_name, ExecutionContext &exe_ctx,
     StructuredData::DictionarySP args_sp) {
   if (class_name.empty())
@@ -50,10 +50,10 @@
   if (!ret_val)
     return {};
 
-  m_object_instance_sp =
-      StructuredData::GenericSP(new StructuredPythonObject(ret_val));
+  m_object_instance = static_cast<StructuredData::Generic *>(
+      new StructuredPythonObject(ret_val));
 
-  return m_object_instance_sp;
+  return m_object_instance;
 }
 
 Status ScriptedProcessPythonInterface::Launch() {
Index: lldb/source/Plugins/Process/scripted/ScriptedThread.h
===================================================================
--- lldb/source/Plugins/Process/scripted/ScriptedThread.h
+++ lldb/source/Plugins/Process/scripted/ScriptedThread.h
@@ -60,7 +60,7 @@
 
   const ScriptedProcess &m_scripted_process;
   std::shared_ptr<DynamicRegisterInfo> m_register_info_sp = nullptr;
-  lldb_private::StructuredData::ObjectSP m_script_object_sp = nullptr;
+  lldb_private::StructuredData::Generic *m_script_object = nullptr;
 };
 
 } // namespace lldb_private
Index: lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
===================================================================
--- lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
+++ lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
@@ -24,7 +24,7 @@
 using namespace lldb_private;
 
 void ScriptedThread::CheckInterpreterAndScriptObject() const {
-  lldbassert(m_script_object_sp && "Invalid Script Object.");
+  lldbassert(m_script_object && "Invalid Script Object.");
   lldbassert(GetInterface() && "Invalid Scripted Thread Interface.");
 }
 
@@ -52,16 +52,16 @@
 
   ExecutionContext exe_ctx(process);
 
-  StructuredData::GenericSP object_sp =
+  StructuredData::Generic *script_object =
       scripted_thread_interface->CreatePluginObject(
           class_name->c_str(), exe_ctx,
           process.m_scripted_process_info.GetArgsSP());
-  if (!object_sp || !object_sp->IsValid()) {
+  if (!script_object || !script_object->IsValid()) {
     error.SetErrorString("Failed to create valid script object");
     return;
   }
 
-  m_script_object_sp = object_sp;
+  m_script_object = script_object;
 
   SetID(scripted_thread_interface->GetThreadID());
 }
Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.h
===================================================================
--- lldb/source/Plugins/Process/scripted/ScriptedProcess.h
+++ lldb/source/Plugins/Process/scripted/ScriptedProcess.h
@@ -110,8 +110,7 @@
   // Member variables.
   const ScriptedProcessInfo m_scripted_process_info;
   lldb_private::ScriptInterpreter *m_interpreter = nullptr;
-  lldb_private::StructuredData::ObjectSP m_script_object_sp = nullptr;
-  //@}
+  lldb_private::StructuredData::Generic *m_script_object = nullptr;
 };
 
 } // namespace lldb_private
Index: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
===================================================================
--- lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
+++ lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
@@ -46,7 +46,7 @@
 
 void ScriptedProcess::CheckInterpreterAndScriptObject() const {
   lldbassert(m_interpreter && "Invalid Script Interpreter.");
-  lldbassert(m_script_object_sp && "Invalid Script Object.");
+  lldbassert(m_script_object && "Invalid Script Object.");
 }
 
 lldb::ProcessSP ScriptedProcess::CreateInstance(lldb::TargetSP target_sp,
@@ -64,8 +64,8 @@
   auto process_sp = std::make_shared<ScriptedProcess>(
       target_sp, listener_sp, scripted_process_info, error);
 
-  if (error.Fail() || !process_sp || !process_sp->m_script_object_sp ||
-      !process_sp->m_script_object_sp->IsValid()) {
+  if (error.Fail() || !process_sp || !process_sp->m_script_object ||
+      !process_sp->m_script_object->IsValid()) {
     LLDB_LOGF(GetLogIfAllCategoriesSet(LIBLLDB_LOG_PROCESS), "%s",
               error.AsCString());
     return nullptr;
@@ -103,18 +103,18 @@
 
   ExecutionContext exe_ctx(target_sp, /*get_process=*/false);
 
-  StructuredData::GenericSP object_sp = GetInterface().CreatePluginObject(
+  StructuredData::Generic *script_object = GetInterface().CreatePluginObject(
       m_scripted_process_info.GetClassName().c_str(), exe_ctx,
       m_scripted_process_info.GetArgsSP());
 
-  if (!object_sp || !object_sp->IsValid()) {
+  if (!script_object || !script_object->IsValid()) {
     error.SetErrorStringWithFormat("ScriptedProcess::%s () - ERROR: %s",
                                    __FUNCTION__,
                                    "Failed to create valid script object");
     return;
   }
 
-  m_script_object_sp = object_sp;
+  m_script_object = script_object;
 }
 
 ScriptedProcess::~ScriptedProcess() {
@@ -217,7 +217,7 @@
 Status ScriptedProcess::DoDestroy() { return Status(); }
 
 bool ScriptedProcess::IsAlive() {
-  if (m_interpreter && m_script_object_sp)
+  if (m_interpreter && m_script_object)
     return GetInterface().IsAlive();
   return false;
 }
Index: lldb/include/lldb/Interpreter/ScriptedProcessInterface.h
===================================================================
--- lldb/include/lldb/Interpreter/ScriptedProcessInterface.h
+++ lldb/include/lldb/Interpreter/ScriptedProcessInterface.h
@@ -21,7 +21,7 @@
 namespace lldb_private {
 class ScriptedProcessInterface : virtual public ScriptedInterface {
 public:
-  StructuredData::GenericSP
+  StructuredData::Generic *
   CreatePluginObject(llvm::StringRef class_name, ExecutionContext &exe_ctx,
                      StructuredData::DictionarySP args_sp) override {
     return nullptr;
@@ -75,7 +75,7 @@
 
 class ScriptedThreadInterface : virtual public ScriptedInterface {
 public:
-  StructuredData::GenericSP
+  StructuredData::Generic *
   CreatePluginObject(llvm::StringRef class_name, ExecutionContext &exe_ctx,
                      StructuredData::DictionarySP args_sp) override {
     return nullptr;
Index: lldb/include/lldb/Interpreter/ScriptedInterface.h
===================================================================
--- lldb/include/lldb/Interpreter/ScriptedInterface.h
+++ lldb/include/lldb/Interpreter/ScriptedInterface.h
@@ -25,7 +25,7 @@
   ScriptedInterface() = default;
   virtual ~ScriptedInterface() = default;
 
-  virtual StructuredData::GenericSP
+  virtual StructuredData::Generic *
   CreatePluginObject(llvm::StringRef class_name, ExecutionContext &exe_ctx,
                      StructuredData::DictionarySP args_sp) = 0;
 
@@ -67,8 +67,12 @@
     return true;
   }
 
+  void SetPluginObject(StructuredData::Generic *object_instance) {
+    m_object_instance = object_instance;
+  }
+
 protected:
-  StructuredData::GenericSP m_object_instance_sp;
+  StructuredData::Generic *m_object_instance;
 };
 } // namespace lldb_private
 #endif // LLDB_INTERPRETER_SCRIPTEDINTERFACE_H
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to