[lldb/Plugins] Fix ScriptedInterface object ptr use-after-free

[mib]

Summary

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 <[e-mail]>

[labath]

lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.cpp

54

This doesn't sound right. This object (StructuredPythonObject instance) is definitely not created by python and will now be leaked. If I correctly understand the problem, the issue is that the this object gets a non-owning reference (the ret_val argument) to the underlying python object, and then frees it as if it was owning it. If that's the case, then the solution is to INCREF it in the constructor (or switch to using a PythonObject wrapper, which will then handle the lifetime management.

You may also be interested in D114722 (which I hope to update soon). It's not _directly_ related to this, but it touches the same parts of the code.

[labath]

lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.cpp

54

So, as far as I can tell ret_val is an owned reference (in LLDBSwigPythonCreateScriptedThread, it comes from PythonObject.release()). Could it be that something else is freeing (decreffing) the object more times than it should (thereby releasing the references that are supposed to be held here) and this code gets blamed/crashes just because it happens to run last?

[JDevlieghere]

lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.cpp

54

Pavel said it better than I would have :-)

[mib]

Abandoning this in favor of D117139.

lldb/include/lldb/Interpreter/ScriptedInterface.h

 #include <string>
 
 namespace lldb_private {
 class ScriptedInterface {
 public:
   ScriptedInterface() = default;
   virtual ~ScriptedInterface() = default;
 
-  virtual StructuredData::GenericSP
+  virtual StructuredData::Generic *
   CreatePluginObject(llvm::StringRef class_name, ExecutionContext &exe_ctx,
                      StructuredData::DictionarySP args_sp) = 0;
 
   template <typename Ret>
   Ret ErrorWithMessage(llvm::StringRef caller_name, llvm::StringRef error_msg,
                        Status &error,
                        uint32_t log_caterogy = LIBLLDB_LOG_PROCESS) {
     LLDB_LOGF(GetLogIfAllCategoriesSet(log_caterogy), "%s ERROR = %s",
bool CheckStructuredDataObject(llvm::StringRef caller, T obj, Status &error) {
     }
 
     if (error.Fail())
       return ErrorWithMessage<bool>(caller, error.AsCString(), error);
 
     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/include/lldb/Interpreter/ScriptedProcessInterface.h

 
 #include "lldb/lldb-private.h"
 
 #include <string>
 
 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;
   }
 
   virtual Status Launch() { return Status("ScriptedProcess did not launch"); }
 
   virtual Status Resume() { return Status("ScriptedProcess did not resume"); }
virtual lldb::ScriptedThreadInterfaceSP GetScriptedThreadInterface() {
     return nullptr;
   }
 
   lldb::ScriptedThreadInterfaceSP m_scripted_thread_interface_sp = nullptr;
 };
 
 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;
   }
 
   virtual lldb::tid_t GetThreadID() { return LLDB_INVALID_THREAD_ID; }
 
   virtual llvm::Optional<std::string> GetName() { return llvm::None; }

lldb/source/Plugins/Process/scripted/ScriptedProcess.h

private:
 
   void CheckInterpreterAndScriptObject() const;
   ScriptedProcessInterface &GetInterface() const;
   static bool IsScriptLanguageSupported(lldb::ScriptLanguage language);
 
   // 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
 
 #endif // LLDB_SOURCE_PLUGINS_SCRIPTED_PROCESS_H

lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp

bool ScriptedProcess::IsScriptLanguageSupported(lldb::ScriptLanguage language) {
   llvm::ArrayRef<lldb::ScriptLanguage> supported_languages =
       llvm::makeArrayRef(g_supported_script_languages);
 
   return llvm::is_contained(supported_languages, language);
 }
 
 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,
                                                 lldb::ListenerSP listener_sp,
                                                 const FileSpec *file,
                                                 bool can_connect) {
   if (!target_sp ||
       !IsScriptLanguageSupported(target_sp->GetDebugger().GetScriptLanguage()))
     return nullptr;
 
   Status error;
   ScriptedProcess::ScriptedProcessInfo scripted_process_info(
       target_sp->GetProcessLaunchInfo());
 
   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;
   }
 
   return process_sp;
 }
 
