Merge "Effect config parser: fix use after free on file path" (afa8c702) · Commits · e / os / android_frameworks_av

media/libeffects/config/include/media/EffectsConfig.h

+1 −1

Original line number	Original line	Diff line number	Diff line
	@@ -96,7 +96,7 @@ struct ParsingResult {
	/** Parsed config, nullptr if the xml lib could not load the file */		/** Parsed config, nullptr if the xml lib could not load the file */
	std::unique_ptr<Config> parsedConfig;		std::unique_ptr<Config> parsedConfig;
	size_t nbSkippedElement; //< Number of skipped invalid library, effect or processing chain		size_t nbSkippedElement; //< Number of skipped invalid library, effect or processing chain
	const char* configPath; //< Path to the loaded configuration		const std::string configPath; //< Path to the loaded configuration
	};		};

	/** Parses the provided effect configuration.		/** Parses the provided effect configuration.

media/libeffects/config/src/EffectsConfig.cpp

+8 −8

Original line number	Original line	Diff line number	Diff line
	@@ -250,14 +250,14 @@ bool parseStream(const XMLElement& xmlStream, Effects& effects, std::vector<Stre
	return true;		return true;
	}		}

	/** Internal version of the public parse(const char* path) with precondition `path != nullptr`. */		/** Internal version of the public parse(const char* path) where path always exist. */
	ParsingResult parseWithPath(const char* path) {		ParsingResult parseWithPath(std::string&& path) {
	XMLDocument doc;		XMLDocument doc;
	doc.LoadFile(path);		doc.LoadFile(path.c_str());
	if (doc.Error()) {		if (doc.Error()) {
	ALOGE("Failed to parse %s: Tinyxml2 error (%d): %s", path,		ALOGE("Failed to parse %s: Tinyxml2 error (%d): %s", path.c_str(),
	doc.ErrorID(), doc.ErrorStr());		doc.ErrorID(), doc.ErrorStr());
	return {nullptr, 0, path};		return {nullptr, 0, std::move(path)};
	}		}

	auto config = std::make_unique<Config>();		auto config = std::make_unique<Config>();
	@@ -295,7 +295,7 @@ ParsingResult parseWithPath(const char* path) {
	}		}
	}		}
	}		}
	return {std::move(config), nbSkippedElements, path};		return {std::move(config), nbSkippedElements, std::move(path)};
	}		}

	}; // namespace		}; // namespace
	@@ -310,14 +310,14 @@ ParsingResult parse(const char* path) {
	if (access(defaultPath.c_str(), R_OK) != 0) {		if (access(defaultPath.c_str(), R_OK) != 0) {
	continue;		continue;
	}		}
	auto result = parseWithPath(defaultPath.c_str());		auto result = parseWithPath(std::move(defaultPath));
	if (result.parsedConfig != nullptr) {		if (result.parsedConfig != nullptr) {
	return result;		return result;
	}		}
	}		}

	ALOGE("Could not parse effect configuration in any of the default locations.");		ALOGE("Could not parse effect configuration in any of the default locations.");
	return {nullptr, 0, nullptr};		return {nullptr, 0, ""};
	}		}

	} // namespace effectsConfig		} // namespace effectsConfig

media/libeffects/factory/EffectsXmlConfigLoader.cpp

+2 −1

Original line number	Original line	Diff line number	Diff line
	@@ -327,7 +327,8 @@ extern "C" ssize_t EffectLoadXmlEffectConfig(const char* path)
	&gSkippedEffects, &gSubEffectList);		&gSkippedEffects, &gSubEffectList);

	ALOGE_IF(result.nbSkippedElement != 0, "%zu errors during loading of configuration: %s",		ALOGE_IF(result.nbSkippedElement != 0, "%zu errors during loading of configuration: %s",
	result.nbSkippedElement, result.configPath ?: "No config file found");		result.nbSkippedElement,
			result.configPath.empty() ? "No config file found" : result.configPath.c_str());

	return result.nbSkippedElement;		return result.nbSkippedElement;
	}		}