Commit 7883835c authored by Matt Mueller's avatar Matt Mueller Committed by Commit Bot

Fix NetworkChangeNotifierPosixTest "flaky" crashes.

By default, NetworkChangeNotifier creates a global
SystemDnsConfigChangeNotifier which has a task_runner_ handle that ends
up pointing to the base::test::TaskEnvironment that was created in the
first NetworkChangeNotifierPosixTest which ran, and then
the next such test ran in the same process crashes due to UAF.

Bug: 999313
Change-Id: I497d6ab4a3a8c97f57ee935b83f1d5d0e17a1d0b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1776724
Commit-Queue: Matt Mueller <mattm@chromium.org>
Reviewed-by: default avatarEric Orth <ericorth@chromium.org>
Cr-Commit-Position: refs/heads/master@{#691859}
parent 6139b7f4
...@@ -22,7 +22,16 @@ namespace net { ...@@ -22,7 +22,16 @@ namespace net {
NetworkChangeNotifierPosix::NetworkChangeNotifierPosix( NetworkChangeNotifierPosix::NetworkChangeNotifierPosix(
NetworkChangeNotifier::ConnectionType initial_connection_type, NetworkChangeNotifier::ConnectionType initial_connection_type,
NetworkChangeNotifier::ConnectionSubtype initial_connection_subtype) NetworkChangeNotifier::ConnectionSubtype initial_connection_subtype)
: NetworkChangeNotifier(NetworkChangeCalculatorParamsPosix()), : NetworkChangeNotifierPosix(initial_connection_type,
initial_connection_subtype,
/*system_dns_config_notifier=*/nullptr) {}
NetworkChangeNotifierPosix::NetworkChangeNotifierPosix(
NetworkChangeNotifier::ConnectionType initial_connection_type,
NetworkChangeNotifier::ConnectionSubtype initial_connection_subtype,
SystemDnsConfigChangeNotifier* system_dns_config_notifier)
: NetworkChangeNotifier(NetworkChangeCalculatorParamsPosix(),
system_dns_config_notifier),
connection_type_(initial_connection_type), connection_type_(initial_connection_type),
max_bandwidth_mbps_( max_bandwidth_mbps_(
NetworkChangeNotifier::GetMaxBandwidthMbpsForConnectionSubtype( NetworkChangeNotifier::GetMaxBandwidthMbpsForConnectionSubtype(
......
...@@ -52,6 +52,14 @@ class NET_EXPORT NetworkChangeNotifierPosix : public NetworkChangeNotifier { ...@@ -52,6 +52,14 @@ class NET_EXPORT NetworkChangeNotifierPosix : public NetworkChangeNotifier {
private: private:
friend class NetworkChangeNotifierPosixTest; friend class NetworkChangeNotifierPosixTest;
// For testing purposes, allows specifying a SystemDnsConfigChangeNotifier.
// If |system_dns_config_notifier| is nullptr, NetworkChangeNotifier create a
// global one.
NetworkChangeNotifierPosix(
NetworkChangeNotifier::ConnectionType initial_connection_type,
NetworkChangeNotifier::ConnectionSubtype initial_connection_subtype,
SystemDnsConfigChangeNotifier* system_dns_config_notifier);
// Calculates parameters used for network change notifier online/offline // Calculates parameters used for network change notifier online/offline
// signals. // signals.
static NetworkChangeNotifier::NetworkChangeCalculatorParams static NetworkChangeNotifier::NetworkChangeCalculatorParams
......
...@@ -17,13 +17,18 @@ namespace net { ...@@ -17,13 +17,18 @@ namespace net {
class NetworkChangeNotifierPosixTest : public testing::Test { class NetworkChangeNotifierPosixTest : public testing::Test {
public: public:
NetworkChangeNotifierPosixTest() NetworkChangeNotifierPosixTest()
: task_environment_(base::test::TaskEnvironment::TimeSource::MOCK_TIME), : task_environment_(base::test::TaskEnvironment::TimeSource::MOCK_TIME) {
notifier_(new NetworkChangeNotifierPosix( // Create a SystemDnsConfigChangeNotifier instead of letting
NetworkChangeNotifier::CONNECTION_UNKNOWN, // NetworkChangeNotifier create a global one, otherwise the global one will
NetworkChangeNotifier::SUBTYPE_UNKNOWN)) { // hold a TaskRunner handle to |task_environment_| and crash if any
// subsequent tests use it.
dns_config_notifier_ = std::make_unique<SystemDnsConfigChangeNotifier>();
notifier_.reset(new NetworkChangeNotifierPosix(
NetworkChangeNotifier::CONNECTION_UNKNOWN,
NetworkChangeNotifier::SUBTYPE_UNKNOWN, dns_config_notifier_.get()));
auto dns_config_service = std::make_unique<TestDnsConfigService>(); auto dns_config_service = std::make_unique<TestDnsConfigService>();
dns_config_service_ = dns_config_service.get(); dns_config_service_ = dns_config_service.get();
notifier_->system_dns_config_notifier()->SetDnsConfigServiceForTesting( dns_config_notifier_->SetDnsConfigServiceForTesting(
std::move(dns_config_service)); std::move(dns_config_service));
} }
...@@ -37,6 +42,7 @@ class NetworkChangeNotifierPosixTest : public testing::Test { ...@@ -37,6 +42,7 @@ class NetworkChangeNotifierPosixTest : public testing::Test {
private: private:
base::test::TaskEnvironment task_environment_; base::test::TaskEnvironment task_environment_;
net::NetworkChangeNotifier::DisableForTest mock_notifier_disabler_; net::NetworkChangeNotifier::DisableForTest mock_notifier_disabler_;
std::unique_ptr<SystemDnsConfigChangeNotifier> dns_config_notifier_;
std::unique_ptr<NetworkChangeNotifierPosix> notifier_; std::unique_ptr<NetworkChangeNotifierPosix> notifier_;
TestDnsConfigService* dns_config_service_; TestDnsConfigService* dns_config_service_;
}; };
......
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