0

Disable close() ScopedFD hook for component builds

As described in b/342530259, libc hooks in shared library does not work
reliably. The motivation for this change is to enable -Bsymbolic for
libchrome builds in ChromeOS, which is broken right now due to another
interaction with RTLD_NEXT.

This change disables the close() hook and ownership enforcement on
component builds. It is worth noting that enforcement was working as
intended on component builds before, because for Chromium we control the
shared library dependency graph and libbase happens to always get loaded
before libc. If someone was relying on that, it is advised that they
move to testing on non-component build.

BUG=b:342530259
TEST=LD_PRELOAD=/usr/lib/x86_64-linux-gnu/libc.so.6 ./out/component/base_unittests --gtest_filter="ScopedFD*"

Change-Id: Icc21669000bdca493cd4acf777b6ebccc5987454
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5644558
Reviewed-by: Ken Rockot <rockot@google.com>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Tatsuyuki Ishi <ishitatsuyuki@google.com>
Cr-Commit-Position: refs/heads/main@{#1324013}
This commit is contained in:
Tatsuyuki Ishi
2024-07-08 02:46:59 +00:00
committed by Chromium LUCI CQ
parent 9a9b2b4d63
commit bd9afa02dd
4 changed files with 15 additions and 4 deletions

@ -56,11 +56,18 @@ struct ScopedFILECloser {
#if BUILDFLAG(IS_CHROMEOS) || BUILDFLAG(IS_LINUX)
namespace subtle {
#if !defined(COMPONENT_BUILD)
// Enables or disables enforcement of FD ownership as tracked by ScopedFD
// objects. Enforcement is disabled by default since it proves unwieldy in some
// test environments, but tracking is always done. It's best to enable this as
// early as possible in a process's lifetime.
//
// This function is not available in component builds, as the close()
// interceptor used by the implementation is unreliable when compiled into
// a shared library (b/342530259). If FD ownership needs to be tested or
// enforced, it should be done on a non-component build instead.
void BASE_EXPORT EnableFDOwnershipEnforcement(bool enabled);
#endif // !defined(COMPONENT_BUILD)
// Resets ownership state of all FDs. The only permissible use of this API is
// in a forked child process between the fork() and a subsequent exec() call.
@ -81,7 +88,7 @@ void BASE_EXPORT EnableFDOwnershipEnforcement(bool enabled);
void BASE_EXPORT ResetFDOwnership();
} // namespace subtle
#endif
#endif // BUILDFLAG(IS_CHROMEOS) || BUILDFLAG(IS_LINUX)
// -----------------------------------------------------------------------------

@ -65,9 +65,11 @@ void ScopedFDCloseTraits::Release(const ScopedFD& owner, int fd) {
namespace subtle {
#if !defined(COMPONENT_BUILD)
void EnableFDOwnershipEnforcement(bool enabled) {
g_is_ownership_enforced = enabled;
}
#endif // !defined(COMPONENT_BUILD)
void ResetFDOwnership() {
std::fill(g_is_fd_owned.begin(), g_is_fd_owned.end(), false);
@ -81,6 +83,7 @@ bool IsFDOwned(int fd) {
} // namespace base
#if !defined(COMPONENT_BUILD)
using LibcCloseFuncPtr = int (*)(int);
// Load the libc close symbol to forward to from the close wrapper.
@ -110,3 +113,4 @@ __attribute__((visibility("default"), noinline)) int close(int fd) {
}
} // extern "C"
#endif // !defined(COMPONENT_BUILD)

@ -34,7 +34,7 @@ TEST_F(ScopedFDOwnershipTrackingTest, BasicTracking) {
EXPECT_FALSE(IsFDOwned(fd_value));
}
#if defined(GTEST_HAS_DEATH_TEST)
#if defined(GTEST_HAS_DEATH_TEST) && !defined(COMPONENT_BUILD)
TEST_F(ScopedFDOwnershipTrackingTest, NoDoubleOwnership) {
ScopedFD fd = OpenFD();
@ -48,7 +48,7 @@ TEST_F(ScopedFDOwnershipTrackingTest, CrashOnUnownedClose) {
EXPECT_DEATH(close(fd.get()), "");
}
#endif // defined(GTEST_HAS_DEATH_TEST)
#endif // defined(GTEST_HAS_DEATH_TEST) && !defined(COMPONENT_BUILD)
} // namespace
} // namespace base

@ -292,7 +292,7 @@ RunContentProcess(ContentMainParams params,
command_line->AppendSwitch(switches::kUseMobileUserAgent);
#endif
#if BUILDFLAG(IS_CHROMEOS) || BUILDFLAG(IS_LINUX)
#if (BUILDFLAG(IS_CHROMEOS) || BUILDFLAG(IS_LINUX)) && !defined(COMPONENT_BUILD)
base::subtle::EnableFDOwnershipEnforcement(true);
#endif