[M120 Migration] Fix use after free of session id string

Patch from:
https://review.tizen.org/gerrit/#/c/310953/

After dtor of WebContentDecryptionModuleSessionImpl, the string of
session id will be released, and the resolve function of close session
promise will trigger dtor of WebContentDecryptionModuleSessionImpl,
so never use session id after resolving promise of close session.

Change-Id: I48266879fd3f98b05861de60371a0c2533442783
Signed-off-by: zhishun.zhou <zhishun.zhou@samsung.com>

diff --git a/third_party/blink/renderer/modules/encryptedmedia/media_key_session.cc b/third_party/blink/renderer/modules/encryptedmedia/media_key_session.cc
index 940e2ba428dbd50afe8ef4940e47709c332356b0..0a7953a91bc52249e776dace1df3004e9e62c74f 100644 (file)
--- a/third_party/blink/renderer/modules/encryptedmedia/media_key_session.cc
+++ b/third_party/blink/renderer/modules/encryptedmedia/media_key_session.cc
@@ -467,7 +467,7 @@ MediaKeySession::~MediaKeySession() {
 }
 
 void MediaKeySession::Dispose() {
-  DVLOG(MEDIA_KEY_SESSION_LOG_LEVEL) << __func__ << "(" << this << ")";
+  LOG(INFO) << "(" << static_cast<void*>(this) << "), " << __func__;
 
   // Drop references to objects from content/ that aren't managed by blink.
   session_.reset();
@@ -794,7 +794,7 @@ void MediaKeySession::UpdateTask(ContentDecryptionModuleResult* result,
 
 ScriptPromise MediaKeySession::close(ScriptState* script_state,
                                      ExceptionState& exception_state) {
-  DVLOG(MEDIA_KEY_SESSION_LOG_LEVEL) << __func__ << "(" << this << ")";
+  LOG(INFO) << "(" << static_cast<void*>(this) << "), " << __func__;
 
   // From https://w3c.github.io/encrypted-media/#close:
   // Indicates that the application no longer needs the session and the CDM
@@ -832,7 +832,7 @@ ScriptPromise MediaKeySession::close(ScriptState* script_state,
 
 void MediaKeySession::CloseTask(ContentDecryptionModuleResult* result) {
   // NOTE: Continue step 4 of MediaKeySession::close().
-  DVLOG(MEDIA_KEY_SESSION_LOG_LEVEL) << __func__ << "(" << this << ")";
+  LOG(INFO) << "(" << static_cast<void*>(this) << "), " << __func__;
 
   // close() in Chromium will execute steps 5.1 through 5.3.
   session_->Close(result->Result());
@@ -844,6 +844,7 @@ void MediaKeySession::OnClosePromiseResolved() {
   // Stop the CDM from firing any more events for this session now that it is
   // closed. This was deferred in OnSessionClosed() as the EME spec resolves
   // the promise after firing the event.
+  LOG(INFO) << "(" << static_cast<void*>(this) << "), " << __func__;
   Dispose();
 }
 
@@ -976,7 +977,7 @@ void MediaKeySession::OnSessionClosed(media::CdmSessionClosedReason reason) {
   // as the result of a close() call, but also happens when update() has been
   // called with a record of license destruction or if the CDM crashes or
   // otherwise becomes unavailable.
-  DVLOG(MEDIA_KEY_SESSION_LOG_LEVEL) << __func__ << "(" << this << ")";
+  LOG(INFO) << "(" << static_cast<void*>(this) << "), " << __func__;
 
   // From http://w3c.github.io/encrypted-media/#session-closed
   // 1. Let session be the associated MediaKeySession object.
@@ -1111,6 +1112,7 @@ bool MediaKeySession::HasPendingActivity() const {
 
 void MediaKeySession::ContextDestroyed() {
   // Stop the CDM from firing any more events for this session.
+  LOG(INFO) << "(" << static_cast<void*>(this) << "), " << __func__;
   session_.reset();
   is_closed_ = true;
   action_timer_.Stop();

diff --git a/third_party/blink/renderer/platform/media/cdm_session_adapter.cc b/third_party/blink/renderer/platform/media/cdm_session_adapter.cc
index 90a08c9261898e566d75f306338b52017c966f2b..7735a01d0df1fb1a235bc68492fb97056d6b47fe 100644 (file)
--- a/third_party/blink/renderer/platform/media/cdm_session_adapter.cc
+++ b/third_party/blink/renderer/platform/media/cdm_session_adapter.cc
@@ -126,7 +126,8 @@ void CdmSessionAdapter::UpdateSession(
 void CdmSessionAdapter::CloseSession(
     const std::string& session_id,
     std::unique_ptr<media::SimpleCdmPromise> promise) {
-  DVLOG(2) << __func__ << ": session_id = " << session_id;
+  LOG(INFO) << "(" << static_cast<void*>(this) << "), " << __func__
+            << ": session_id = " << session_id;
   cdm_->CloseSession(session_id, std::move(promise));
 }
 
@@ -247,13 +248,19 @@ void CdmSessionAdapter::OnSessionExpirationUpdate(const std::string& session_id,
 
 void CdmSessionAdapter::OnSessionClosed(const std::string& session_id,
                                         media::CdmSessionClosedReason reason) {
+  LOG(INFO) << "(" << static_cast<void*>(this) << "), " << __func__
+            << " for session " << session_id;
   WebContentDecryptionModuleSessionImpl* session = GetSession(session_id);
   DLOG_IF(WARNING, !session)
       << __func__ << " for unknown session " << session_id;
   if (session) {
-    DVLOG(2) << __func__ << ": session_id = " << session_id
-             << ", reason = " << static_cast<int>(reason);
+    LOG(INFO) << "(" << static_cast<void*>(this) << "), " << __func__
+              << ": session_id = " << session_id
+              << ", reason = " << static_cast<int>(reason);
     session->OnSessionClosed(reason);
+  } else {
+    LOG(INFO) << "(" << static_cast<void*>(this) << "), " << __func__
+              << " find no session for id " << session_id;
   }
 }
 

diff --git a/third_party/blink/renderer/platform/media/web_content_decryption_module_session_impl.cc b/third_party/blink/renderer/platform/media/web_content_decryption_module_session_impl.cc
index 4690838e244a5412febb5ccee9a315f146aad840..73aa11e9b2e06fda7ea27845e2e5dcc01a7f9d8e 100644 (file)
--- a/third_party/blink/renderer/platform/media/web_content_decryption_module_session_impl.cc
+++ b/third_party/blink/renderer/platform/media/web_content_decryption_module_session_impl.cc
@@ -246,12 +246,17 @@ WebContentDecryptionModuleSessionImpl::WebContentDecryptionModuleSessionImpl(
     : adapter_(adapter),
       session_type_(ConvertSessionType(session_type)),
       has_close_been_called_(false),
-      is_closed_(false) {}
+      is_closed_(false) {
+  LOG(INFO) << "WebContentDecryptionModuleSessionImpl("
+            << static_cast<void*>(this) << ")";
+}
 
 WebContentDecryptionModuleSessionImpl::
     ~WebContentDecryptionModuleSessionImpl() {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
 
+  LOG(INFO) << "~WebContentDecryptionModuleSessionImpl("
+            << static_cast<void*>(this) << ")";
   if (!session_id_.empty()) {
     adapter_->UnregisterSession(session_id_);
 
@@ -267,6 +272,9 @@ WebContentDecryptionModuleSessionImpl::
     // destroyed, there is no need for the promise to do anything as this
     // session will be gone.
     if (!is_closed_ && !has_close_been_called_) {
+      LOG(INFO) << "~WebContentDecryptionModuleSessionImpl("
+                << static_cast<void*>(this) << ")"
+                << " trigger close session";
       adapter_->CloseSession(session_id_,
                              std::make_unique<media::DoNothingCdmPromise<>>());
     }
@@ -431,12 +439,17 @@ void WebContentDecryptionModuleSessionImpl::Close(
   DCHECK(!session_id_.empty());
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
 
+  LOG(INFO) << "(" << static_cast<void*>(this) << "), " << __func__
+            << ", session id: " << session_id_;
+
   // close() shouldn't be called if the session is already closed. Since the
   // operation is asynchronous, there is a window where close() was called
   // just before the closed event arrives. The CDM should handle the case where
   // close() is called after it has already closed the session. However, if
   // we can tell the session is now closed, simply resolve the promise.
   if (is_closed_) {
+    LOG(INFO) << "(" << static_cast<void*>(this) << "), " << __func__
+              << ", session id: " << session_id_ << ", already closed, ignore";
     result.Complete();
     return;
   }
@@ -513,6 +526,9 @@ void WebContentDecryptionModuleSessionImpl::OnSessionClosed(
     media::CdmSessionClosedReason reason) {
   DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
 
+  LOG(INFO) << "(" << static_cast<void*>(this) << "), " << __func__
+            << ", session id: " << session_id_;
+
   // Only send one closed event to blink.
   if (is_closed_)
     return;

diff --git a/tizen_src/chromium_impl/media/filters/ieme_drm_bridge.cc b/tizen_src/chromium_impl/media/filters/ieme_drm_bridge.cc
index 5fec31626e40a207f2290e4e9baf9cb33fe1ccbb..cf5ac00e4b9da565c9efc2df91bc624ff950629d 100644 (file)
--- a/tizen_src/chromium_impl/media/filters/ieme_drm_bridge.cc
+++ b/tizen_src/chromium_impl/media/filters/ieme_drm_bridge.cc
@@ -255,7 +255,8 @@ IEMEDrmBridge::IEMEDrmBridge(
 }
 
 IEMEDrmBridge::~IEMEDrmBridge() {
-  LOG(INFO) << "Destroying IEMEDrmBridge";
+  LOG(INFO) << "(" << static_cast<void*>(this) << ") "
+            << " Destroying IEMEDrmBridge";
 
   // Destroy our CDM so no callbacks are called from this point onwards.
   cdm_.reset();
@@ -605,7 +606,8 @@ void IEMEDrmBridge::CloseSession(
     const std::string& js_session_id,
     std::unique_ptr<media::SimpleCdmPromise> promise) {
   const auto ieme_session_id = session_id_map_.mapJsToIeme(js_session_id);
-  LOG(INFO) << "Closing session id: " << ieme_session_id
+  LOG(INFO) << "(" << static_cast<void*>(this) << ") " << __func__
+            << ", Closing session id: " << ieme_session_id
             << ", js_session_id: " << js_session_id;
 
   const auto close_result_status = cdm_->session_close(ieme_session_id);
@@ -619,11 +621,18 @@ void IEMEDrmBridge::CloseSession(
   session_key_statuses_map_.erase(ieme_session_id);
   last_expiration_times_map_.erase(ieme_session_id);
 
+  LOG(INFO) << "Run close callback for session id: " << js_session_id;
+  session_closed_cb_.Run(js_session_id, CdmSessionClosedReason::kClose);
+  // since session already closed, retrieve key statuses from cdm_ will fail,
+  // no need invoke onKeyStatusesChange anymore
+  // onKeyStatusesChange(ieme_session_id);
+
+  // After dtor of WebContentDecryptionModuleSessionImpl, the string of
+  // session_id will be released, and promise->resolve() will trigger dtor of
+  // WebContentDecryptionModuleSessionImpl, so never use session_id after
+  // promise->resolve()
+  LOG(INFO) << "Resolve promise for session id: " << js_session_id;
   ResolveCdmPromise(promise.get());
-  task_runner_->PostTask(FROM_HERE,
-                         base::BindOnce(session_closed_cb_, js_session_id,
-                                        CdmSessionClosedReason::kClose));
-  onKeyStatusesChange(ieme_session_id);
 }
 
 void IEMEDrmBridge::RemoveSession(