Fix close override not applying to CEF

Recent Chromium versions use RTLD_NEXT to find the close() to invoke.
This results in libcef.so, which comes *after* zypak's libraries in the
resolution order, calling libc's close instead of our close. To fix
that, split out close() overrides into their own library, and put libcef
in LD_PRELOAD between the other zypak overrides and the close()
override.

Fixes #34.

Signed-off-by: Ryan Gonzalez <ryan.gonzalez@collabora.com>

Author: refi64

Makefile

@@ -80,13 +80,20 @@ preload_host_spawn_strategy_DEPS := preload dbus base
 preload_host_spawn_strategy_SOURCES := \
 	bus_safe_fork.cc \
 	initialize.cc \
-	no_close_host_fd.cc \
 	process_override.cc \
 	spawn_launcher_delegate.cc \
 	supervisor.cc \
 
 $(call build_shlib,preload_host_spawn_strategy)
 
+preload_host_spawn_strategy_close_SOURCE_DIR := preload/host/spawn_strategy/close
+preload_host_spawn_strategy_close_NAME := zypak-preload-host-spawn-strategy-close
+preload_host_spawn_strategy_close_DEPS := preload base
+preload_host_spawn_strategy_close_SOURCES := \
+	no_close_host_fd.cc \
+
+$(call build_shlib,preload_host_spawn_strategy_close)
+
 preload_child_SOURCE_DIR := preload/child
 preload_child_NAME := zypak-preload-child
 preload_child_DEPS := preload base
@@ -150,6 +157,7 @@ install : all
 	install -Dm 755 -t $(FLATPAK_DEST)/lib build/libzypak-preload-host.so
 	install -Dm 755 -t $(FLATPAK_DEST)/lib build/libzypak-preload-host-mimic-strategy.so
 	install -Dm 755 -t $(FLATPAK_DEST)/lib build/libzypak-preload-host-spawn-strategy.so
+	install -Dm 755 -t $(FLATPAK_DEST)/lib build/libzypak-preload-host-spawn-strategy-close.so
 	install -Dm 755 -t $(FLATPAK_DEST)/lib build/libzypak-preload-child.so
 	install -Dm 755 -t $(FLATPAK_DEST)/lib build/libzypak-preload-child-mimic-strategy.so
 	install -Dm 755 -t $(FLATPAK_DEST)/lib build/libzypak-preload-child-spawn-strategy.so

README.md

@@ -82,6 +82,11 @@ application process. If you need to add your own libraries to `LD_PRELOAD`, plac
 `ZYPAK_LD_PRELOAD`, which will result in Zypak adding them to the `LD_PRELOAD` list, in addition to
 its own required libraries.
 
+## CEF support
+
+If the application uses CEF, set `ZYPAK_CEF_LIBRARY_PATH` to the absolute path to the `libcef.so`
+library.
+
 ## Using a different version
 
 If you want to try a different Zypak version for testing, or without using the

src/base/env.h

@@ -39,6 +39,7 @@ class Env {
   static constexpr cstring_view kZypakSettingExposeWidevinePath = "ZYPAK_EXPOSE_WIDEVINE_PATH";
   static constexpr cstring_view kZypakSettingLdPreload = "ZYPAK_LD_PRELOAD";
   static constexpr cstring_view kZypakSettingSpawnLatestOnReexec = "ZYPAK_SPAWN_LATEST_ON_REEXEC";
+  static constexpr cstring_view kZypakSettingCefLibraryPath = "ZYPAK_CEF_LIBRARY_PATH";
 };
 
 }  // namespace zypak

src/helper/main.cc

@@ -118,16 +118,11 @@ bool ApplyFdMapFromArgs(ArgsView::iterator* it, ArgsView::iterator last) {
   return true;
 }
 
