From 6179f6448ab910b1d2d7ccfa91152a792efc740a Mon Sep 17 00:00:00 2001 From: Chad Brubaker Date: Wed, 23 Sep 2015 15:17:29 -0700 Subject: Fix benign overflow in AudioTrack two uint32_t's were being used in a computation that could be negative, cast to int32_t before the subtraction to prevent incorrect overflow detection. Change-Id: I33c5ef79a0ebbba055daa0ea041b42229d0c3152 --- media/libmedia/AudioTrack.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'media/libmedia') diff --git a/media/libmedia/AudioTrack.cpp b/media/libmedia/AudioTrack.cpp index b969449..39536a2 100644 --- a/media/libmedia/AudioTrack.cpp +++ b/media/libmedia/AudioTrack.cpp @@ -2197,8 +2197,7 @@ uint32_t AudioTrack::updateAndGetPosition_l() { // This is the sole place to read server consumed frames uint32_t newServer = mProxy->getPosition(); - int32_t delta = newServer - mServer; - mServer = newServer; + uint32_t delta = newServer > mServer ? newServer - mServer : 0; // TODO There is controversy about whether there can be "negative jitter" in server position. // This should be investigated further, and if possible, it should be addressed. // A more definite failure mode is infrequent polling by client. @@ -2207,11 +2206,12 @@ uint32_t AudioTrack::updateAndGetPosition_l() // That should ensure delta never goes negative for infrequent polling // unless the server has more than 2^31 frames in its buffer, // in which case the use of uint32_t for these counters has bigger issues. - if (delta < 0) { - ALOGE("detected illegal retrograde motion by the server: mServer advanced by %d", delta); - delta = 0; + if (newServer < mServer) { + ALOGE("detected illegal retrograde motion by the server: mServer advanced by %d", + (int32_t) newServer - mServer); } - return mPosition += (uint32_t) delta; + mServer = newServer; + return mPosition += delta; } bool AudioTrack::isSampleRateSpeedAllowed_l(uint32_t sampleRate, float speed) const -- cgit v1.1