From 454dc266cc0437da6371c65720b722322ef8eecc Mon Sep 17 00:00:00 2001
From: Kramer Ge <[email protected]>
Date: Wed, 24 Sep 2025 19:20:27 -0700
Subject: [PATCH] [Ozone/Wayland]Remove idle_inhibitor_ object from WaylandScreen

..to prevent UAF on chrome shutdown. Unlike most objects created by
WaylandConnection, WaylandScreen is owned by ChromeBrowserMainExtraParts
and outlives WaylandConnection.

To prevent accessing wl_display internals after destruction of
WaylandConnection, WaylandScreen should either destroy wl::Object when
connection resets, or reference wl::Object indirectly.

Manage inhibitor in zwp_idle_inhibit_manager. Also fix a logic in
IsScreenSaverActive() where `inhibitor` mean screen saver is blocked.

Bug: 433027577, 433643249
Change-Id: If02755ddced08f8cf795ac21ed144387d0aa4077
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6979565
Commit-Queue: Kramer Ge <[email protected]>
Reviewed-by: Thomas Anderson <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1520174}
---

diff --git a/ui/ozone/platform/wayland/host/wayland_screen.cc b/ui/ozone/platform/wayland/host/wayland_screen.cc
index c3e6b7df..cc9225dd 100644
--- a/ui/ozone/platform/wayland/host/wayland_screen.cc
+++ b/ui/ozone/platform/wayland/host/wayland_screen.cc
@@ -128,10 +128,7 @@
   }
 }
 