-std::string GetPreload(std::string_view mode, std::string_view libdir) {
-  std::vector<std::string> preload_names;
-
-  preload_names.emplace_back(mode);
-  if (Env::Test(Env::kZypakZygoteStrategySpawn)) {
-    preload_names.push_back(std::string(mode) + "-spawn-strategy");
-  } else {
-    preload_names.push_back(std::string(mode) + "-mimic-strategy");
-  }
+std::string GetZypakLib(std::string_view libdir, std::string_view name) {
+  return (fs::path(libdir) / ("libzypak-preload-"s + std::string(name) + ".so")).string();
+}
 
+std::string GetPreload(std::string_view mode, std::string_view libdir) {
   std::vector<std::string> preload_libs;
 
   // LD_PRELOAD is loaded in left-to-right order, and the last reference to a symbol is used to
@@ -137,9 +132,18 @@ std::string GetPreload(std::string_view mode, std::string_view libdir) {
     preload_libs.push_back(std::string(*preload));
   }
 
-  for (const std::string& name : preload_names) {
-    fs::path path = fs::path(libdir) / ("libzypak-preload-"s + name + ".so");
-    preload_libs.push_back(path.string());
+  preload_libs.push_back(GetZypakLib(libdir, mode));
+  if (Env::Test(Env::kZypakZygoteStrategySpawn)) {
+    preload_libs.push_back(GetZypakLib(libdir, std::string(mode) + "-spawn-strategy"));
+    if (mode == "host") {
+      if (auto crlib = Env::Get(Env::kZypakSettingCefLibraryPath)) {
+        preload_libs.emplace_back(*crlib);
+      }
+
+      preload_libs.push_back(GetZypakLib(libdir, "host-spawn-strategy-close"));
+    }
+  } else {
+    preload_libs.push_back(GetZypakLib(libdir, std::string(mode) + "-mimic-strategy"));
   }
 
   return Join(preload_libs.begin(), preload_libs.end(), ":");

src/preload/host/spawn_strategy/bus_safe_fork.cc

@@ -14,7 +14,7 @@
 #include "base/debug.h"
 #include "dbus/bus.h"
 #include "preload/declare_override.h"
-#include "preload/host/spawn_strategy/no_close_host_fd.h"
+#include "preload/host/spawn_strategy/close/no_close_host_fd.h"
 
 using namespace zypak;
 

…/host/spawn_strategy/no_close_host_fd.cc …spawn_strategy/close/no_close_host_fd.ccsrc/preload/host/spawn_strategy/no_close_host_fd.cc renamed to src/preload/host/spawn_strategy/close/no_close_host_fd.cc src/preload/host/spawn_strategy/no_close_host_fd.cc renamed to src/preload/host/spawn_strategy/close/no_close_host_fd.cc

@@ -16,10 +16,26 @@
 
 bool zypak::preload::block_supervisor_fd_close = false;
 
+// Overriding close is...tricky.
+//
 // Chrome 92 introduces its own close override:
-// https://chromium-review.googlesource.com/q/I918d79c343c0027ee1ce4353c7acbe7c0e79d1dd This will
-// mean that an override of "close" here will not take effect anymore. In order to work around it,
-// we can also override glibc's __close instead, which Chromium's close override calls into.
+// https://chromium-review.googlesource.com/q/I918d79c343c0027ee1ce4353c7acbe7c0e79d1dd
+// This will mean that an override of "close" here will not take effect anymore. In order to work
+// around it, we can also override glibc's __close instead, which Chromium's close override calls
+// into.
+//
+// Later versions change it to call the close from dlsym(RTLD_NEXT) instead of hardcoding glibc's
+// close:
+// https://chromium-review.googlesource.com/c/chromium/src/+/4418528
+// But, if close() is being called from a shared library like libcef, then RTLD_NEXT is *not*
+// going to point to Zypak, because the resolution order will look like:
+//
+// LD_PRELOAD(zypak is here) -> libcef.so -> ... -> libc.so
+//
+// That's why this override is in its own shared library, so it can be added to the LD_PRELOAD list
+// after the main overrides & with libcef in-between, resulting in a resolution order like:
+
+// LD_PRELOAD(zypak main preload -> libcef.so -> this preload library w/ close) -> ... -> libc.so
 
 DECLARE_OVERRIDE_THROW(int, __close, int fd) {
   if (fd == zypak::sandbox::kZypakSupervisorFd && zypak::preload::block_supervisor_fd_close) {