if (!m_interpreter) {
     error.SetErrorStringWithFormat("ScriptedProcess::%s () - ERROR: %s",
                                    __FUNCTION__,
                                    "Debugger has no Script Interpreter");
     return;
   }
 
   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() {
   Clear();
   // We need to call finalize on the process before destroying ourselves to
   // make sure all of the broadcaster cleanup goes as planned. If we destruct
   // this class, then Process::~Process() might have problems trying to fully
   // destroy the broadcaster.
Status ScriptedProcess::DoStop() {
 
   LLDB_LOGF(log, "ScriptedProcess::%s Delayed stop", __FUNCTION__);
   return GetInterface().Stop();
 }
 
 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;
 }
 
 size_t ScriptedProcess::DoReadMemory(lldb::addr_t addr, void *buf, size_t size,
                                      Status &error) {
   if (!m_interpreter)
     return GetInterface().ErrorWithMessage<size_t>(LLVM_PRETTY_FUNCTION,

lldb/source/Plugins/Process/scripted/ScriptedThread.h

private:
 
   ScriptedThread(const ScriptedThread &) = delete;
   const ScriptedThread &operator=(const ScriptedThread &) = delete;
 
   std::shared_ptr<DynamicRegisterInfo> GetDynamicRegisterInfo();
 
   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
 
 #endif // LLDB_SOURCE_PLUGINS_SCRIPTED_THREAD_H

lldb/source/Plugins/Process/scripted/ScriptedThread.cpp

 #include "lldb/Utility/Logging.h"
 
 #include <memory>
 
 using namespace lldb;
 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.");
 }
 
 ScriptedThread::ScriptedThread(ScriptedProcess &process, Status &error)
     : Thread(process, LLDB_INVALID_THREAD_ID), m_scripted_process(process) {
   if (!process.IsValid()) {
     error.SetErrorString("Invalid scripted process");
     return;
llvm::Optional<std::string> class_name =
       process.GetInterface().GetScriptedThreadPluginName();
   if (!class_name || class_name->empty()) {
     error.SetErrorString("Failed to get scripted thread class name.");
     return;
   }
 
   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());
 }
 
 ScriptedThread::~ScriptedThread() { DestroyThread(); }
 
 const char *ScriptedThread::GetName() {
   CheckInterpreterAndScriptObject();

lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.h

 #include "lldb/Interpreter/ScriptedProcessInterface.h"
 
 namespace lldb_private {
 class ScriptedProcessPythonInterface : public ScriptedProcessInterface,
                                        public ScriptedPythonInterface {
 public:
   ScriptedProcessPythonInterface(ScriptInterpreterPythonImpl &interpreter);
 
-  StructuredData::GenericSP
+  StructuredData::Generic *
   CreatePluginObject(const llvm::StringRef class_name,
                      ExecutionContext &exe_ctx,
                      StructuredData::DictionarySP args_sp) override;
 
   Status Launch() override;
 
   Status Resume() override;
 

lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp

 using namespace lldb_private;
 using namespace lldb_private::python;
 using Locker = ScriptInterpreterPythonImpl::Locker;
 
 ScriptedProcessPythonInterface::ScriptedProcessPythonInterface(
     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())
     return {};
 
   TargetSP target_sp = exe_ctx.GetTargetSP();
   StructuredDataImpl args_impl(args_sp);
   std::string error_string;
 
   Locker py_lock(&m_interpreter, Locker::AcquireLock | Locker::NoSTDIN,
                  Locker::FreeLock);
 
   void *ret_val = LLDBSwigPythonCreateScriptedProcess(
       class_name.str().c_str(), m_interpreter.GetDictionaryName(), target_sp,
       args_impl, error_string);
 
   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() {
   return GetStatusFromMethod("launch");
 }
 
 Status ScriptedProcessPythonInterface::Resume() {
   return GetStatusFromMethod("resume");

lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h

protected:
   T Dispatch(llvm::StringRef method_name, Status &error, Args... args) {
     using namespace python;
     using Locker = ScriptInterpreterPythonImpl::Locker;
 
     std::string caller_signature =
         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,
                                  "Python implementor not allocated.", error);
 
     PythonObject pmeth(
         PyRefType::Owned,
         PyObject_GetAttrString(implementor.get(), method_name.str().c_str()));

lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.h

 #include "lldb/Interpreter/ScriptedProcessInterface.h"
 
 namespace lldb_private {
 class ScriptedThreadPythonInterface : public ScriptedThreadInterface,
                                       public ScriptedPythonInterface {
 public:
   ScriptedThreadPythonInterface(ScriptInterpreterPythonImpl &interpreter);
 
-  StructuredData::GenericSP
+  StructuredData::Generic *
   CreatePluginObject(llvm::StringRef class_name, ExecutionContext &exe_ctx,
                      StructuredData::DictionarySP args_sp) override;
 
   lldb::tid_t GetThreadID() override;
 
   llvm::Optional<std::string> GetName() override;
 
   lldb::StateType GetState() override;

lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.cpp

 using namespace lldb_private;
 using namespace lldb_private::python;
 using Locker = ScriptInterpreterPythonImpl::Locker;
 
 ScriptedThreadPythonInterface::ScriptedThreadPythonInterface(
     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) {
 
   if (class_name.empty())
     return {};
 
   ProcessSP process_sp = exe_ctx.GetProcessSP();
   StructuredDataImpl args_impl(args_sp);
   std::string error_string;
 
   Locker py_lock(&m_interpreter, Locker::AcquireLock | Locker::NoSTDIN,
                  Locker::FreeLock);
 
   void *ret_val = LLDBSwigPythonCreateScriptedThread(
       class_name.str().c_str(), m_interpreter.GetDictionaryName(), process_sp,
       args_impl, error_string);
 
   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() {
   Status error;
   StructuredData::ObjectSP obj = Dispatch("get_thread_id", error);
 
   if (!CheckStructuredDataObject(LLVM_PRETTY_FUNCTION, obj, error))
     return LLDB_INVALID_THREAD_ID;