From: uazo Date: Thu, 11 Dec 2025 09:53:13 +0000 Subject: Supporting Dangling Ptr Detection via BackupRefPtr Enable Dangling Ptr Detection (DPD) via BackupRefPtr (BRP) (disabled by default) and additional safety checks that are too expensive to have on by default. Enable checking raw_ptr do not become dangling during their lifetime. Write a fixed cookie pattern at the end of each allocation to ensure there is no OOB write. Enable MTE activation in Android for enabled devices (flag disabled by default) BRP and MTE activate additional checks related to memory usage at the expense of performance and increased memory usage, but guarantee the user that the browser will not suffer from development errors related to memory management. License: GPL-2.0-or-later - https://spdx.org/licenses/GPL-2.0-or-later.html --- base/allocator/partition_alloc_features.cc | 5 +++- base/allocator/partition_alloc_support.cc | 30 ++++++++++++++----- .../partition_allocator/partition_alloc.gni | 6 ++-- base/observer_list_types.cc | 11 +++++++ build_overrides/partition_alloc.gni | 3 +- .../java/res/xml/privacy_preferences.xml | 5 ++++ .../compositor/CompositorViewHolder.java | 2 +- .../layouts/LayoutManagerChromeTablet.java | 12 ++++---- ...ngling-Ptr-Detection-via-BackupRefPtr.grdp | 9 ++++++ .../Enable-Partition-Alloc-BRP-Checks.inc | 11 +++++++ .../platform/wtf/allocator/partitions.cc | 10 +------ 11 files changed, 76 insertions(+), 28 deletions(-) create mode 100644 chrome/browser/ui/android/strings/cromite_android_chrome_strings_grd/Supporting-Dangling-Ptr-Detection-via-BackupRefPtr.grdp create mode 100644 cromite_flags/chrome/browser/about_flags_cc/Enable-Partition-Alloc-BRP-Checks.inc diff --git a/base/allocator/partition_alloc_features.cc b/base/allocator/partition_alloc_features.cc --- a/base/allocator/partition_alloc_features.cc +++ b/base/allocator/partition_alloc_features.cc @@ -100,6 +100,7 @@ BASE_FEATURE_PARAM(int, partition_alloc::internal::SlotSpanRingMaxSize::kMedium); BASE_FEATURE(kPartitionAllocWithAdvancedChecks, FEATURE_DISABLED_BY_DEFAULT); +SET_CROMITE_FEATURE_ENABLED(kPartitionAllocWithAdvancedChecks); constexpr FeatureParam::Option kPartitionAllocWithAdvancedChecksEnabledProcessesOptions[] = { {PartitionAllocWithAdvancedChecksEnabledProcesses::kBrowserOnly, @@ -176,6 +177,7 @@ BASE_FEATURE(kPartitionAllocBackupRefPtr, FEATURE_DISABLED_BY_DEFAULT #endif ); +SET_CROMITE_FEATURE_ENABLED(kPartitionAllocBackupRefPtr); constexpr FeatureParam::Option kBackupRefPtrEnabledProcessesOptions[] = { @@ -190,7 +192,7 @@ BASE_FEATURE_ENUM_PARAM(BackupRefPtrEnabledProcesses, &kPartitionAllocBackupRefPtr, kPAFeatureEnabledProcessesStr, // Exception for IS_DESKTOP_ANDROID approved in crbug.com/482155132. -#if BUILDFLAG(IS_ANDROID) && !BUILDFLAG(IS_DESKTOP_ANDROID) +#if BUILDFLAG(IS_ANDROID) && !BUILDFLAG(IS_DESKTOP_ANDROID_CROMITE) BackupRefPtrEnabledProcesses::kNonRenderer, #else BackupRefPtrEnabledProcesses::kAllProcesses, @@ -231,6 +233,7 @@ BASE_FEATURE(kPartitionAllocMemoryTagging, FEATURE_DISABLED_BY_DEFAULT #endif ); +SET_CROMITE_FEATURE_DISABLED(kPartitionAllocMemoryTagging); constexpr FeatureParam::Option kMemtagModeOptions[] = { {MemtagMode::kSync, "sync"}, diff --git a/base/allocator/partition_alloc_support.cc b/base/allocator/partition_alloc_support.cc --- a/base/allocator/partition_alloc_support.cc +++ b/base/allocator/partition_alloc_support.cc @@ -286,6 +286,20 @@ void StartMemoryReclaimer(scoped_refptr task_runner) { std::map ProposeSyntheticFinchTrials() { std::map trials; +#if PA_BUILDFLAG(ENABLE_BACKUP_REF_PTR_SUPPORT) + trials.emplace("BackupRefPtrSupport", "Enabled"); + if (base::FeatureList::IsEnabled(base::features::kPartitionAllocBackupRefPtr)) { + trials.emplace("BackupRefPtrFlag", "Enabled"); + } else { + trials.emplace("BackupRefPtrFlag", "Disabled"); + } + if (base::FeatureList::IsEnabled(base::features::kPartitionAllocWithAdvancedChecks)) { + trials.emplace("PAAdvancedChecks", "Enabled"); + } else { + trials.emplace("PAAdvancedChecks", "Disabled"); + } +#endif + #if PA_BUILDFLAG(ENABLE_DANGLING_RAW_PTR_CHECKS) trials.emplace("DanglingPointerDetector", "Enabled"); #else @@ -297,13 +311,18 @@ std::map ProposeSyntheticFinchTrials() { trials.emplace("VectorRawPtrExperiment", "Disabled"); #if PA_BUILDFLAG(HAS_MEMORY_TAGGING) + bool has_mte = base::CPU::GetInstanceNoAllocation().has_mte(); + if (has_mte) { + trials.emplace("MteOnDevice", "Supported"); + } else { + trials.emplace("MteOnDevice", "Not Supported"); + } if (base::FeatureList::IsEnabled( base::features::kPartitionAllocMemoryTagging)) { - bool has_mte = base::CPU::GetInstanceNoAllocation().has_mte(); if (has_mte) { - trials.emplace("MemoryTaggingDogfood", "Enabled"); + trials.emplace("MemoryTagging", "Enabled"); } else { - trials.emplace("MemoryTaggingDogfood", "Disabled"); + trials.emplace("MemoryTagging", "Mte unsupported on device"); } #if BUILDFLAG(IS_ANDROID) BootloaderOverride bootloader_override = GetBootloaderOverride(); @@ -939,10 +958,7 @@ bool PartitionAllocSupport::ShouldEnablePartitionAllocWithAdvancedChecks( base::features::kPartitionAllocWithAdvancedChecks)) { return false; } - return ShouldEnableFeatureOnProcess( - base::features::kPartitionAllocWithAdvancedChecksEnabledProcessesParam - .Get(), - process_type); + return true; #endif // !PA_BUILDFLAG(USE_PARTITION_ALLOC_AS_MALLOC) } diff --git a/base/allocator/partition_allocator/partition_alloc.gni b/base/allocator/partition_allocator/partition_alloc.gni --- a/base/allocator/partition_allocator/partition_alloc.gni +++ b/base/allocator/partition_allocator/partition_alloc.gni @@ -221,8 +221,8 @@ declare_args() { # This will write a fixed cookie pattern at the end of each allocation, and # later verify the pattern remain unchanged to ensure there is no OOB write. # It comes with performance and memory cost, hence enabled only in debug. - use_partition_cookie = - partition_alloc_is_debug || partition_alloc_dcheck_always_on + use_partition_cookie = true + #partition_alloc_is_debug || partition_alloc_dcheck_always_on # This will change partition cookie size to 4B or 8B, whichever equivalent to # size of InSlotMetadata. This option is useful for InSlotMetadata corruption @@ -373,7 +373,7 @@ declare_args() { # This is meant to be used primarily on bots. It is much easier to override # the feature flags using a binary flag instead of updating multiple bots's # scripts to pass command line arguments. - use_full_mte = false + use_full_mte = true } # We want to provide assertions that guard against inconsistent build diff --git a/base/observer_list_types.cc b/base/observer_list_types.cc --- a/base/observer_list_types.cc +++ b/base/observer_list_types.cc @@ -3,11 +3,22 @@ // found in the LICENSE file. #include "base/observer_list_types.h" +#include "base/logging.h" +#include "base/debug/stack_trace.h" namespace base { CheckedObserver::CheckedObserver() = default; CheckedObserver::~CheckedObserver() = default; + // If weak_ptr was invalidated then this attempt to iterate over the + // pointer is a UAF. Tip: If it's unclear where the `delete` occurred, try + // adding CHECK(!IsInObserverList()) to the ~CheckedObserver() (destructor) + // override. However, note that this is not always a bug: a destroyed + // observer can exist in an ObserverList so long as nothing iterates over + // the ObserverList before the list itself is destroyed. + // if(IsInObserverList()) { + // LOG(INFO) << "--- ~CheckedObserver " << base::debug::StackTrace(); + // } bool CheckedObserver::IsInObserverList() const { return factory_.HasWeakPtrs(); diff --git a/build_overrides/partition_alloc.gni b/build_overrides/partition_alloc.gni --- a/build_overrides/partition_alloc.gni +++ b/build_overrides/partition_alloc.gni @@ -106,7 +106,7 @@ use_partition_alloc_as_malloc_default = enable_backup_ref_ptr_support_default = use_partition_alloc_as_malloc_default -enable_backup_ref_ptr_slow_checks_default = false +enable_backup_ref_ptr_slow_checks_default = true enable_dangling_raw_ptr_checks_default = # The DanglingPointerDetector relies on BackupRefPtr: @@ -133,6 +133,7 @@ enable_dangling_raw_ptr_checks_default = # Only the `android-rel` CQ bot has enforced DanglingPointerDetector checks # at the moment. The other Android bots are not ready for it yet. !is_android +enable_dangling_raw_ptr_checks_default = true raw_ptr_zero_on_construct_default = true raw_ptr_zero_on_move_default = true diff --git a/chrome/android/java/res/xml/privacy_preferences.xml b/chrome/android/java/res/xml/privacy_preferences.xml --- a/chrome/android/java/res/xml/privacy_preferences.xml +++ b/chrome/android/java/res/xml/privacy_preferences.xml @@ -98,6 +98,11 @@ found in the LICENSE file. android:summary="@string/throttle_main_thread_to_60hz_summary" app:featureName="throttle-main-thread-to-60hz" app:needRestart="true" /> + diff --git a/chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorViewHolder.java b/chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorViewHolder.java --- a/chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorViewHolder.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/compositor/CompositorViewHolder.java @@ -722,8 +722,8 @@ public class CompositorViewHolder extends FrameLayout mApplicationBottomInsetSupplier.getSupplier().removeObserver(mOnViewportInsetsChanged); } - mCompositorView.shutDown(); if (mLayoutManager != null) mLayoutManager.destroy(); + mCompositorView.shutDown(); if (mOnscreenContentProvider != null) mOnscreenContentProvider.destroy(); if (mContentView != null) { mContentView.removeOnHierarchyChangeListener(this); diff --git a/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/LayoutManagerChromeTablet.java b/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/LayoutManagerChromeTablet.java --- a/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/LayoutManagerChromeTablet.java +++ b/chrome/android/java/src/org/chromium/chrome/browser/compositor/layouts/LayoutManagerChromeTablet.java @@ -178,18 +178,18 @@ public class LayoutManagerChromeTablet extends LayoutManagerChrome { @Override @SuppressWarnings("NullAway") public void destroy() { - super.destroy(); + if (mTabStripLayoutHelperManager != null) { + removeObserver(mTabStripLayoutHelperManager.getTabSwitcherObserver()); + mTabStripLayoutHelperManager.destroy(); + mTabStripLayoutHelperManager = null; + } if (mLayerTitleCache != null) { mLayerTitleCache.shutDown(); mLayerTitleCache = null; } - if (mTabStripLayoutHelperManager != null) { - removeObserver(mTabStripLayoutHelperManager.getTabSwitcherObserver()); - mTabStripLayoutHelperManager.destroy(); - mTabStripLayoutHelperManager = null; - } + super.destroy(); } @Override diff --git a/chrome/browser/ui/android/strings/cromite_android_chrome_strings_grd/Supporting-Dangling-Ptr-Detection-via-BackupRefPtr.grdp b/chrome/browser/ui/android/strings/cromite_android_chrome_strings_grd/Supporting-Dangling-Ptr-Detection-via-BackupRefPtr.grdp new file mode 100644 --- /dev/null +++ b/chrome/browser/ui/android/strings/cromite_android_chrome_strings_grd/Supporting-Dangling-Ptr-Detection-via-BackupRefPtr.grdp @@ -0,0 +1,9 @@ + + + + Enable Dangling Ptr Detection (DPD) via BackupRefPtr (BRP) + + + Enable checking raw pointer do not become dangling during their lifetime to prevent UAF and write a fixed cookie pattern at the end of each allocation to ensure there is no OOB write. + + diff --git a/cromite_flags/chrome/browser/about_flags_cc/Enable-Partition-Alloc-BRP-Checks.inc b/cromite_flags/chrome/browser/about_flags_cc/Enable-Partition-Alloc-BRP-Checks.inc new file mode 100644 --- /dev/null +++ b/cromite_flags/chrome/browser/about_flags_cc/Enable-Partition-Alloc-BRP-Checks.inc @@ -0,0 +1,11 @@ +#ifdef FLAG_SECTION + + {"enable-dangling-pointer-detector", + "Enable Dangling Pointer Detector", + "Enable Dangling Ptr Detection (DPD) via BackupRefPtr (BRP). " + "Enable additional safety checks that are too expensive to have on by default. " + "Enable checking raw_ptr do not become dangling during their lifetime. " + "Write a fixed cookie pattern at the end of each allocation to ensure there is no OOB write.", kOsAll, + FEATURE_VALUE_TYPE(base::features::kPartitionAllocBackupRefPtr)}, + +#endif diff --git a/third_party/blink/renderer/platform/wtf/allocator/partitions.cc b/third_party/blink/renderer/platform/wtf/allocator/partitions.cc --- a/third_party/blink/renderer/platform/wtf/allocator/partitions.cc +++ b/third_party/blink/renderer/platform/wtf/allocator/partitions.cc @@ -86,16 +86,8 @@ partition_alloc::PartitionOptions PartitionOptionsFromFeatures() { using partition_alloc::PartitionOptions; #if PA_BUILDFLAG(ENABLE_BACKUP_REF_PTR_SUPPORT) - const auto brp_mode = base::features::kBackupRefPtrModeParam.Get(); - const bool process_affected_by_brp_flag = - base::features::kBackupRefPtrEnabledProcessesParam.Get() == - BackupRefPtrEnabledProcesses::kAllProcesses || - base::features::kBackupRefPtrEnabledProcessesParam.Get() == - BackupRefPtrEnabledProcesses::kBrowserAndRenderer; const bool enable_brp = base::FeatureList::IsEnabled( - base::features::kPartitionAllocBackupRefPtr) && - (brp_mode == BackupRefPtrMode::kEnabled) && - process_affected_by_brp_flag; + base::features::kPartitionAllocBackupRefPtr); #else // PA_BUILDFLAG(ENABLE_BACKUP_REF_PTR_SUPPORT) const bool enable_brp = false; #endif --