Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 78b761da authored by jiabin's avatar jiabin
Browse files

Validate audio data instead of santinize in binder call. 

In binder call, if the data is silently santinized, there may be
unexpected behavior. In that case, it is better to just validate the
data and returned error code if there is any error with the data.

Test: audio smoke test
Bug: 174189330
Change-Id: I2f0534f3013f9127a441306c3ff7d5ca8cc0e223
parent 4106a42a

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_*/