Browse Source

fix: potential crash caused by dlopen different gtk libraries (#33813)

Co-authored-by: deepak1556 <[email protected]>
trop[bot] 3 years ago
parent
commit
a4308f6ca5

+ 34 - 25
BUILD.gn

@@ -43,6 +43,7 @@ if (is_mac) {
 
 if (is_linux) {
   import("//build/config/linux/pkg_config.gni")
+  import("//tools/generate_stubs/rules.gni")
 
   pkg_config("gio_unix") {
     packages = [ "gio-unix-2.0" ]
@@ -54,6 +55,38 @@ if (is_linux) {
       "gdk-pixbuf-2.0",
     ]
   }
+
+  generate_library_loader("libnotify_loader") {
+    name = "LibNotifyLoader"
+    output_h = "libnotify_loader.h"
+    output_cc = "libnotify_loader.cc"
+    header = "<libnotify/notify.h>"
+    config = ":libnotify_config"
+
+    functions = [
+      "notify_is_initted",
+      "notify_init",
+      "notify_get_server_caps",
+      "notify_get_server_info",
+      "notify_notification_new",
+      "notify_notification_add_action",
+      "notify_notification_set_image_from_pixbuf",
+      "notify_notification_set_timeout",
+      "notify_notification_set_urgency",
+      "notify_notification_set_hint_string",
+      "notify_notification_show",
+      "notify_notification_close",
+    ]
+  }
+
+  generate_stubs("electron_gtk_stubs") {
+    sigs = [ "shell/browser/ui/electron_gtk.sigs" ]
+    extra_header = "shell/browser/ui/electron_gtk.fragment"
+    output_name = "electron_gtk_stubs"
+    public_deps = [ "//ui/gtk:gtk_config" ]
+    logging_function = "LogNoop()"
+    logging_include = "ui/gtk/log_noop.h"
+  }
 }
 
 declare_args() {
@@ -253,31 +286,6 @@ copy("copy_shell_devtools_discovery_page") {
   outputs = [ "$target_gen_dir/shell_devtools_discovery_page.html" ]
 }
 
-if (is_linux) {
-  generate_library_loader("libnotify_loader") {
-    name = "LibNotifyLoader"
-    output_h = "libnotify_loader.h"
-    output_cc = "libnotify_loader.cc"
-    header = "<libnotify/notify.h>"
-    config = ":libnotify_config"
-
-    functions = [
-      "notify_is_initted",
-      "notify_init",
-      "notify_get_server_caps",
-      "notify_get_server_info",
-      "notify_notification_new",
-      "notify_notification_add_action",
-      "notify_notification_set_image_from_pixbuf",
-      "notify_notification_set_timeout",
-      "notify_notification_set_urgency",
-      "notify_notification_set_hint_string",
-      "notify_notification_show",
-      "notify_notification_close",
-    ]
-  }
-}
-
 npm_action("electron_version_args") {
   script = "generate-version-json"
 
@@ -537,6 +545,7 @@ source_set("electron_lib") {
   if (is_linux) {
     libs = [ "xshmfence" ]
     deps += [
+      ":electron_gtk_stubs",
       ":libnotify_loader",
       "//build/config/linux/gtk",
       "//dbus",

+ 1 - 1
patches/chromium/.patches

@@ -66,7 +66,6 @@ feat_enable_offscreen_rendering_with_viz_compositor.patch
 gpu_notify_when_dxdiag_request_fails.patch
 feat_allow_embedders_to_add_observers_on_created_hunspell.patch
 feat_add_onclose_to_messageport.patch
-ui_gtk_public_header.patch
 allow_in-process_windows_to_have_different_web_prefs.patch
 refactor_expose_cursor_changes_to_the_webcontentsobserver.patch
 crash_allow_setting_more_options.patch
@@ -115,3 +114,4 @@ build_disable_partition_alloc_on_mac.patch
 fix_non-client_mouse_tracking_and_message_bubbling_on_windows.patch
 build_make_libcxx_abi_unstable_false_for_electron.patch
 introduce_ozoneplatform_electron_can_call_x11_property.patch
+make_gtk_getlibgtk_public.patch

+ 52 - 0
patches/chromium/make_gtk_getlibgtk_public.patch

@@ -0,0 +1,52 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: deepak1556 <[email protected]>
+Date: Thu, 7 Apr 2022 20:30:16 +0900
+Subject: Make gtk::GetLibGtk public
+
+Allows embedders to get a handle to the gtk library
+already loaded in the process.
+
+diff --git a/ui/gtk/gtk_compat.cc b/ui/gtk/gtk_compat.cc
+index 7104a0d5f4489c687f3cb9e63bc7cbef59d2fa62..f0b9ed834a3f7310da09377f0b2105cf635ffeb7 100644
+--- a/ui/gtk/gtk_compat.cc
++++ b/ui/gtk/gtk_compat.cc
+@@ -86,12 +86,6 @@ void* GetLibGtk4(bool check = true) {
+   return libgtk4;
+ }
+ 
+-void* GetLibGtk() {
+-  if (GtkCheckVersion(4))
+-    return GetLibGtk4();
+-  return GetLibGtk3();
+-}
+-
+ bool LoadGtk3() {
+   if (!GetLibGtk3(false))
+     return false;
+@@ -133,6 +127,12 @@ gfx::Insets InsetsFromGtkBorder(const GtkBorder& border) {
+ 
+ }  // namespace
+ 
++void* GetLibGtk() {
++  if (GtkCheckVersion(4))
++    return GetLibGtk4();
++  return GetLibGtk3();
++}
++
+ bool LoadGtk() {
+   static bool loaded = LoadGtkImpl();
+   return loaded;
+diff --git a/ui/gtk/gtk_compat.h b/ui/gtk/gtk_compat.h
+index 72981270fe26579211afcaf3c596a412f69f5fac..b5dbfde5b011d57d26960d245e0dc61cac9341e4 100644
+--- a/ui/gtk/gtk_compat.h
++++ b/ui/gtk/gtk_compat.h
+@@ -37,6 +37,9 @@ using SkColor = uint32_t;
+ 
+ namespace gtk {
+ 
++// Get handle to the currently loaded gtk library in the process.
++void* GetLibGtk();
++
+ // Loads libgtk and related libraries and returns true on success.
+ bool LoadGtk();
+ 

+ 0 - 38
patches/chromium/ui_gtk_public_header.patch

@@ -1,38 +0,0 @@
-From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
-From: deepak1556 <[email protected]>
-Date: Fri, 10 Apr 2020 17:47:18 -0700
-Subject: ui_gtk_public_header.patch
-
-Allow electron to depend on gtk_util.h and gtk_ui.h from //ui/gtk/
-
-diff --git a/ui/gtk/BUILD.gn b/ui/gtk/BUILD.gn
-index 4093df78da0bbb1d8df743942f364cf728ad3414..2f7c404307bfebb0e2890148cf9b0d6d9c68094f 100644
---- a/ui/gtk/BUILD.gn
-+++ b/ui/gtk/BUILD.gn
-@@ -69,7 +69,11 @@ generate_stubs("gtk_stubs") {
- }
- 
- component("gtk") {
--  public = [ "gtk_ui_factory.h" ]
-+  public = [
-+    "gtk_ui.h",
-+    "gtk_ui_factory.h",
-+    "gtk_util.h",
-+  ]
- 
-   sources = [
-     "gtk_color_mixers.cc",
-@@ -79,13 +83,11 @@ component("gtk") {
-     "gtk_key_bindings_handler.cc",
-     "gtk_key_bindings_handler.h",
-     "gtk_ui.cc",
--    "gtk_ui.h",
-     "gtk_ui_factory.cc",
-     "gtk_ui_platform.h",
-     "gtk_ui_platform_stub.cc",
-     "gtk_ui_platform_stub.h",
-     "gtk_util.cc",
--    "gtk_util.h",
-     "input_method_context_impl_gtk.cc",
-     "input_method_context_impl_gtk.h",
-     "native_theme_gtk.cc",

+ 1 - 1
shell/browser/browser_linux.cc

@@ -17,7 +17,7 @@
 
 #if BUILDFLAG(IS_LINUX)
 #include "shell/browser/linux/unity_service.h"
-#include "ui/gtk/gtk_util.h"
+#include "ui/gtk/gtk_util.h"  // nogncheck
 #endif
 
 namespace electron {

+ 10 - 2
shell/browser/electron_browser_main_parts.cc

@@ -69,11 +69,13 @@
 #include "base/threading/thread_task_runner_handle.h"
 #include "device/bluetooth/bluetooth_adapter_factory.h"
 #include "device/bluetooth/dbus/dbus_bluez_manager_wrapper_linux.h"
+#include "electron/electron_gtk_stubs.h"
 #include "ui/base/cursor/cursor_factory.h"
 #include "ui/base/ime/linux/linux_input_method_context_factory.h"
 #include "ui/gfx/color_utils.h"
-#include "ui/gtk/gtk_ui_factory.h"
-#include "ui/gtk/gtk_util.h"
+#include "ui/gtk/gtk_compat.h"      // nogncheck
+#include "ui/gtk/gtk_ui_factory.h"  // nogncheck
+#include "ui/gtk/gtk_util.h"        // nogncheck
 #include "ui/ozone/public/ozone_platform.h"
 #include "ui/views/linux_ui/linux_ui.h"
 #endif
@@ -367,6 +369,12 @@ void ElectronBrowserMainParts::ToolkitInitialized() {
   linux_ui->Initialize();
   DCHECK(ui::LinuxInputMethodContextFactory::instance());
 
+  // Try loading gtk symbols used by Electron.
+  electron::InitializeElectron_gtk(gtk::GetLibGtk());
+  if (!electron::IsElectron_gtkInitialized()) {
+    electron::UninitializeElectron_gtk();
+  }
+
   // Chromium does not respect GTK dark theme setting, but they may change
   // in future and this code might be no longer needed. Check the Chromium
   // issue to keep updated:

+ 1 - 1
shell/browser/notifications/linux/libnotify_notification.cc

@@ -16,7 +16,7 @@
 #include "shell/common/application_info.h"
 #include "shell/common/platform_util.h"
 #include "third_party/skia/include/core/SkBitmap.h"
-#include "ui/gtk/gtk_util.h"
+#include "ui/gtk/gtk_util.h"  // nogncheck
 
 namespace electron {
 

+ 1 - 0
shell/browser/ui/electron_gtk.fragment

@@ -0,0 +1 @@
+#include <gtk/gtk.h>

+ 6 - 0
shell/browser/ui/electron_gtk.sigs

@@ -0,0 +1,6 @@
+GtkFileChooserNative* gtk_file_chooser_native_new(const gchar* title, GtkWindow* parent, GtkFileChooserAction action, const gchar* accept_label, const gchar* cancel_label);
+void gtk_native_dialog_set_modal(GtkNativeDialog* self, gboolean modal);
+void gtk_native_dialog_show(GtkNativeDialog* self);
+void gtk_native_dialog_hide(GtkNativeDialog* self);
+gint gtk_native_dialog_run(GtkNativeDialog* self);
+void gtk_native_dialog_destroy(GtkNativeDialog* self);

+ 18 - 109
shell/browser/ui/file_dialog_gtk.cc

@@ -2,8 +2,6 @@
 // Use of this source code is governed by the MIT license that can be
 // found in the LICENSE file.
 
-#include <gmodule.h>
-
 #include <memory>
 #include <string>
 
@@ -11,6 +9,7 @@
 #include "base/files/file_util.h"
 #include "base/strings/string_util.h"
 #include "base/threading/thread_restrictions.h"
+#include "electron/electron_gtk_stubs.h"
 #include "shell/browser/javascript_environment.h"
 #include "shell/browser/native_window_views.h"
 #include "shell/browser/ui/file_dialog.h"
@@ -18,32 +17,11 @@
 #include "shell/browser/unresponsive_suppressor.h"
 #include "shell/common/gin_converters/file_path_converter.h"
 #include "ui/base/glib/glib_signal.h"
-#include "ui/gtk/gtk_ui.h"
-#include "ui/gtk/gtk_util.h"
+#include "ui/gtk/gtk_ui.h"    // nogncheck
+#include "ui/gtk/gtk_util.h"  // nogncheck
 
 namespace file_dialog {
 
-static GModule* gtk_module;
-static absl::optional<bool> supports_gtk_file_chooser_native;
-
-using dl_gtk_native_dialog_show_t = void (*)(void*);
-using dl_gtk_native_dialog_destroy_t = void (*)(void*);
-using dl_gtk_native_dialog_set_modal_t = void (*)(void*, gboolean);
-using dl_gtk_native_dialog_run_t = int (*)(void*);
-using dl_gtk_native_dialog_hide_t = void (*)(void*);
-using dl_gtk_file_chooser_native_new_t = void* (*)(const char*,
-                                                   GtkWindow*,
-                                                   GtkFileChooserAction,
-                                                   const char*,
-                                                   const char*);
-
-static dl_gtk_native_dialog_show_t dl_gtk_native_dialog_show;
-static dl_gtk_native_dialog_destroy_t dl_gtk_native_dialog_destroy;
-static dl_gtk_native_dialog_set_modal_t dl_gtk_native_dialog_set_modal;
-static dl_gtk_native_dialog_run_t dl_gtk_native_dialog_run;
-static dl_gtk_native_dialog_hide_t dl_gtk_native_dialog_hide;
-static dl_gtk_file_chooser_native_new_t dl_gtk_file_chooser_native_new;
-
 DialogSettings::DialogSettings() = default;
 DialogSettings::DialogSettings(const DialogSettings&) = default;
 DialogSettings::~DialogSettings() = default;
@@ -53,85 +31,16 @@ namespace {
 static const int kPreviewWidth = 256;
 static const int kPreviewHeight = 512;
 
-void InitGtkFileChooserNativeSupport() {
-  // Return early if we have already setup the native functions or we have tried
-  // once before and failed. Avoid running expensive dynamic library operations.
-  if (supports_gtk_file_chooser_native) {
-    return;
-  }
-
-  // Mark that we have attempted to initialize support at least once
-  supports_gtk_file_chooser_native = false;
-
-  if (!g_module_supported()) {
-    return;
-  }
-
-  gtk_module = g_module_open("libgtk-3.so", G_MODULE_BIND_LAZY);
-  if (!gtk_module) {
-    return;
-  }
-
-  // Will never be unloaded
-  g_module_make_resident(gtk_module);
-
-  bool found = g_module_symbol(
-      gtk_module, "gtk_file_chooser_native_new",
-      reinterpret_cast<void**>(&dl_gtk_file_chooser_native_new));
-  if (!found) {
-    return;
-  }
-
-  found = g_module_symbol(
-      gtk_module, "gtk_native_dialog_set_modal",
-      reinterpret_cast<void**>(&dl_gtk_native_dialog_set_modal));
-  if (!found) {
-    return;
-  }
-
-  found =
-      g_module_symbol(gtk_module, "gtk_native_dialog_destroy",
-                      reinterpret_cast<void**>(&dl_gtk_native_dialog_destroy));
-  if (!found) {
-    return;
-  }
-
-  found = g_module_symbol(gtk_module, "gtk_native_dialog_show",
-                          reinterpret_cast<void**>(&dl_gtk_native_dialog_show));
-  if (!found) {
-    return;
-  }
-
-  found = g_module_symbol(gtk_module, "gtk_native_dialog_hide",
-                          reinterpret_cast<void**>(&dl_gtk_native_dialog_hide));
-  if (!found) {
-    return;
-  }
-
-  found = g_module_symbol(gtk_module, "gtk_native_dialog_run",
-                          reinterpret_cast<void**>(&dl_gtk_native_dialog_run));
-  if (!found) {
-    return;
-  }
-
-  supports_gtk_file_chooser_native =
-      dl_gtk_file_chooser_native_new && dl_gtk_native_dialog_set_modal &&
-      dl_gtk_native_dialog_destroy && dl_gtk_native_dialog_run &&
-      dl_gtk_native_dialog_show && dl_gtk_native_dialog_hide;
-}
-
 class FileChooserDialog {
  public:
   FileChooserDialog(GtkFileChooserAction action, const DialogSettings& settings)
       : parent_(
             static_cast<electron::NativeWindowViews*>(settings.parent_window)),
         filters_(settings.filters) {
-    InitGtkFileChooserNativeSupport();
-
     auto label = settings.button_label;
 
-    if (*supports_gtk_file_chooser_native) {
-      dialog_ = GTK_FILE_CHOOSER(dl_gtk_file_chooser_native_new(
+    if (electron::IsElectron_gtkInitialized()) {
+      dialog_ = GTK_FILE_CHOOSER(gtk_file_chooser_native_new(
           settings.title.c_str(), NULL, action,
           label.empty() ? nullptr : label.c_str(), nullptr));
     } else {
@@ -150,8 +59,8 @@ class FileChooserDialog {
 
     if (parent_) {
       parent_->SetEnabled(false);
-      if (*supports_gtk_file_chooser_native) {
-        dl_gtk_native_dialog_set_modal(static_cast<void*>(dialog_), TRUE);
+      if (electron::IsElectron_gtkInitialized()) {
+        gtk_native_dialog_set_modal(GTK_NATIVE_DIALOG(dialog_), TRUE);
       } else {
         gtk::SetGtkTransientForAura(GTK_WIDGET(dialog_),
                                     parent_->GetNativeWindow());
@@ -188,7 +97,7 @@ class FileChooserDialog {
     // org.freedesktop.portal.FileChooser portal. In the case of running through
     // the org.freedesktop.portal.FileChooser portal, anything having to do with
     // the update-preview signal or the preview widget will just be ignored.
-    if (!*supports_gtk_file_chooser_native) {
+    if (!electron::IsElectron_gtkInitialized()) {
       preview_ = gtk_image_new();
       g_signal_connect(dialog_, "update-preview",
                        G_CALLBACK(OnUpdatePreviewThunk), this);
@@ -197,8 +106,8 @@ class FileChooserDialog {
   }
 
   ~FileChooserDialog() {
-    if (*supports_gtk_file_chooser_native) {
-      dl_gtk_native_dialog_destroy(static_cast<void*>(dialog_));
+    if (electron::IsElectron_gtkInitialized()) {
+      gtk_native_dialog_destroy(GTK_NATIVE_DIALOG(dialog_));
     } else {
       gtk_widget_destroy(GTK_WIDGET(dialog_));
     }
@@ -236,8 +145,8 @@ class FileChooserDialog {
   void RunAsynchronous() {
     g_signal_connect(dialog_, "response", G_CALLBACK(OnFileDialogResponseThunk),
                      this);
-    if (*supports_gtk_file_chooser_native) {
-      dl_gtk_native_dialog_show(static_cast<void*>(dialog_));
+    if (electron::IsElectron_gtkInitialized()) {
+      gtk_native_dialog_show(GTK_NATIVE_DIALOG(dialog_));
     } else {
       gtk_widget_show_all(GTK_WIDGET(dialog_));
       gtk::GtkUi::GetPlatform()->ShowGtkWindow(GTK_WINDOW(dialog_));
@@ -305,8 +214,8 @@ class FileChooserDialog {
 };
 
 void FileChooserDialog::OnFileDialogResponse(GtkWidget* widget, int response) {
-  if (*supports_gtk_file_chooser_native) {
-    dl_gtk_native_dialog_hide(static_cast<void*>(dialog_));
+  if (electron::IsElectron_gtkInitialized()) {
+    gtk_native_dialog_hide(GTK_NATIVE_DIALOG(dialog_));
   } else {
     gtk_widget_hide(GTK_WIDGET(dialog_));
   }
@@ -371,7 +280,7 @@ bool CanPreview(const struct stat& st) {
 }
 
 void FileChooserDialog::OnUpdatePreview(GtkFileChooser* chooser) {
-  CHECK(!*supports_gtk_file_chooser_native);
+  CHECK(!electron::IsElectron_gtkInitialized());
   gchar* filename = gtk_file_chooser_get_preview_filename(chooser);
   if (!filename) {
     gtk_file_chooser_set_preview_widget_active(chooser, FALSE);
@@ -400,15 +309,15 @@ void FileChooserDialog::OnUpdatePreview(GtkFileChooser* chooser) {
 
 void ShowFileDialog(const FileChooserDialog& dialog) {
   // gtk_native_dialog_run() will call gtk_native_dialog_show() for us.
-  if (!*supports_gtk_file_chooser_native) {
+  if (!electron::IsElectron_gtkInitialized()) {
     gtk_widget_show_all(GTK_WIDGET(dialog.dialog()));
   }
 }
 
 int RunFileDialog(const FileChooserDialog& dialog) {
   int response = 0;
-  if (*supports_gtk_file_chooser_native) {
-    response = dl_gtk_native_dialog_run(static_cast<void*>(dialog.dialog()));
+  if (electron::IsElectron_gtkInitialized()) {
+    response = gtk_native_dialog_run(GTK_NATIVE_DIALOG(dialog.dialog()));
   } else {
     response = gtk_dialog_run(GTK_DIALOG(dialog.dialog()));
   }

+ 2 - 2
shell/browser/ui/message_box_gtk.cc

@@ -18,8 +18,8 @@
 #include "shell/browser/unresponsive_suppressor.h"
 #include "ui/base/glib/glib_signal.h"
 #include "ui/gfx/image/image_skia.h"
-#include "ui/gtk/gtk_ui.h"
-#include "ui/gtk/gtk_util.h"
+#include "ui/gtk/gtk_ui.h"    // nogncheck
+#include "ui/gtk/gtk_util.h"  // nogncheck
 
 #if defined(USE_OZONE)
 #include "ui/base/ui_base_features.h"

+ 1 - 1
shell/browser/ui/views/client_frame_view_linux.cc

@@ -22,7 +22,7 @@
 #include "ui/gfx/skia_util.h"
 #include "ui/gfx/text_constants.h"
 #include "ui/gtk/gtk_compat.h"  // nogncheck
-#include "ui/gtk/gtk_util.h"
+#include "ui/gtk/gtk_util.h"    // nogncheck
 #include "ui/native_theme/native_theme.h"
 #include "ui/strings/grit/ui_strings.h"
 #include "ui/views/controls/button/image_button.h"

+ 1 - 1
shell/browser/ui/views/menu_bar.cc

@@ -14,7 +14,7 @@
 #include "ui/views/layout/box_layout.h"
 
 #if BUILDFLAG(IS_LINUX)
-#include "ui/gtk/gtk_util.h"
+#include "ui/gtk/gtk_util.h"  // nogncheck
 #endif
 
 #if BUILDFLAG(IS_WIN)

+ 1 - 1
shell/common/application_info_linux.cc

@@ -13,7 +13,7 @@
 #include "base/logging.h"
 #include "electron/electron_version.h"
 #include "shell/common/platform_util.h"
-#include "ui/gtk/gtk_util.h"
+#include "ui/gtk/gtk_util.h"  // nogncheck
 
 namespace {
 

+ 1 - 1
shell/common/platform_util_linux.cc

@@ -28,7 +28,7 @@
 #include "dbus/object_proxy.h"
 #include "shell/common/platform_util_internal.h"
 #include "third_party/abseil-cpp/absl/types/optional.h"
-#include "ui/gtk/gtk_util.h"
+#include "ui/gtk/gtk_util.h"  // nogncheck
 #include "url/gurl.h"
 
 #define ELECTRON_TRASH "ELECTRON_TRASH"