Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2639483003:
Avoid leaving dangling pointers to User in ProfileHelper. (Closed)
|
Created:
3 years, 11 months ago by Shuhei Takahashi Modified:
3 years, 11 months ago Reviewers:
hidehiko CC:
chromium-reviews, elijahtaylor+arcwatch_chromium.org, yusukes+watch_chromium.org, hidehiko+watch_chromium.org, lhchavez+watch_chromium.org, oshima+watch_chromium.org, davemoore+watch_chromium.org Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionAvoid leaving dangling pointers to User in ProfileHelper.
FakeChromeUserManager::AddUser() calls
ProfileHelper::SetProfileToUserMappingForTesting() to inject a new user
to ProfileHelper. However, FakeChromeUserManager does not remove
the added users from ProfileHelper unless we explicitly call
FakeChromeUserManager::RemoveUserFromList(), which ends up leaving
dangling pointers in ProfileHelper.user_list_for_testing_.
This caused use-after-free in crrev.com/2638713002 because
ProfileHelper::GetUserByProfile() is called in VolumeManager::Shutdown()
after a FakeChromeUserManager is destructed.
BUG=chromium:681752
TEST=browser_tests --gtest_filter='ArcSessionManagerTest.*' # with ASAN
Review-Url: https://codereview.chromium.org/2639483003
Cr-Commit-Position: refs/heads/master@{#443993}
Committed: https://chromium.googlesource.com/chromium/src/+/acfaa8216d01efc4da558881290577e79b4c950b
Patch Set 1 #
Messages
Total messages: 16 (12 generated)
Description was changed from ========== Avoid leaving dangling pointers to User. BUG= ========== to ========== Avoid leaving dangling pointers to User in ProfileHelper. FakeChromeUserManager::AddUser() calls ProfileHelper::SetProfileToUserMappingForTesting() to inject a new user to ProfileHelper. However, FakeChromeUserManager does not remove the added users from ProfileHelper unless they are explicitly removed by FakeChromeUserManager::RemoveUserFromList(), ending up dangling pointers in ProfileHelper.user_list_for_testing_. BUG=N/A TEST=browser_tests --gtest_filter='ArcSessionManagerTest.*' # with ASAN ==========
Description was changed from ========== Avoid leaving dangling pointers to User in ProfileHelper. FakeChromeUserManager::AddUser() calls ProfileHelper::SetProfileToUserMappingForTesting() to inject a new user to ProfileHelper. However, FakeChromeUserManager does not remove the added users from ProfileHelper unless they are explicitly removed by FakeChromeUserManager::RemoveUserFromList(), ending up dangling pointers in ProfileHelper.user_list_for_testing_. BUG=N/A TEST=browser_tests --gtest_filter='ArcSessionManagerTest.*' # with ASAN ========== to ========== Avoid leaving dangling pointers to User in ProfileHelper. FakeChromeUserManager::AddUser() calls ProfileHelper::SetProfileToUserMappingForTesting() to inject a new user to ProfileHelper. However, FakeChromeUserManager does not remove the added users from ProfileHelper unless we explicitly call FakeChromeUserManager::RemoveUserFromList(), which ends up leaving dangling pointers in ProfileHelper.user_list_for_testing_. This caused use-after-free in crrev.com/2638713002 because ProfileHelper::GetUserByProfile() is called in VolumeManager::Shutdown() which is called after a FakeChromeUserManager is destructed. BUG=N/A TEST=browser_tests --gtest_filter='ArcSessionManagerTest.*' # with ASAN ==========
Description was changed from ========== Avoid leaving dangling pointers to User in ProfileHelper. FakeChromeUserManager::AddUser() calls ProfileHelper::SetProfileToUserMappingForTesting() to inject a new user to ProfileHelper. However, FakeChromeUserManager does not remove the added users from ProfileHelper unless we explicitly call FakeChromeUserManager::RemoveUserFromList(), which ends up leaving dangling pointers in ProfileHelper.user_list_for_testing_. This caused use-after-free in crrev.com/2638713002 because ProfileHelper::GetUserByProfile() is called in VolumeManager::Shutdown() which is called after a FakeChromeUserManager is destructed. BUG=N/A TEST=browser_tests --gtest_filter='ArcSessionManagerTest.*' # with ASAN ========== to ========== Avoid leaving dangling pointers to User in ProfileHelper. FakeChromeUserManager::AddUser() calls ProfileHelper::SetProfileToUserMappingForTesting() to inject a new user to ProfileHelper. However, FakeChromeUserManager does not remove the added users from ProfileHelper unless we explicitly call FakeChromeUserManager::RemoveUserFromList(), which ends up leaving dangling pointers in ProfileHelper.user_list_for_testing_. This caused use-after-free in crrev.com/2638713002 because ProfileHelper::GetUserByProfile() is called in VolumeManager::Shutdown() after a FakeChromeUserManager is destructed. BUG=N/A TEST=browser_tests --gtest_filter='ArcSessionManagerTest.*' # with ASAN ==========
The CQ bit was checked by nya@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
nya@chromium.org changed reviewers: + hidehiko@chromium.org
hidehiko: PTAL
LGTM. Thank you for investigation!
Description was changed from ========== Avoid leaving dangling pointers to User in ProfileHelper. FakeChromeUserManager::AddUser() calls ProfileHelper::SetProfileToUserMappingForTesting() to inject a new user to ProfileHelper. However, FakeChromeUserManager does not remove the added users from ProfileHelper unless we explicitly call FakeChromeUserManager::RemoveUserFromList(), which ends up leaving dangling pointers in ProfileHelper.user_list_for_testing_. This caused use-after-free in crrev.com/2638713002 because ProfileHelper::GetUserByProfile() is called in VolumeManager::Shutdown() after a FakeChromeUserManager is destructed. BUG=N/A TEST=browser_tests --gtest_filter='ArcSessionManagerTest.*' # with ASAN ========== to ========== Avoid leaving dangling pointers to User in ProfileHelper. FakeChromeUserManager::AddUser() calls ProfileHelper::SetProfileToUserMappingForTesting() to inject a new user to ProfileHelper. However, FakeChromeUserManager does not remove the added users from ProfileHelper unless we explicitly call FakeChromeUserManager::RemoveUserFromList(), which ends up leaving dangling pointers in ProfileHelper.user_list_for_testing_. This caused use-after-free in crrev.com/2638713002 because ProfileHelper::GetUserByProfile() is called in VolumeManager::Shutdown() after a FakeChromeUserManager is destructed. BUG=chromium:681752 TEST=browser_tests --gtest_filter='ArcSessionManagerTest.*' # with ASAN ==========
The CQ bit was checked by nya@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 1, "attempt_start_ts": 1484635647402600, "parent_rev":
"89827840c471011999bf72118831da54f73e87d5", "commit_rev":
"acfaa8216d01efc4da558881290577e79b4c950b"}
Message was sent while issue was closed.
Description was changed from ========== Avoid leaving dangling pointers to User in ProfileHelper. FakeChromeUserManager::AddUser() calls ProfileHelper::SetProfileToUserMappingForTesting() to inject a new user to ProfileHelper. However, FakeChromeUserManager does not remove the added users from ProfileHelper unless we explicitly call FakeChromeUserManager::RemoveUserFromList(), which ends up leaving dangling pointers in ProfileHelper.user_list_for_testing_. This caused use-after-free in crrev.com/2638713002 because ProfileHelper::GetUserByProfile() is called in VolumeManager::Shutdown() after a FakeChromeUserManager is destructed. BUG=chromium:681752 TEST=browser_tests --gtest_filter='ArcSessionManagerTest.*' # with ASAN ========== to ========== Avoid leaving dangling pointers to User in ProfileHelper. FakeChromeUserManager::AddUser() calls ProfileHelper::SetProfileToUserMappingForTesting() to inject a new user to ProfileHelper. However, FakeChromeUserManager does not remove the added users from ProfileHelper unless we explicitly call FakeChromeUserManager::RemoveUserFromList(), which ends up leaving dangling pointers in ProfileHelper.user_list_for_testing_. This caused use-after-free in crrev.com/2638713002 because ProfileHelper::GetUserByProfile() is called in VolumeManager::Shutdown() after a FakeChromeUserManager is destructed. BUG=chromium:681752 TEST=browser_tests --gtest_filter='ArcSessionManagerTest.*' # with ASAN Review-Url: https://codereview.chromium.org/2639483003 Cr-Commit-Position: refs/heads/master@{#443993} Committed: https://chromium.googlesource.com/chromium/src/+/acfaa8216d01efc4da5588812905... ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1) as https://chromium.googlesource.com/chromium/src/+/acfaa8216d01efc4da5588812905... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
