summaryrefslogtreecommitdiffstats
path: root/Source
diff options
context:
space:
mode:
authorBrad King <brad.king@kitware.com>2020-03-18 13:51:46 (GMT)
committerBrad King <brad.king@kitware.com>2020-03-19 10:41:39 (GMT)
commit8affe9aa336b873e9c8e40ec5911ffe23c2ef03a (patch)
tree0d2b79f86c604c23d3ceff99a69a93fcb9b6c38a /Source
parent1ec72e09471287630cf142d8587a9b8d9abad629 (diff)
downloadCMake-8affe9aa336b873e9c8e40ec5911ffe23c2ef03a.zip
CMake-8affe9aa336b873e9c8e40ec5911ffe23c2ef03a.tar.gz
CMake-8affe9aa336b873e9c8e40ec5911ffe23c2ef03a.tar.bz2
export: Fix use-after-free on multiple calls overwriting same FILE
CMake 3.16 and below allow multiple `export()` calls with the same output file even without using `APPEND`. The implementation worked by accident by leaking memory. Refactoring in commit 5444a8095d (cmGlobalGenerator: modernize memrory managemenbt, 2019-12-29, v3.17.0-rc1~239^2) cleaned up that memory leak and converted it to a use-after-free instead. The problem is caused by using the `cmGlobalGenerator::BuildExportSets` map to own `cmExportBuildFileGenerator` instances. It can own only one instance per output FILE name at a time, so repeating use of the same file now frees the old `cmExportBuildFileGenerator` instance and leaves the pointer in the `cmMakefile::ExportBuildFileGenerators` vector dangling. Move ownership of the instances into `cmMakefile`'s vector since its entries are not replaced on a repeat output FILE. In future work we should introduce a policy to error out on this case. For now simply fix the use-after-free to restore CMake <= 3.16 behavior. Fixes: #20469
Diffstat (limited to 'Source')
-rw-r--r--Source/cmExportCommand.cxx6
-rw-r--r--Source/cmGlobalGenerator.cxx19
-rw-r--r--Source/cmGlobalGenerator.h10
-rw-r--r--Source/cmMakefile.cxx16
-rw-r--r--Source/cmMakefile.h10
5 files changed, 33 insertions, 28 deletions
diff --git a/Source/cmExportCommand.cxx b/Source/cmExportCommand.cxx
index b7cc193..e49c174 100644
--- a/Source/cmExportCommand.cxx
+++ b/Source/cmExportCommand.cxx
@@ -198,7 +198,6 @@ bool cmExportCommand(std::vector<std::string> const& args,
} else {
ebfg->SetTargets(targets);
}
- mf.AddExportBuildFileGenerator(ebfg.get());
ebfg->SetExportOld(arguments.ExportOld);
// Compute the set of configurations exported.
@@ -211,10 +210,11 @@ bool cmExportCommand(std::vector<std::string> const& args,
ebfg->AddConfiguration(ct);
}
if (exportSet != nullptr) {
- gg->AddBuildExportExportSet(std::move(ebfg));
+ gg->AddBuildExportExportSet(ebfg.get());
} else {
- gg->AddBuildExportSet(std::move(ebfg));
+ gg->AddBuildExportSet(ebfg.get());
}
+ mf.AddExportBuildFileGenerator(std::move(ebfg));
return true;
}
diff --git a/Source/cmGlobalGenerator.cxx b/Source/cmGlobalGenerator.cxx
index 0404715..6a2d4c7 100644
--- a/Source/cmGlobalGenerator.cxx
+++ b/Source/cmGlobalGenerator.cxx
@@ -262,17 +262,16 @@ void cmGlobalGenerator::ResolveLanguageCompiler(const std::string& lang,
}
}
-void cmGlobalGenerator::AddBuildExportSet(
- std::unique_ptr<cmExportBuildFileGenerator> gen)
+void cmGlobalGenerator::AddBuildExportSet(cmExportBuildFileGenerator* gen)
{
- this->BuildExportSets[gen->GetMainExportFileName()] = std::move(gen);
+ this->BuildExportSets[gen->GetMainExportFileName()] = gen;
}
void cmGlobalGenerator::AddBuildExportExportSet(
- std::unique_ptr<cmExportBuildFileGenerator> gen)
+ cmExportBuildFileGenerator* gen)
{
- this->BuildExportExportSets[gen->GetMainExportFileName()] = gen.get();
- this->AddBuildExportSet(std::move(gen));
+ this->BuildExportExportSets[gen->GetMainExportFileName()] = gen;
+ this->AddBuildExportSet(gen);
}
bool cmGlobalGenerator::GenerateImportFile(const std::string& file)
@@ -283,7 +282,7 @@ bool cmGlobalGenerator::GenerateImportFile(const std::string& file)
if (!this->ConfigureDoneCMP0026AndCMP0024) {
for (const auto& m : this->Makefiles) {
- m->RemoveExportBuildFileGeneratorCMP0024(it->second.get());
+ m->RemoveExportBuildFileGeneratorCMP0024(it->second);
}
}
@@ -1317,7 +1316,7 @@ cmExportBuildFileGenerator* cmGlobalGenerator::GetExportedTargetsFile(
const std::string& filename) const
{
auto const it = this->BuildExportSets.find(filename);
- return it == this->BuildExportSets.end() ? nullptr : it->second.get();
+ return it == this->BuildExportSets.end() ? nullptr : it->second;
}
void cmGlobalGenerator::AddCMP0042WarnTarget(const std::string& target)
@@ -1353,9 +1352,9 @@ bool cmGlobalGenerator::CheckALLOW_DUPLICATE_CUSTOM_TARGETS() const
void cmGlobalGenerator::ComputeBuildFileGenerators()
{
for (unsigned int i = 0; i < this->LocalGenerators.size(); ++i) {
- std::vector<cmExportBuildFileGenerator*> gens =
+ std::vector<std::unique_ptr<cmExportBuildFileGenerator>> const& gens =
this->Makefiles[i]->GetExportBuildFileGenerators();
- for (cmExportBuildFileGenerator* g : gens) {
+ for (std::unique_ptr<cmExportBuildFileGenerator> const& g : gens) {
g->Compute(this->LocalGenerators[i].get());
}
}
diff --git a/Source/cmGlobalGenerator.h b/Source/cmGlobalGenerator.h
index ba997b2..7dc4822 100644
--- a/Source/cmGlobalGenerator.h
+++ b/Source/cmGlobalGenerator.h
@@ -463,13 +463,12 @@ public:
void ProcessEvaluationFiles();
- std::map<std::string, std::unique_ptr<cmExportBuildFileGenerator>>&
- GetBuildExportSets()
+ std::map<std::string, cmExportBuildFileGenerator*>& GetBuildExportSets()
{
return this->BuildExportSets;
}
- void AddBuildExportSet(std::unique_ptr<cmExportBuildFileGenerator>);
- void AddBuildExportExportSet(std::unique_ptr<cmExportBuildFileGenerator>);
+ void AddBuildExportSet(cmExportBuildFileGenerator* gen);
+ void AddBuildExportExportSet(cmExportBuildFileGenerator* gen);
bool IsExportedTargetsFile(const std::string& filename) const;
bool GenerateImportFile(const std::string& file);
cmExportBuildFileGenerator* GetExportedTargetsFile(
@@ -580,8 +579,7 @@ protected:
std::set<std::string> InstallComponents;
// Sets of named target exports
cmExportSetMap ExportSets;
- std::map<std::string, std::unique_ptr<cmExportBuildFileGenerator>>
- BuildExportSets;
+ std::map<std::string, cmExportBuildFileGenerator*> BuildExportSets;
std::map<std::string, cmExportBuildFileGenerator*> BuildExportExportSets;
std::map<std::string, std::string> AliasTargets;
diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx
index b2e59bd..18689fa 100644
--- a/Source/cmMakefile.cxx
+++ b/Source/cmMakefile.cxx
@@ -32,6 +32,7 @@
#include "cmCustomCommandLines.h"
#include "cmExecutionStatus.h"
#include "cmExpandedCommandArgument.h" // IWYU pragma: keep
+#include "cmExportBuildFileGenerator.h"
#include "cmFileLockPool.h"
#include "cmFunctionBlocker.h"
#include "cmGeneratedFileStream.h"
@@ -780,7 +781,7 @@ cmMakefile::GetEvaluationFiles() const
return this->EvaluationFiles;
}
-std::vector<cmExportBuildFileGenerator*>
+std::vector<std::unique_ptr<cmExportBuildFileGenerator>> const&
cmMakefile::GetExportBuildFileGenerators() const
{
return this->ExportBuildFileGenerators;
@@ -789,16 +790,21 @@ cmMakefile::GetExportBuildFileGenerators() const
void cmMakefile::RemoveExportBuildFileGeneratorCMP0024(
cmExportBuildFileGenerator* gen)
{
- auto it = std::find(this->ExportBuildFileGenerators.begin(),
- this->ExportBuildFileGenerators.end(), gen);
+ auto it =
+ std::find_if(this->ExportBuildFileGenerators.begin(),
+ this->ExportBuildFileGenerators.end(),
+ [gen](std::unique_ptr<cmExportBuildFileGenerator> const& p) {
+ return p.get() == gen;
+ });
if (it != this->ExportBuildFileGenerators.end()) {
this->ExportBuildFileGenerators.erase(it);
}
}
-void cmMakefile::AddExportBuildFileGenerator(cmExportBuildFileGenerator* gen)
+void cmMakefile::AddExportBuildFileGenerator(
+ std::unique_ptr<cmExportBuildFileGenerator> gen)
{
- this->ExportBuildFileGenerators.push_back(gen);
+ this->ExportBuildFileGenerators.emplace_back(std::move(gen));
}
namespace {
diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h
index f1a68c2..d918abe 100644
--- a/Source/cmMakefile.h
+++ b/Source/cmMakefile.h
@@ -945,10 +945,11 @@ public:
const std::vector<std::unique_ptr<cmGeneratorExpressionEvaluationFile>>&
GetEvaluationFiles() const;
- std::vector<cmExportBuildFileGenerator*> GetExportBuildFileGenerators()
- const;
+ std::vector<std::unique_ptr<cmExportBuildFileGenerator>> const&
+ GetExportBuildFileGenerators() const;
void RemoveExportBuildFileGeneratorCMP0024(cmExportBuildFileGenerator* gen);
- void AddExportBuildFileGenerator(cmExportBuildFileGenerator* gen);
+ void AddExportBuildFileGenerator(
+ std::unique_ptr<cmExportBuildFileGenerator> gen);
// Maintain a stack of package roots to allow nested PACKAGE_ROOT_PATH
// searches
@@ -1053,7 +1054,8 @@ private:
mutable cmsys::RegularExpression cmNamedCurly;
std::vector<cmMakefile*> UnConfiguredDirectories;
- std::vector<cmExportBuildFileGenerator*> ExportBuildFileGenerators;
+ std::vector<std::unique_ptr<cmExportBuildFileGenerator>>
+ ExportBuildFileGenerators;
std::vector<std::unique_ptr<cmGeneratorExpressionEvaluationFile>>
EvaluationFiles;