From 7b3a6af7d16cdfd571ede490dffc648a7c80b658 Mon Sep 17 00:00:00 2001
From: Robbert van der Helm <mail@robbertvanderhelm.nl>
Date: Mon, 7 Dec 2020 18:23:04 +0100
Subject: [PATCH] Use raw pointers for the plugin factory

Since the object cleans up after itself after the smart pointers are
dropped on the host side this would result in a use after free by the
smart pointers.
---
 src/plugin/bridges/vst3.cpp | 12 ++++++------
 src/plugin/bridges/vst3.h   |  8 ++++----
 src/plugin/vst3-plugin.cpp  |  3 ---
 3 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/src/plugin/bridges/vst3.cpp b/src/plugin/bridges/vst3.cpp
index 66447d9c..ae2887e5 100644
--- a/src/plugin/bridges/vst3.cpp
+++ b/src/plugin/bridges/vst3.cpp
@@ -86,10 +86,10 @@ Vst3PluginBridge::Vst3PluginBridge()
 Steinberg::IPluginFactory* Vst3PluginBridge::get_plugin_factory() {
     // Even though we're working with raw pointers here, we should pretend that
     // we're `IPtr<Steinberg::IPluginFactory>` and do the reference counting
-    // ourselves because you can't always safely pass those around The VST3
-    // interface can't pass smart pointers around because of binary
-    // compatibility, so we'll have to do the reference counting by hand like in
-    // the implementation in `public.sdk/source/main/pluginfactory.h`.
+    // ourselves. This should work the same was as the standard implementation
+    // in `public.sdk/source/main/pluginfactory.h`. If we were to use an IPtr or
+    // an STL smart pointer we would get a double free (or rather, a use after
+    // free).
     if (plugin_factory) {
         plugin_factory->addRef();
     } else {
@@ -97,11 +97,11 @@ Steinberg::IPluginFactory* Vst3PluginBridge::get_plugin_factory() {
         // will request after loading the module. Host callback handlers should
         // have started before this since the Wine plugin host will request a
         // copy of the configuration during its initialization.
-        plugin_factory = Steinberg::owned(new YaPluginFactoryPluginImpl(*this));
+        plugin_factory = new YaPluginFactoryPluginImpl(*this);
         sockets.host_vst_control.receive_into(
             WantsPluginFactory{}, *plugin_factory,
             std::pair<Vst3Logger&, bool>(logger, true));
     }
 
-    return plugin_factory.get();
+    return plugin_factory;
 }
diff --git a/src/plugin/bridges/vst3.h b/src/plugin/bridges/vst3.h
index 056418e6..0b383f4e 100644
--- a/src/plugin/bridges/vst3.h
+++ b/src/plugin/bridges/vst3.h
@@ -80,11 +80,11 @@ class Vst3PluginBridge : PluginBridge<Vst3Sockets<std::jthread>> {
      * Our plugin factory. All information about the plugin and its supported
      * classes are copied directly from the Windows VST3 plugin's factory on the
      * Wine side, and we'll provide an implementation that can send control
-     * messages to the Wine plugin host. The VST3 interface only passes raw
-     * pointers around and the receiving side should then use `IPtr<T>::adopt()`
-     * or `owned()` to get back the orignal smart pointer.
+     * messages to the Wine plugin host. As explained in `get_plugin_factory()`,
+     * this cannot be a smart pointer because the factory is supposed to free
+     * itself when the host removes its last adopted `IPtr<IpluginFactory>`.
      *
      * @related get_plugin_factory
      */
-    Steinberg::IPtr<YaPluginFactory> plugin_factory;
+    YaPluginFactory* plugin_factory;
 };
diff --git a/src/plugin/vst3-plugin.cpp b/src/plugin/vst3-plugin.cpp
index 75b5e201..3e27c884 100644
--- a/src/plugin/vst3-plugin.cpp
+++ b/src/plugin/vst3-plugin.cpp
@@ -38,9 +38,6 @@ bool InitModule() {
     assert(bridge == nullptr);
 
     try {
-        // This is the only place where we have to use manual memory management.
-        // The bridge's destructor is called when the `effClose` opcode is
-        // received.
         bridge = new Vst3PluginBridge();
 
         return true;