-WaylandScreen::~WaylandScreen() {
-  // Destroy the idle inhibitor early.  See https://crbug.com/433643249
-  idle_inhibitor_.reset();
-}
+WaylandScreen::~WaylandScreen() = default;
 
 void WaylandScreen::OnOutputAddedOrUpdated(
     const WaylandOutput::Metrics& metrics) {
@@ -448,31 +445,19 @@
     return false;
 
   if (suspend) {
-    // Wayland inhibits idle behaviour on certain output, and implies that a
-    // surface bound to that output should obtain the inhibitor and hold it
-    // until it no longer needs to prevent the output to go idle.
-    // We assume that the idle lock is initiated by the user, and therefore the
-    // surface that we should use is the one owned by the window that is focused
-    // currently.
-    const auto* window_manager = connection_->window_manager();
-    DCHECK(window_manager);
-    const auto* current_window = window_manager->GetCurrentFocusedWindow();
-    if (!current_window) {
-      LOG(WARNING) << "Cannot inhibit going idle when no window is focused";
-      return false;
-    }
-    DCHECK(current_window->root_surface());
-    idle_inhibitor_ = connection_->zwp_idle_inhibit_manager()->CreateInhibitor(
-        current_window->root_surface()->surface());
+    connection_->zwp_idle_inhibit_manager()->CreateInhibitor();
   } else {
-    idle_inhibitor_.reset();
+    connection_->zwp_idle_inhibit_manager()->RemoveInhibitor();
   }
 
   return true;
 }
 
 bool WaylandScreen::IsScreenSaverActive() const {
-  return idle_inhibitor_ != nullptr;
+  // idle_inhibitor prevents screen saver from engaging, but does not indicate
+  // whether screen saver is active or not. Assume not here.
+  NOTIMPLEMENTED_LOG_ONCE();
+  return false;
 }
 
 base::TimeDelta WaylandScreen::CalculateIdleTime() const {
diff --git a/ui/ozone/platform/wayland/host/wayland_screen.h b/ui/ozone/platform/wayland/host/wayland_screen.h
index e09aa150..23f0903 100644
--- a/ui/ozone/platform/wayland/host/wayland_screen.h
+++ b/ui/ozone/platform/wayland/host/wayland_screen.h
@@ -38,7 +38,8 @@
 class OrgGnomeMutterIdleMonitor;
 #endif
 
-// A PlatformScreen implementation for Wayland.
+// A PlatformScreen implementation for Wayland. Note that this object outlives
+// WaylandConnection.
 class WaylandScreen : public PlatformScreen, public DeviceScaleFactorObserver {
  public:
   explicit WaylandScreen(WaylandConnection* connection);
@@ -136,7 +137,6 @@
       org_gnome_mutter_idle_monitor_;
 #endif
 
-  wl::Object<zwp_idle_inhibitor_v1> idle_inhibitor_;
   uint32_t screen_saver_suspension_count_ = 0;
 
   base::ScopedObservation<ui::LinuxUi, DeviceScaleFactorObserver>
diff --git a/ui/ozone/platform/wayland/host/zwp_idle_inhibit_manager.cc b/ui/ozone/platform/wayland/host/zwp_idle_inhibit_manager.cc
index 4ca26f5..cd4fdf3 100644
--- a/ui/ozone/platform/wayland/host/zwp_idle_inhibit_manager.cc
+++ b/ui/ozone/platform/wayland/host/zwp_idle_inhibit_manager.cc
@@ -8,6 +8,8 @@
 
 #include "base/logging.h"
 #include "ui/ozone/platform/wayland/host/wayland_connection.h"
+#include "ui/ozone/platform/wayland/host/wayland_surface.h"
+#include "ui/ozone/platform/wayland/host/wayland_window.h"
 
 namespace ui {
 
@@ -45,14 +47,37 @@
 ZwpIdleInhibitManager::ZwpIdleInhibitManager(
     zwp_idle_inhibit_manager_v1* manager,
     WaylandConnection* connection)
-    : manager_(manager) {}
+    : connection_(connection), manager_(manager) {}
 
 ZwpIdleInhibitManager::~ZwpIdleInhibitManager() = default;
 
-wl::Object<zwp_idle_inhibitor_v1> ZwpIdleInhibitManager::CreateInhibitor(
-    wl_surface* surface) {
-  return wl::Object<zwp_idle_inhibitor_v1>(
-      zwp_idle_inhibit_manager_v1_create_inhibitor(manager_.get(), surface));
+bool ZwpIdleInhibitManager::CreateInhibitor() {
+  // Wayland inhibits idle behaviour on certain output, and implies that a
+  // surface bound to that output should obtain the inhibitor and hold it
+  // until it no longer needs to prevent the output to go idle.
+  // We assume that the idle lock is initiated by the user, and therefore the
+  // surface that we should use is the one owned by the window that is focused
+  // currently.
+  const auto* window_manager = connection_->window_manager();
+  DCHECK(window_manager);
+  auto* current_window = window_manager->GetCurrentFocusedWindow();
+  if (!current_window) {
+    LOG(WARNING) << "Cannot inhibit going idle when no window is focused";
+    return false;
+  }
+
+  DCHECK(current_window->root_surface());
+  auto new_inhibitor = wl::Object<zwp_idle_inhibitor_v1>(
+      zwp_idle_inhibit_manager_v1_create_inhibitor(
+          manager_.get(), current_window->root_surface()->surface()));
+
+  idle_inhibitor_.swap(new_inhibitor);
+  return true;
+}
+
+void ZwpIdleInhibitManager::RemoveInhibitor() {
+  inhibiting_window_ = nullptr;
+  idle_inhibitor_.reset();
 }
 
 }  // namespace ui
diff --git a/ui/ozone/platform/wayland/host/zwp_idle_inhibit_manager.h b/ui/ozone/platform/wayland/host/zwp_idle_inhibit_manager.h
index f7af500..b0a72d4 100644
--- a/ui/ozone/platform/wayland/host/zwp_idle_inhibit_manager.h
+++ b/ui/ozone/platform/wayland/host/zwp_idle_inhibit_manager.h
@@ -5,11 +5,13 @@
 #ifndef UI_OZONE_PLATFORM_WAYLAND_HOST_ZWP_IDLE_INHIBIT_MANAGER_H_
 #define UI_OZONE_PLATFORM_WAYLAND_HOST_ZWP_IDLE_INHIBIT_MANAGER_H_
 
+#include "base/memory/weak_ptr.h"
 #include "ui/ozone/platform/wayland/common/wayland_object.h"
 
 namespace ui {
 
 class WaylandConnection;
+class WaylandWindow;
 
 // Wraps the idle inhibit manager, which is provided via
 // zwp_idle_inhibit_manager_v1 interface.
@@ -30,11 +32,19 @@
   ZwpIdleInhibitManager& operator=(const ZwpIdleInhibitManager&) = delete;
   ~ZwpIdleInhibitManager();
 
-  wl::Object<zwp_idle_inhibitor_v1> CreateInhibitor(wl_surface* surface);
+  bool CreateInhibitor();
+  void RemoveInhibitor();
+
+  bool is_inhibiting() const { return !!inhibiting_window_; }
 
  private:
+  const raw_ptr<WaylandConnection> connection_;
+
   // Wayland object wrapped by this class.
   wl::Object<zwp_idle_inhibit_manager_v1> manager_;
+
+  base::WeakPtr<WaylandWindow> inhibiting_window_;
+  wl::Object<zwp_idle_inhibitor_v1> idle_inhibitor_;
 };
 
 }  // namespace ui