Merge "Validate audio data instead of santinize in binder call." (b0ad7b67) · Commits · e / os / android_frameworks_av

media/libaudioclient/IAudioFlinger.cpp

+4 −4

Original line number	Diff line number	Diff line
		@@ -24,7 +24,7 @@

		#include <binder/IPCThreadState.h>
		#include <binder/Parcel.h>
		#include <media/AudioSanitizer.h>
		#include <media/AudioValidator.h>
		#include <media/IAudioPolicyService.h>
		#include <mediautils/ServiceUtilities.h>
		#include <mediautils/TimeCheck.h>
		@@ -1651,7 +1651,7 @@ status_t BnAudioFlinger::onTransact(
		ALOGE("b/23905951");
		return status;
		}
		status = AudioSanitizer::sanitizeAudioPort(&port);
		status = AudioValidator::validateAudioPort(port);
		if (status == NO_ERROR) {
		status = getAudioPort(&port);
		}
		@@ -1674,7 +1674,7 @@ status_t BnAudioFlinger::onTransact(
		ALOGE("b/23905951");
		return status;
		}
		status = AudioSanitizer::sanitizeAudioPatch(&patch);
		status = AudioValidator::validateAudioPatch(patch);
		if (status == NO_ERROR) {
		status = createAudioPatch(&patch, &handle);
		}
		@@ -1726,7 +1726,7 @@ status_t BnAudioFlinger::onTransact(
		if (status != NO_ERROR) {
		return status;
		}
		status = AudioSanitizer::sanitizeAudioPortConfig(&config);
		status = AudioValidator::validateAudioPortConfig(config);
		if (status == NO_ERROR) {
		status = setAudioPortConfig(&config);
		}

media/libaudioclient/IAudioPolicyService.cpp

+33 −25

Original line number	Diff line number	Diff line
		@@ -26,7 +26,7 @@
		#include <binder/IPCThreadState.h>
		#include <binder/Parcel.h>
		#include <media/AudioEffect.h>
		#include <media/AudioSanitizer.h>
		#include <media/AudioValidator.h>
		#include <media/IAudioPolicyService.h>
		#include <mediautils/ServiceUtilities.h>
		#include <mediautils/TimeCheck.h>
		@@ -529,7 +529,11 @@ public:
		Parcel data, reply;
		data.writeInterfaceToken(IAudioPolicyService::getInterfaceDescriptor());
		data.write(desc, sizeof(effect_descriptor_t));
		remote()->transact(GET_OUTPUT_FOR_EFFECT, data, &reply);
		status_t status = remote()->transact(GET_OUTPUT_FOR_EFFECT, data, &reply);
		if (status != NO_ERROR \|\|
		(status = (status_t)reply.readInt32()) != NO_ERROR) {
		return AUDIO_IO_HANDLE_NONE;
		}
		return static_cast <audio_io_handle_t> (reply.readInt32());
		}

		@@ -1779,13 +1783,15 @@ status_t BnAudioPolicyService::onTransact(
		audio_io_handle_t output = 0;
		std::vector<audio_io_handle_t> secondaryOutputs;

		status = AudioSanitizer::sanitizeAudioAttributes(&attr, "68953950");
		if (status == NO_ERROR) {
		status = AudioValidator::validateAudioAttributes(attr, "68953950");
		if (status != NO_ERROR) {
		reply->writeInt32(status);
		return NO_ERROR;
		}
		status = getOutputForAttr(&attr,
		&output, session, &stream, pid, uid,
		&config,
		flags, &selectedDeviceId, &portId, &secondaryOutputs);
		}
		reply->writeInt32(status);
		status = reply->write(&attr, sizeof(audio_attributes_t));
		if (status != NO_ERROR) {
		@@ -1842,7 +1848,7 @@ status_t BnAudioPolicyService::onTransact(
		audio_port_handle_t selectedDeviceId = (audio_port_handle_t) data.readInt32();
		audio_port_handle_t portId = (audio_port_handle_t)data.readInt32();

		status = AudioSanitizer::sanitizeAudioAttributes(&attr, "68953950");
		status = AudioValidator::validateAudioAttributes(attr, "68953950");
		if (status == NO_ERROR) {
		status = getInputForAttr(&attr, &input, riid, session, pid, uid,
		opPackageName, &config,
		@@ -1932,7 +1938,7 @@ status_t BnAudioPolicyService::onTransact(
		int index = data.readInt32();
		audio_devices_t device = static_cast <audio_devices_t>(data.readInt32());

		status = AudioSanitizer::sanitizeAudioAttributes(&attributes, "169572641");
		status = AudioValidator::validateAudioAttributes(attributes, "169572641");
		if (status == NO_ERROR) {
		status = setVolumeIndexForAttributes(attributes, index, device);
		}
		@@ -1950,7 +1956,7 @@ status_t BnAudioPolicyService::onTransact(
		audio_devices_t device = static_cast <audio_devices_t>(data.readInt32());

		int index = 0;
		status = AudioSanitizer::sanitizeAudioAttributes(&attributes, "169572641");
		status = AudioValidator::validateAudioAttributes(attributes, "169572641");
		if (status == NO_ERROR) {
		status = getVolumeIndexForAttributes(attributes, index, device);
		}
		@@ -1970,7 +1976,7 @@ status_t BnAudioPolicyService::onTransact(
		}

		int index = 0;
		status = AudioSanitizer::sanitizeAudioAttributes(&attributes, "169572641");
		status = AudioValidator::validateAudioAttributes(attributes, "169572641");
		if (status == NO_ERROR) {
		status = getMinVolumeIndexForAttributes(attributes, index);
		}
		@@ -1990,7 +1996,7 @@ status_t BnAudioPolicyService::onTransact(
		}

		int index = 0;
		status = AudioSanitizer::sanitizeAudioAttributes(&attributes, "169572641");
		status = AudioValidator::validateAudioAttributes(attributes, "169572641");
		if (status == NO_ERROR) {
		status = getMaxVolumeIndexForAttributes(attributes, index);
		}
		@@ -2017,12 +2023,12 @@ status_t BnAudioPolicyService::onTransact(
		android_errorWriteLog(0x534e4554, "73126106");
		return status;
		}
		audio_io_handle_t output = AUDIO_IO_HANDLE_NONE;
		status = AudioSanitizer::sanitizeEffectDescriptor(&desc, "73126106");
		status = AudioValidator::validateEffectDescriptor(desc, "73126106");
		reply->writeInt32(status);
		if (status == NO_ERROR) {
		output = getOutputForEffect(&desc);
		}
		audio_io_handle_t output = getOutputForEffect(&desc);
		reply->writeInt32(static_cast <int32_t>(output));
		}
		return NO_ERROR;
		} break;

		@@ -2038,7 +2044,7 @@ status_t BnAudioPolicyService::onTransact(
		uint32_t strategy = data.readInt32();
		audio_session_t session = (audio_session_t) data.readInt32();
		int id = data.readInt32();
		status = AudioSanitizer::sanitizeEffectDescriptor(&desc, "73126106");
		status = AudioValidator::validateEffectDescriptor(desc, "73126106");
		if (status == NO_ERROR) {
		status = registerEffect(&desc, io, strategy, session, id);
		}
		@@ -2151,7 +2157,7 @@ status_t BnAudioPolicyService::onTransact(
		if (status != NO_ERROR) return status;
		status = data.read(&attributes, sizeof(audio_attributes_t));
		if (status != NO_ERROR) return status;
		status = AudioSanitizer::sanitizeAudioAttributes(&attributes, "169572641");
		status = AudioValidator::validateAudioAttributes(attributes, "169572641");
		if (status == NO_ERROR) {
		status = isDirectOutputSupported(config, attributes);
		}
		@@ -2199,7 +2205,7 @@ status_t BnAudioPolicyService::onTransact(
		ALOGE("b/23912202");
		return status;
		}
		status = AudioSanitizer::sanitizeAudioPort(&port);
		status = AudioValidator::validateAudioPort(port);
		if (status == NO_ERROR) {
		status = getAudioPort(&port);
		}
		@@ -2223,7 +2229,7 @@ status_t BnAudioPolicyService::onTransact(
		ALOGE("b/23912202");
		return status;
		}
		status = AudioSanitizer::sanitizeAudioPatch(&patch);
		status = AudioValidator::validateAudioPatch(patch);
		if (status == NO_ERROR) {
		status = createAudioPatch(&patch, &handle);
		}
		@@ -2280,8 +2286,10 @@ status_t BnAudioPolicyService::onTransact(
		if (status != NO_ERROR) {
		return status;
		}
		(void)AudioSanitizer::sanitizeAudioPortConfig(&config);
		status = AudioValidator::validateAudioPortConfig(config);
		if (status == NO_ERROR) {
		status = setAudioPortConfig(&config);
		}
		reply->writeInt32(status);
		return NO_ERROR;
		}
		@@ -2366,11 +2374,11 @@ status_t BnAudioPolicyService::onTransact(
		if (status != NO_ERROR) {
		return status;
		}
		status = AudioSanitizer::sanitizeAudioPortConfig(&source);
		status = AudioValidator::validateAudioPortConfig(source);
		if (status == NO_ERROR) {
		// OK to not always sanitize attributes as startAudioSource() is not called if
		// the port config is invalid.
		status = AudioSanitizer::sanitizeAudioAttributes(&attributes, "68953950");
		status = AudioValidator::validateAudioAttributes(attributes, "68953950");
		}
		audio_port_handle_t portId = AUDIO_PORT_HANDLE_NONE;
		if (status == NO_ERROR) {

media/libmediahelper/Android.bp

+1 −1

Original line number	Diff line number	Diff line
		@@ -20,7 +20,7 @@ cc_library {
		double_loadable: true,
		srcs: [
		"AudioParameter.cpp",
		"AudioSanitizer.cpp",
		"AudioValidator.cpp",
		"TypeConverter.cpp",
		],
		cflags: [

media/libmediahelper/AudioSanitizer.cpp→media/libmediahelper/AudioValidator.cpp

+124 −0

Original line number	Diff line number	Diff line
		@@ -14,56 +14,51 @@
		* limitations under the License.
		*/

		#include <media/AudioSanitizer.h>
		#include <media/AudioValidator.h>

		namespace android {

		/** returns true if string overflow was prevented by zero termination */
		/** returns true if string is overflow */
		template <size_t size>
		bool preventStringOverflow(char (&s)[size]) {
		if (strnlen(s, size) < size) return false;
		s[size - 1] = '\0';
		return true;
		bool checkStringOverflow(const char (&s)[size]) {
		return strnlen(s, size) >= size;
		}

		status_t safetyNetLog(status_t status, const char *bugNumber) {
		if (status != NO_ERROR && bugNumber != nullptr) {
		android_errorWriteLog(0x534e4554, bugNumber); // SafetyNet logging
		status_t safetyNetLog(status_t status, std::string_view bugNumber) {
		if (status != NO_ERROR && !bugNumber.empty()) {
		android_errorWriteLog(0x534e4554, bugNumber.data()); // SafetyNet logging
		}
		return status;
		}

		status_t AudioSanitizer::sanitizeAudioAttributes(
		audio_attributes_t attr, const char bugNumber)
		status_t AudioValidator::validateAudioAttributes(
		const audio_attributes_t& attr, std::string_view bugNumber)
		{
		status_t status = NO_ERROR;
		const size_t tagsMaxSize = AUDIO_ATTRIBUTES_TAGS_MAX_SIZE;
		if (strnlen(attr->tags, tagsMaxSize) >= tagsMaxSize) {
		if (strnlen(attr.tags, tagsMaxSize) >= tagsMaxSize) {
		status = BAD_VALUE;
		}
		attr->tags[tagsMaxSize - 1] = '\0';
		return safetyNetLog(status, bugNumber);
		}

		/** returns BAD_VALUE if sanitization was required. */
		status_t AudioSanitizer::sanitizeEffectDescriptor(
		effect_descriptor_t desc, const char bugNumber)
		status_t AudioValidator::validateEffectDescriptor(
		const effect_descriptor_t& desc, std::string_view bugNumber)
		{
		status_t status = NO_ERROR;
		if (preventStringOverflow(desc->name)
		\| /* always */ preventStringOverflow(desc->implementor)) {
		if (checkStringOverflow(desc.name)
		\| /* always */ checkStringOverflow(desc.implementor)) {
		status = BAD_VALUE;
		}
		return safetyNetLog(status, bugNumber);
		}

		/** returns BAD_VALUE if sanitization was required. */
		status_t AudioSanitizer::sanitizeAudioPortConfig(
		struct audio_port_config config, const char bugNumber)
		status_t AudioValidator::validateAudioPortConfig(
		const struct audio_port_config& config, std::string_view bugNumber)
		{
		status_t status = NO_ERROR;
		if (config->type == AUDIO_PORT_TYPE_DEVICE &&
		preventStringOverflow(config->ext.device.address)) {
		if (config.type == AUDIO_PORT_TYPE_DEVICE &&
		checkStringOverflow(config.ext.device.address)) {
		status = BAD_VALUE;
		}
		return safetyNetLog(status, bugNumber);
		@@ -73,16 +68,16 @@ namespace {

		template <typename T, std::enable_if_t<std::is_same<T, struct audio_port>::value
		\|\| std::is_same<T, struct audio_port_v7>::value, int> = 0>
		static status_t sanitizeAudioPortInternal(T port, const char bugNumber = nullptr) {
		static status_t validateAudioPortInternal(const T& port, std::string_view bugNumber = {}) {
		status_t status = NO_ERROR;
		if (preventStringOverflow(port->name)) {
		if (checkStringOverflow(port.name)) {
		status = BAD_VALUE;
		}
		if (AudioSanitizer::sanitizeAudioPortConfig(&port->active_config) != NO_ERROR) {
		if (AudioValidator::validateAudioPortConfig(port.active_config) != NO_ERROR) {
		status = BAD_VALUE;
		}
		if (port->type == AUDIO_PORT_TYPE_DEVICE &&
		preventStringOverflow(port->ext.device.address)) {
		if (port.type == AUDIO_PORT_TYPE_DEVICE &&
		checkStringOverflow(port.ext.device.address)) {
		status = BAD_VALUE;
		}
		return safetyNetLog(status, bugNumber);
		@@ -90,40 +85,36 @@ static status_t sanitizeAudioPortInternal(T port, const char bugNumber = nullp

		} // namespace

		/** returns BAD_VALUE if sanitization was required. */
		status_t AudioSanitizer::sanitizeAudioPort(
		struct audio_port port, const char bugNumber)
		status_t AudioValidator::validateAudioPort(
		const struct audio_port& port, std::string_view bugNumber)
		{
		return sanitizeAudioPortInternal(port, bugNumber);
		return validateAudioPortInternal(port, bugNumber);
		}

		/** returns BAD_VALUE if sanitization was required. */
		status_t AudioSanitizer::sanitizeAudioPort(
		struct audio_port_v7 port, const char bugNumber)
		status_t AudioValidator::validateAudioPort(
		const struct audio_port_v7& port, std::string_view bugNumber)
		{
		return sanitizeAudioPortInternal(port, bugNumber);
		return validateAudioPortInternal(port, bugNumber);
		}

		/** returns BAD_VALUE if sanitization was required. */
		status_t AudioSanitizer::sanitizeAudioPatch(
		struct audio_patch patch, const char bugNumber)
		status_t AudioValidator::validateAudioPatch(
		const struct audio_patch& patch, std::string_view bugNumber)
		{
		status_t status = NO_ERROR;
		if (patch->num_sources > AUDIO_PATCH_PORTS_MAX) {
		patch->num_sources = AUDIO_PATCH_PORTS_MAX;
		if (patch.num_sources > AUDIO_PATCH_PORTS_MAX) {
		status = BAD_VALUE;
		}
		if (patch->num_sinks > AUDIO_PATCH_PORTS_MAX) {
		patch->num_sinks = AUDIO_PATCH_PORTS_MAX;
		if (patch.num_sinks > AUDIO_PATCH_PORTS_MAX) {
		status = BAD_VALUE;
		}
		for (size_t i = 0; i < patch->num_sources; i++) {
		if (sanitizeAudioPortConfig(&patch->sources[i]) != NO_ERROR) {
		for (size_t i = 0; i < patch.num_sources; i++) {
		if (validateAudioPortConfig(patch.sources[i]) != NO_ERROR) {
		status = BAD_VALUE;
		}
		}
		for (size_t i = 0; i < patch->num_sinks; i++) {
		if (sanitizeAudioPortConfig(&patch->sinks[i]) != NO_ERROR) {
		for (size_t i = 0; i < patch.num_sinks; i++) {
		if (validateAudioPortConfig(patch.sinks[i]) != NO_ERROR) {
		status = BAD_VALUE;
		}
		}

media/libmediahelper/include/media/AudioSanitizer.h→media/libmediahelper/include/media/AudioValidator.h

+80 −0

Original line number	Diff line number	Diff line
		@@ -14,37 +14,67 @@
		* limitations under the License.
		*/

		#ifndef ANDROID_AUDIO_SANITIZER_H_
		#define ANDROID_AUDIO_SANITIZER_H_
		#ifndef ANDROID_AUDIO_VALIDATOR_H_
		#define ANDROID_AUDIO_VALIDATOR_H_

		#include <system/audio.h>
		#include <system/audio_effect.h>
		#include <utils/Errors.h>
		#include <utils/Log.h>

		#include <string_view>

		namespace android {

		class AudioSanitizer {
		/**
		* AudioValidator is a class to validate audio data in binder call. NO_ERROR will be returned only
		* when there is no error with the data.
		*/
		class AudioValidator {
		public:
		static status_t sanitizeAudioAttributes(
		audio_attributes_t attr, const char bugNumber = nullptr);
		/**
		* Return NO_ERROR only when there is no error with the given audio attributes.
		* Otherwise, return BAD_VALUE.
		*/
		static status_t validateAudioAttributes(
		const audio_attributes_t& attr, std::string_view bugNumber = {});

		static status_t sanitizeEffectDescriptor(
		effect_descriptor_t desc, const char bugNumber = nullptr);
		/**
		* Return NO_ERROR only when there is no error with the given effect descriptor.
		* Otherwise, return BAD_VALUE.
		*/
		static status_t validateEffectDescriptor(
		const effect_descriptor_t& desc, std::string_view bugNumber = {});

		static status_t sanitizeAudioPortConfig(
		struct audio_port_config config, const char bugNumber = nullptr);
		/**
		* Return NO_ERROR only when there is no error with the given audio port config.
		* Otherwise, return BAD_VALUE.
		*/
		static status_t validateAudioPortConfig(
		const struct audio_port_config& config, std::string_view bugNumber = {});

		static status_t sanitizeAudioPort(
		struct audio_port port, const char bugNumber = nullptr);
		/**
		* Return NO_ERROR only when there is no error with the given audio port.
		* Otherwise, return BAD_VALUE.
		*/
		static status_t validateAudioPort(
		const struct audio_port& port, std::string_view bugNumber = {});

		static status_t sanitizeAudioPort(
		struct audio_port_v7 port, const char bugNumber = nullptr);
		/**
		* Return NO_ERROR only when there is no error with the given audio_port_v7.
		* Otherwise, return BAD_VALUE.
		*/
		static status_t validateAudioPort(
		const struct audio_port_v7& port, std::string_view ugNumber = {});

		static status_t sanitizeAudioPatch(
		struct audio_patch patch, const char bugNumber = nullptr);
		/**
		* Return NO_ERROR only when there is no error with the given audio patch.
		* Otherwise, return BAD_VALUE.
		*/
		static status_t validateAudioPatch(
		const struct audio_patch& patch, std::string_view bugNumber = {});
		};

		}; // namespace android

		#endif /ANDROID_AUDIO_SANITIZER_H_/
		#endif /ANDROID_AUDIO_VALIDATOR_H_/