commit c270aba43820c64ed28f0e3e8e7fc8781fd4fc97
author Steven Bennetts <stevenjb@chromium.org> Mon May 17 13:36:38 2021 -0700
committer Commit Bot <commit-bot@chromium.org> Wed May 19 15:18:29 2021 +0000
tree fe5a481b6bc2e1b574242de0b14965780c63da25
parent 17d5e00e88219647feabdecf83dd1a4288f5bb77

shill: CellularCapability3gpp: Fix modem_proxy_ crashes

CellularCapability3gpp::ReleaseProxies is called from
CellularCapability3gpp::Stop_PowerDownCompleted which is called
during disable before the capability is destroyed.

This allows other asynchronous callbacks that access the proxy
to potentially get called and crash.

This CL tests for a null proxy before its use.

BUG=b:188035492
TEST=Check for crashes in shill::CellularCapability3gpp::Stop_Disable

Change-Id: Ie7bcdd8614c14c4aac1946f685f190d68e65cda1
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/2900892
Tested-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Eric Caruso <ejcaruso@chromium.org>
Commit-Queue: Steven Bennetts <stevenjb@chromium.org>
(cherry picked from commit ca679b818f9941d886511c524af79cc078bbcbfc)
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/2906272
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>

shill/cellular/cellular_capability_3gpp.cc

diff --git a/shill/cellular/cellular_capability_3gpp.cc b/shill/cellular/cellular_capability_3gpp.cc
index 277c3f1..a10adff 100644
--- a/shill/cellular/cellular_capability_3gpp.cc
+++ b/shill/cellular/cellular_capability_3gpp.cc
@@ -319,6 +319,10 @@
   SLOG(this, 1) << __func__;
   InitProxies();
   CHECK(!callback.is_null());
+  if (!modem_proxy_) {
+    Error::PopulateAndLog(FROM_HERE, error, Error::kWrongState, "No proxy");
+    return;
+  }
   Error local_error(Error::kOperationInitiated);
   metrics_->NotifyDeviceEnableStarted(cellular()->interface_index());
   modem_proxy_->Enable(true, &local_error,
@@ -379,6 +383,11 @@
 void CellularCapability3gpp::Stop_Disable(const ResultCallback& callback) {
   SLOG(this, 3) << __func__;
   Error error;
+  if (!modem_proxy_) {
+    Error::PopulateAndLog(FROM_HERE, &error, Error::kWrongState, "No proxy");
+    callback.Run(error);
+    return;
+  }
   metrics_->NotifyDeviceDisableStarted(cellular()->interface_index());
   modem_proxy_->Enable(false, &error,
                        Bind(&CellularCapability3gpp::Stop_DisableCompleted,
@@ -405,6 +414,11 @@
 void CellularCapability3gpp::Stop_PowerDown(const ResultCallback& callback) {
   SLOG(this, 3) << __func__;
   Error error;
+  if (!modem_proxy_) {
+    Error::PopulateAndLog(FROM_HERE, &error, Error::kWrongState, "No proxy");
+    callback.Run(error);
+    return;
+  }
   modem_proxy_->SetPowerState(
       MM_MODEM_POWER_STATE_LOW, &error,
       Bind(&CellularCapability3gpp::Stop_PowerDownCompleted,
@@ -980,6 +994,10 @@
                           "Already resetting");
     return;
   }
+  if (!modem_proxy_) {
+    Error::PopulateAndLog(FROM_HERE, error, Error::kWrongState, "No proxy");
+    return;
+  }
   ResultCallback cb = Bind(&CellularCapability3gpp::OnResetReply,
                            weak_ptr_factory_.GetWeakPtr(), callback);
   modem_proxy_->Reset(error, cb, kTimeoutReset);
@@ -1419,7 +1437,11 @@
 
 void CellularCapability3gpp::SetPrimarySimSlot(size_t slot) {
   size_t slot_id = slot + 1;
-  LOG(INFO) << "SetPrimarySimSlot: " << slot_id << " (index=" << slot << ")";
+  LOG(INFO) << __func__ << ": " << slot_id << " (index=" << slot << ")";
+  if (!modem_proxy_) {
+    LOG(ERROR) << __func__ << ": No proxy";
+    return;
+  }
   modem_proxy_->SetPrimarySimSlot(
       slot_id, base::Bind([](const Error& error) {
         if (error.IsFailure())