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

Commit b1cf0316 authored by Marco Nelissen's avatar Marco Nelissen
Browse files

Move overflow checks into SkipCutBuffer 

Previously SkipCutBuffer would check its input parameters to ensure
they were sane, however since bogus values might be the result of
overflows, and overflow protection was recently turned on for
libstagefright, the compiler's overflow checks were performed before
SkipCutBuffer's, resulting in abort rather than just ignoring the
bogus values.
Moving the multiplication by framesize into SkipCutBuffer fixes this.

Change-Id: I1ad6744bb045a5212701bbf6ee44eecb5f318210
parent 5642a2e0

include/media/stagefright/SkipCutBuffer.h

+4 −3
Original line number Diff line number Diff line
@@ -29,9 +29,10 @@ namespace android { 
 */

class SkipCutBuffer: public RefBase {

 public:

    // 'skip' is the number of bytes to skip from the beginning

    // 'cut' is the number of bytes to cut from the end

    SkipCutBuffer(int32_t skip, int32_t cut);

    // 'skip' is the number of frames to skip from the beginning

    // 'cut' is the number of frames to cut from the end

    // 'num16BitChannels' is the number of channels, which are assumed to be 16 bit wide each

    SkipCutBuffer(size_t skip, size_t cut, size_t num16Channels);



    // Submit one MediaBuffer for skipping and cutting. This may consume all or

    // some of the data in the buffer, or it may add data to it.

media/libstagefright/ACodec.cpp

+1 −4
Original line number Diff line number Diff line
@@ -4438,16 +4438,13 @@ void ACodec::sendFormatChange(const sp<AMessage> &reply) { 
               (mEncoderDelay || mEncoderPadding)) {

        int32_t channelCount;

        CHECK(notify->findInt32("channel-count", &channelCount));

        size_t frameSize = channelCount * sizeof(int16_t);

        if (mSkipCutBuffer != NULL) {

            size_t prevbufsize = mSkipCutBuffer->size();

            if (prevbufsize != 0) {

                ALOGW("Replacing SkipCutBuffer holding %zu bytes", prevbufsize);

            }

        }

        mSkipCutBuffer = new SkipCutBuffer(

                mEncoderDelay * frameSize,

                mEncoderPadding * frameSize);

        mSkipCutBuffer = new SkipCutBuffer(mEncoderDelay, mEncoderPadding, channelCount);

    }



    notify->post();

media/libstagefright/OMXCodec.cpp

+1 −2
Original line number Diff line number Diff line
@@ -1751,14 +1751,13 @@ status_t OMXCodec::allocateBuffersOnPort(OMX_U32 portIndex) { 
        int32_t numchannels = 0;

        if (delay + padding) {

            if (mOutputFormat->findInt32(kKeyChannelCount, &numchannels)) {

                size_t frameSize = numchannels * sizeof(int16_t);

                if (mSkipCutBuffer != NULL) {

                    size_t prevbuffersize = mSkipCutBuffer->size();

                    if (prevbuffersize != 0) {

                        ALOGW("Replacing SkipCutBuffer holding %zu bytes", prevbuffersize);

                    }

                }

                mSkipCutBuffer = new SkipCutBuffer(delay * frameSize, padding * frameSize);

                mSkipCutBuffer = new SkipCutBuffer(delay, padding, numchannels);

            }

        }

    }

media/libstagefright/SkipCutBuffer.cpp

+30 −9
Original line number Diff line number Diff line
@@ -24,21 +24,32 @@ 


namespace android {



SkipCutBuffer::SkipCutBuffer(int32_t skip, int32_t cut) {

SkipCutBuffer::SkipCutBuffer(size_t skip, size_t cut, size_t num16BitChannels) {



    if (skip < 0 || cut < 0 || cut > 64 * 1024) {

        ALOGW("out of range skip/cut: %d/%d, using passthrough instead", skip, cut);

        skip = 0;

        cut = 0;

    mWriteHead = 0;

    mReadHead = 0;

    mCapacity = 0;

    mCutBuffer = NULL;



    if (num16BitChannels == 0 || num16BitChannels > INT32_MAX / 2) {

        ALOGW("# channels out of range: %zu, using passthrough instead", num16BitChannels);

        return;

    }

    size_t frameSize = num16BitChannels * 2;

    if (skip > INT32_MAX / frameSize || cut > INT32_MAX / frameSize

            || cut * frameSize > INT32_MAX - 4096) {

        ALOGW("out of range skip/cut: %zu/%zu, using passthrough instead",

                skip, cut);

        return;

    }

    skip *= frameSize;

    cut *= frameSize;



    mFrontPadding = mSkip = skip;

    mBackPadding = cut;

    mWriteHead = 0;

    mReadHead = 0;

    mCapacity = cut + 4096;

    mCutBuffer = new char[mCapacity];

    ALOGV("skipcutbuffer %d %d %d", skip, cut, mCapacity);

    mCutBuffer = new (std::nothrow) char[mCapacity];

    ALOGV("skipcutbuffer %zu %zu %d", skip, cut, mCapacity);

}



SkipCutBuffer::~SkipCutBuffer() {
@@ -46,6 +57,11 @@ SkipCutBuffer::~SkipCutBuffer() { 
}



void SkipCutBuffer::submit(MediaBuffer *buffer) {

    if (mCutBuffer == NULL) {

        // passthrough mode

        return;

    }



    int32_t offset = buffer->range_offset();

    int32_t buflen = buffer->range_length();
@@ -73,6 +89,11 @@ void SkipCutBuffer::submit(MediaBuffer *buffer) { 
}



void SkipCutBuffer::submit(const sp<ABuffer>& buffer) {

    if (mCutBuffer == NULL) {

        // passthrough mode

        return;

    }



    int32_t offset = buffer->offset();

    int32_t buflen = buffer->size();