widevine-partner/android
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fixed dangling pointer issue in CdmInfo.

Browse Source 
[ Cherry-pick of http://ag/34331848 ]
[ Merge of http://go/wvgerrit/224951 ]

There was a potential dangling pointer issue that was enabled
by how CdmInfo is initilized.  The file system that was passed
into the CdmEngine instance was pointing to a location in memory
that was not stable between move operations in the CdmInfo.
See b/429054262 for memory diagram of issue.

The CdmInfo is a private class within the Android CDM class, which
restricts the potential operations on it.  The easiest solution
is wrap the file system in a unique pointer; ensuring the pointer
remains stable even if a particular data segment of CdmInfo is
moved.

The default constructor for CdmInfo is deleted; this will force the
compiler to fail if |cdms_| is used in ways that would result in
uninitialized pointers.

Bug: 429054262
Test: WvTs on Komodo
Change-Id: I76a49fc5181ebd1613e238aa49986083a9f397ec
(cherry picked from commit 4c105faa4923bd9bd6352f757dedf3eaf9ed88fd)
This commit is contained in:
Alex Dale
2025-07-02 14:42:52 -07:00
parent e2fba86327
commit d7eab65c8c
2 changed files with 61 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files

26
libwvdrmengine/cdm/include/wv_content_decryption_module.h
View File

@@ -299,11 +299,29 @@ class WvContentDecryptionModule : public android::RefBase, public TimerHandler {
const std::string& signature);
private:
struct CdmInfo {
CdmInfo();
class CdmInfo {
public:
// This should never be used.
CdmInfo() = delete;
// It is expected that the filesystem loaded into |cdm_engine|
// is the same instance as |file_system|.
CdmInfo(std::unique_ptr<wvutil::FileSystem>&& file_system,
std::unique_ptr<CdmEngine>&& cdm_engine);
// No copy operators.
CdmInfo(const CdmInfo&) = delete;
CdmInfo& operator=(const CdmInfo&) = delete;
// Move operators OK.
CdmInfo(CdmInfo&&) = default;
CdmInfo& operator==(CdmInfo&& other);
wvutil::FileSystem file_system;
std::unique_ptr<CdmEngine> cdm_engine;
wvutil::FileSystem* file_system() { return file_system_.get(); }
CdmEngine* cdm_engine() { return cdm_engine_.get(); }
private:
// Order matters, |cdm_engine_| is expected to contain a pointer
// to |file_system_|.
std::unique_ptr<wvutil::FileSystem> file_system_;
std::unique_ptr<CdmEngine> cdm_engine_;
};
// Finds the CdmEngine instance for the given identifier, creating one if

57
libwvdrmengine/cdm/src/wv_content_decryption_module.cpp
View File

@@ -566,7 +566,7 @@ CdmResponseType WvContentDecryptionModule::GetCurrentMetricsInternal(
// TODO(blueeyes): Add a better error.
return CdmResponseType(UNKNOWN_ERROR);
}
return it->second.cdm_engine->GetMetricsSnapshot(metrics)
return it->second.cdm_engine()->GetMetricsSnapshot(metrics)
? CdmResponseType(NO_ERROR)
: CdmResponseType(UNKNOWN_ERROR);
}
@@ -582,31 +582,52 @@ void WvContentDecryptionModule::SaveMetrics(
WvMetricsSnapshot::MakeSnapshot(identifier, std::move(metrics)));
}
WvContentDecryptionModule::CdmInfo::CdmInfo()
: cdm_engine(CdmEngineFactory::CreateCdmEngine(&file_system)) {}
WvContentDecryptionModule::CdmInfo::CdmInfo(
std::unique_ptr<wvutil::FileSystem>&& file_system,
std::unique_ptr<CdmEngine>&& cdm_engine)
: file_system_(std::move(file_system)),
cdm_engine_(std::move(cdm_engine)) {}
WvContentDecryptionModule::CdmInfo&
WvContentDecryptionModule::CdmInfo::operator==(
WvContentDecryptionModule::CdmInfo&& other) {
// Must move |cdm_engine_| first; the current value depends
// on the current value of |file_system_|.
cdm_engine_ = std::move(other.cdm_engine_);
file_system_ = std::move(other.file_system_);
return *this;
}
CdmEngine* WvContentDecryptionModule::EnsureCdmForIdentifier(
const CdmIdentifier& identifier) {
CdmEngine* cdm_engine;
CdmEngine* cdm_engine = nullptr;
bool enable_timer = false;
{
std::unique_lock<std::mutex> auto_lock(cdms_lock_);
if (cdms_.size() == 0) enable_timer = true;
if (cdms_.find(identifier) == cdms_.end()) {
// Accessing the map entry will create a new instance using the default
// constructor. We then need to provide it with two pieces of info: The
if (cdms_.empty()) enable_timer = true;
auto it = cdms_.find(identifier);
if (it != cdms_.end()) {
// Already exists.
cdm_engine = it->second.cdm_engine();
} else {
std::unique_ptr<wvutil::FileSystem> fs =
std::make_unique<wvutil::FileSystem>();
// Provide it with two pieces of info: The
// origin provided by the app and an identifier that uniquely identifies
// this CDM. We concatenate all pieces of the CdmIdentifier in order to
// create an ID that is unique to that identifier.
cdms_[identifier].file_system.set_origin(identifier.origin);
cdms_[identifier].file_system.set_identifier(identifier.spoid +
identifier.origin);
cdms_[identifier].cdm_engine->SetAppPackageName(
identifier.app_package_name);
cdms_[identifier].cdm_engine->SetSpoid(identifier.spoid);
cdms_[identifier].cdm_engine->SetUserId(identifier.user_id);
fs->set_origin(identifier.origin);
fs->set_identifier(identifier.spoid + identifier.origin);
std::unique_ptr<CdmEngine> engine(
CdmEngineFactory::CreateCdmEngine(fs.get()));
engine->SetAppPackageName(identifier.app_package_name);
engine->SetSpoid(identifier.spoid);
engine->SetUserId(identifier.user_id);
cdm_engine = engine.get();
cdms_.emplace(identifier, CdmInfo(std::move(fs), std::move(engine)));
}
cdm_engine = cdms_[identifier].cdm_engine.get();
}
// Do not enable timer while holding on to the |cdms_lock_|
if (enable_timer) EnableTimer();
@@ -649,7 +670,7 @@ CdmResponseType WvContentDecryptionModule::CloseCdm(
// TODO(blueeyes): Create a better error.
return CdmResponseType(UNKNOWN_ERROR);
}
CdmEngine* cdm_engine = cdm_it->second.cdm_engine.get();
CdmEngine* cdm_engine = cdm_it->second.cdm_engine();
// Save metrics snapshot.
drm_metrics::WvCdmMetrics metrics;
const bool success = cdm_engine->GetMetricsSnapshot(&metrics);
@@ -738,7 +759,7 @@ void WvContentDecryptionModule::OnTimerEvent() {
std::unique_lock<std::mutex> auto_lock(cdms_lock_);
for (auto it = cdms_.begin(); it != cdms_.end(); ++it) {
LoggingUidSetter set_uid(it->first.user_id);
it->second.cdm_engine->OnTimerEvent();
it->second.cdm_engine()->OnTimerEvent();
}
if (cdms_.empty()) {
// The following code cannot be attributed to any app uid.