Commit bb9a0e71 authored by Josh Simmons's avatar Josh Simmons Committed by Commit Bot

smbfs: Prevent potential Chrome crash after resume from suspend

If a share fails to remount after suspend, a race condition can cause
the SmbService's share map to be changed while it's being iterated
over, ultimately resulting in Chrome crash due to invalid pointer
dereference. Take a copy of the map's keys and iterate over them so
that the map itself can be safely mutated during the resume process.

Test: Run suspend_stress_test and ensure Chrome doesn't crash
Test: unit_tests --gtest_filter="*SmbFs*"
Bug: 1147299
Change-Id: I9b3982a381951b72f594fc2067992a8df9f9b204
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2559473Reviewed-by: default avatarAnand K Mistry <amistry@chromium.org>
Commit-Queue: Josh Simmons <simmonsjosh@google.com>
Cr-Commit-Position: refs/heads/master@{#830849}
parent 417ba6d3
...@@ -1049,9 +1049,20 @@ void SmbService::OnSuspendUnmountDone( ...@@ -1049,9 +1049,20 @@ void SmbService::OnSuspendUnmountDone(
} }
void SmbService::SuspendDone(const base::TimeDelta& sleep_duration) { void SmbService::SuspendDone(const base::TimeDelta& sleep_duration) {
for (auto it = smbfs_shares_.begin(); it != smbfs_shares_.end(); ++it) { // Don't iterate directly over the share map during the remount
SmbFsShare* share = it->second.get(); // process as shares can be removed on failure in OnSmbfsMountDone.
const std::string mount_id = share->mount_id(); std::vector<std::string> mount_ids;
for (const auto& s : smbfs_shares_)
mount_ids.push_back(s.first);
for (const auto& mount_id : mount_ids) {
auto share_it = smbfs_shares_.find(mount_id);
if (share_it == smbfs_shares_.end()) {
LOG(WARNING) << "Smbfs mount id " << mount_id
<< " no longer present during remount after suspend";
continue;
}
SmbFsShare* share = share_it->second.get();
// Don't try to reconnect as we race the network stack in getting an IP // Don't try to reconnect as we race the network stack in getting an IP
// address. // address.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment