[Backport] CVE-2021-30585: Use after free in sensor handling

Manual cherry-pick of patch originally reviedwed on
https://chromium-review.googlesource.com/c/chromium/src/+/2911135:
sensors: Add locking when passing sensor updates to the client

This change updates the Win32 and WinRT sensor backends to acquire the
lock when calling back into the client. This is important because the
client_ variable is set to nullptr when the sensor reader is destroyed
and so synchronization is needed to prevent a null pointer dereference
or use after free.

(cherry picked from commit 6d6e9b5443d3cafce07b8cfc64a52f4ee59cb8ad)

Bug: 1023503
Change-Id: Ie677c7a7376e1b01bacaad66264439c5f5af6a0e
Commit-Queue: Reilly Grant <reillyg@chromium.org>
Auto-Submit: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Chris Mumford <cmumford@google.com>
Cr-Original-Commit-Position: refs/heads/master@{#885336}
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/4515@{#47}
Cr-Branched-From: 488fc70865ddaa05324ac00a54a6eb783b4bc41c-refs/heads/master@{#885287}
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
(cherry picked from commit 7a8e3587227aab29f07f943d980e6afd5be9540f)

4 files changed, 40 insertions, 5 deletions

diff --git a/chromium/services/device/generic_sensor/platform_sensor_reader_win.cc b/chromium/services/device/generic_sensor/platform_sensor_reader_win.cc
index 63bd89b7b2d..3ebfb17afc9 100644
--- a/chromium/services/device/generic_sensor/platform_sensor_reader_win.cc
+++ b/chromium/services/device/generic_sensor/platform_sensor_reader_win.cc
@@ -490,7 +490,8 @@ bool PlatformSensorReaderWin32::SetReportingInterval(
 
 HRESULT PlatformSensorReaderWin32::SensorReadingChanged(
     ISensorDataReport* report,
-    SensorReading* reading) const {
+    SensorReading* reading) {
+  base::AutoLock autolock(lock_);
   if (!client_)
     return E_FAIL;
 
@@ -501,6 +502,7 @@ HRESULT PlatformSensorReaderWin32::SensorReadingChanged(
 }
 
 void PlatformSensorReaderWin32::SensorError() {
+  base::AutoLock autolock(lock_);
   if (client_)
     client_->OnSensorError();
 }
diff --git a/chromium/services/device/generic_sensor/platform_sensor_reader_win.h b/chromium/services/device/generic_sensor/platform_sensor_reader_win.h
index 40bc079f77c..f197dba5fc7 100644
--- a/chromium/services/device/generic_sensor/platform_sensor_reader_win.h
+++ b/chromium/services/device/generic_sensor/platform_sensor_reader_win.h
@@ -8,6 +8,8 @@
 #include <SensorsApi.h>
 #include <wrl/client.h>
 
+#include "base/synchronization/lock.h"
+#include "base/thread_annotations.h"
 #include "services/device/generic_sensor/platform_sensor_reader_win_base.h"
 #include "services/device/public/mojom/sensor.mojom.h"
 
@@ -52,7 +54,7 @@ class PlatformSensorReaderWin32 final : public PlatformSensorReaderWinBase {
       WARN_UNUSED_RESULT;
   void ListenSensorEvent();
   HRESULT SensorReadingChanged(ISensorDataReport* report,
-                               SensorReading* reading) const WARN_UNUSED_RESULT;
+                               SensorReading* reading) WARN_UNUSED_RESULT;
   void SensorError();
 
  private:
@@ -64,8 +66,8 @@ class PlatformSensorReaderWin32 final : public PlatformSensorReaderWinBase {
   // StartSensor and StopSensor are called from another thread by
   // PlatformSensorWin that can modify internal state of the object.
   base::Lock lock_;
-  bool sensor_active_;
-  Client* client_;
+  bool sensor_active_ GUARDED_BY(lock_);
+  Client* client_ GUARDED_BY(lock_);
   Microsoft::WRL::ComPtr<ISensor> sensor_;
   Microsoft::WRL::ComPtr<ISensorEvents> event_listener_;
   base::WeakPtrFactory<PlatformSensorReaderWin32> weak_factory_{this};
diff --git a/chromium/services/device/generic_sensor/platform_sensor_reader_winrt.cc b/chromium/services/device/generic_sensor/platform_sensor_reader_winrt.cc
index 622d85f02ae..840e3ff7846 100644
--- a/chromium/services/device/generic_sensor/platform_sensor_reader_winrt.cc
+++ b/chromium/services/device/generic_sensor/platform_sensor_reader_winrt.cc
@@ -344,6 +344,11 @@ HRESULT PlatformSensorReaderWinrtLightSensor::OnReadingChangedCallback(
   if (!has_received_first_sample_ ||
       (abs(lux - last_reported_lux_) >=
        (last_reported_lux_ * kLuxPercentThreshold))) {
+    base::AutoLock autolock(lock_);
+    if (!client_) {
+      return S_OK;
+    }
+
     SensorReading reading;
     reading.als.value = lux;
     reading.als.timestamp = timestamp_delta.InSecondsF();
@@ -417,6 +422,11 @@ HRESULT PlatformSensorReaderWinrtAccelerometer::OnReadingChangedCallback(
       (abs(x - last_reported_x_) >= kAxisThreshold) ||
       (abs(y - last_reported_y_) >= kAxisThreshold) ||
       (abs(z - last_reported_z_) >= kAxisThreshold)) {
+    base::AutoLock autolock(lock_);
+    if (!client_) {
+      return S_OK;
+    }
+
     // Windows.Devices.Sensors.Accelerometer exposes acceleration as
     // proportional and in the same direction as the force of gravity.
     // The generic sensor interface exposes acceleration simply as
@@ -497,6 +507,11 @@ HRESULT PlatformSensorReaderWinrtGyrometer::OnReadingChangedCallback(
       (abs(x - last_reported_x_) >= kDegreeThreshold) ||
       (abs(y - last_reported_y_) >= kDegreeThreshold) ||
       (abs(z - last_reported_z_) >= kDegreeThreshold)) {
+    base::AutoLock autolock(lock_);
+    if (!client_) {
+      return S_OK;
+    }
+
     // Windows.Devices.Sensors.Gyrometer exposes angular velocity as degrees,
     // but the generic sensor interface uses radians so the data must be
     // converted.
@@ -576,6 +591,11 @@ HRESULT PlatformSensorReaderWinrtMagnetometer::OnReadingChangedCallback(
       (abs(x - last_reported_x_) >= kMicroteslaThreshold) ||
       (abs(y - last_reported_y_) >= kMicroteslaThreshold) ||
       (abs(z - last_reported_z_) >= kMicroteslaThreshold)) {
+    base::AutoLock autolock(lock_);
+    if (!client_) {
+      return S_OK;
+    }
+
     SensorReading reading;
     reading.magn.x = x;
     reading.magn.y = y;
@@ -654,6 +674,11 @@ PlatformSensorReaderWinrtAbsOrientationEulerAngles::OnReadingChangedCallback(
       (abs(x - last_reported_x_) >= kDegreeThreshold) ||
       (abs(y - last_reported_y_) >= kDegreeThreshold) ||
       (abs(z - last_reported_z_) >= kDegreeThreshold)) {
+    base::AutoLock autolock(lock_);
+    if (!client_) {
+      return S_OK;
+    }
+
     SensorReading reading;
     reading.orientation_euler.x = x;
     reading.orientation_euler.y = y;
@@ -762,6 +787,11 @@ PlatformSensorReaderWinrtAbsOrientationQuaternion::OnReadingChangedCallback(
   auto angle =
       abs(GetAngleBetweenOrientationSamples(reading, last_reported_sample));
   if (!has_received_first_sample_ || (angle >= kRadianThreshold)) {
+    base::AutoLock autolock(lock_);
+    if (!client_) {
+      return S_OK;
+    }
+
     client_->OnReadingUpdated(reading);
 
     last_reported_sample = reading;
diff --git a/chromium/services/device/generic_sensor/platform_sensor_reader_winrt.h b/chromium/services/device/generic_sensor/platform_sensor_reader_winrt.h
index 0abc1c18628..628447d9964 100644
--- a/chromium/services/device/generic_sensor/platform_sensor_reader_winrt.h
+++ b/chromium/services/device/generic_sensor/platform_sensor_reader_winrt.h
@@ -15,6 +15,7 @@
 #include "base/callback.h"
 #include "base/optional.h"
 #include "base/synchronization/lock.h"
+#include "base/thread_annotations.h"
 #include "services/device/generic_sensor/platform_sensor_reader_win_base.h"
 #include "services/device/public/cpp/generic_sensor/sensor_reading.h"
 #include "ui/gfx/geometry/angle_conversions.h"
@@ -95,7 +96,7 @@ class PlatformSensorReaderWinrtBase : public PlatformSensorReaderWinBase {
   // threads by PlatformSensorWin.
   base::Lock lock_;
   // Null if there is no client to notify, non-null otherwise.
-  Client* client_;
+  Client* client_ GUARDED_BY(lock_);
 
   // Always report the first sample received after starting the sensor.
   bool has_received_first_sample_ = false;