aaudio: call validate() from readFromParcel()
This is to ensure that validate() is always called
in the future.
Bug: 74558178
Test: run AAudio CTS
Test: on Marlin, adb shell write_sine_callback -pl
Change-Id: I661f3c4e690be2268ca61b80a98bbdf9a7368c1b
diff --git a/media/libaaudio/src/binding/AudioEndpointParcelable.cpp b/media/libaaudio/src/binding/AudioEndpointParcelable.cpp
index 9eed96d..61d7d27 100644
--- a/media/libaaudio/src/binding/AudioEndpointParcelable.cpp
+++ b/media/libaaudio/src/binding/AudioEndpointParcelable.cpp
@@ -64,27 +64,54 @@
* The read and write must be symmetric.
*/
status_t AudioEndpointParcelable::writeToParcel(Parcel* parcel) const {
- parcel->writeInt32(mNumSharedMemories);
+ status_t status = AAudioConvert_aaudioToAndroidStatus(validate());
+ if (status != NO_ERROR) goto error;
+
+ status = parcel->writeInt32(mNumSharedMemories);
+ if (status != NO_ERROR) goto error;
+
for (int i = 0; i < mNumSharedMemories; i++) {
- mSharedMemories[i].writeToParcel(parcel);
+ status = mSharedMemories[i].writeToParcel(parcel);
+ if (status != NO_ERROR) goto error;
}
- mUpMessageQueueParcelable.writeToParcel(parcel);
- mDownMessageQueueParcelable.writeToParcel(parcel);
- mUpDataQueueParcelable.writeToParcel(parcel);
- mDownDataQueueParcelable.writeToParcel(parcel);
- return NO_ERROR; // TODO check for errors above
+ status = mUpMessageQueueParcelable.writeToParcel(parcel);
+ if (status != NO_ERROR) goto error;
+ status = mDownMessageQueueParcelable.writeToParcel(parcel);
+ if (status != NO_ERROR) goto error;
+ status = mUpDataQueueParcelable.writeToParcel(parcel);
+ if (status != NO_ERROR) goto error;
+ status = mDownDataQueueParcelable.writeToParcel(parcel);
+ if (status != NO_ERROR) goto error;
+
+ return NO_ERROR;
+
+error:
+ ALOGE("%s returning %d", __func__, status);
+ return status;
}
status_t AudioEndpointParcelable::readFromParcel(const Parcel* parcel) {
- parcel->readInt32(&mNumSharedMemories);
+ status_t status = parcel->readInt32(&mNumSharedMemories);
+ if (status != NO_ERROR) goto error;
+
for (int i = 0; i < mNumSharedMemories; i++) {
mSharedMemories[i].readFromParcel(parcel);
+ if (status != NO_ERROR) goto error;
}
- mUpMessageQueueParcelable.readFromParcel(parcel);
- mDownMessageQueueParcelable.readFromParcel(parcel);
- mUpDataQueueParcelable.readFromParcel(parcel);
- mDownDataQueueParcelable.readFromParcel(parcel);
- return NO_ERROR; // TODO check for errors above
+ status = mUpMessageQueueParcelable.readFromParcel(parcel);
+ if (status != NO_ERROR) goto error;
+ status = mDownMessageQueueParcelable.readFromParcel(parcel);
+ if (status != NO_ERROR) goto error;
+ status = mUpDataQueueParcelable.readFromParcel(parcel);
+ if (status != NO_ERROR) goto error;
+ status = mDownDataQueueParcelable.readFromParcel(parcel);
+ if (status != NO_ERROR) goto error;
+
+ return AAudioConvert_aaudioToAndroidStatus(validate());
+
+error:
+ ALOGE("%s returning %d", __func__, status);
+ return status;
}
aaudio_result_t AudioEndpointParcelable::resolve(EndpointDescriptor *descriptor) {
@@ -109,35 +136,11 @@
return AAudioConvert_androidToAAudioResult(err);
}
-aaudio_result_t AudioEndpointParcelable::validate() {
- aaudio_result_t result;
+aaudio_result_t AudioEndpointParcelable::validate() const {
if (mNumSharedMemories < 0 || mNumSharedMemories >= MAX_SHARED_MEMORIES) {
ALOGE("invalid mNumSharedMemories = %d", mNumSharedMemories);
return AAUDIO_ERROR_INTERNAL;
}
- for (int i = 0; i < mNumSharedMemories; i++) {
- result = mSharedMemories[i].validate();
- if (result != AAUDIO_OK) {
- ALOGE("invalid mSharedMemories[%d] = %d", i, result);
- return result;
- }
- }
- if ((result = mUpMessageQueueParcelable.validate()) != AAUDIO_OK) {
- ALOGE("invalid mUpMessageQueueParcelable = %d", result);
- return result;
- }
- if ((result = mDownMessageQueueParcelable.validate()) != AAUDIO_OK) {
- ALOGE("invalid mDownMessageQueueParcelable = %d", result);
- return result;
- }
- if ((result = mUpDataQueueParcelable.validate()) != AAUDIO_OK) {
- ALOGE("invalid mUpDataQueueParcelable = %d", result);
- return result;
- }
- if ((result = mDownDataQueueParcelable.validate()) != AAUDIO_OK) {
- ALOGE("invalid mDownDataQueueParcelable = %d", result);
- return result;
- }
return AAUDIO_OK;
}
diff --git a/media/libaaudio/src/binding/AudioEndpointParcelable.h b/media/libaaudio/src/binding/AudioEndpointParcelable.h
index aa8573f..e4f8b9e 100644
--- a/media/libaaudio/src/binding/AudioEndpointParcelable.h
+++ b/media/libaaudio/src/binding/AudioEndpointParcelable.h
@@ -56,8 +56,6 @@
aaudio_result_t resolve(EndpointDescriptor *descriptor);
- aaudio_result_t validate();
-
aaudio_result_t close();
void dump();
@@ -70,6 +68,8 @@
RingBufferParcelable mDownDataQueueParcelable; // eg. playback
private:
+ aaudio_result_t validate() const;
+
int32_t mNumSharedMemories = 0;
SharedMemoryParcelable mSharedMemories[MAX_SHARED_MEMORIES];
};
diff --git a/media/libaaudio/src/binding/IAAudioService.cpp b/media/libaaudio/src/binding/IAAudioService.cpp
index b3c4934..620edc7 100644
--- a/media/libaaudio/src/binding/IAAudioService.cpp
+++ b/media/libaaudio/src/binding/IAAudioService.cpp
@@ -121,17 +121,11 @@
ALOGE("BpAAudioService::client GET_STREAM_DESCRIPTION passed result %d", result);
return result;
}
- err = parcelable.readFromParcel(&reply);;
+ err = parcelable.readFromParcel(&reply);
if (err != NO_ERROR) {
ALOGE("BpAAudioService::client transact(GET_STREAM_DESCRIPTION) read endpoint %d", err);
return AAudioConvert_androidToAAudioResult(err);
}
- //parcelable.dump();
- result = parcelable.validate();
- if (result != AAUDIO_OK) {
- ALOGE("BpAAudioService::client GET_STREAM_DESCRIPTION validation fails %d", result);
- return result;
- }
return result;
}
@@ -250,6 +244,7 @@
pid_t tid;
int64_t nanoseconds;
aaudio_result_t result;
+ status_t status = NO_ERROR;
ALOGV("BnAAudioService::onTransact(%i) %i", code, flags);
switch(code) {
@@ -294,21 +289,20 @@
case GET_STREAM_DESCRIPTION: {
CHECK_INTERFACE(IAAudioService, data, reply);
- data.readInt32(&streamHandle);
+ status = data.readInt32(&streamHandle);
+ if (status != NO_ERROR) {
+ return status;
+ }
aaudio::AudioEndpointParcelable parcelable;
result = getStreamDescription(streamHandle, parcelable);
if (result != AAUDIO_OK) {
return AAudioConvert_aaudioToAndroidStatus(result);
}
- result = parcelable.validate();
- if (result != AAUDIO_OK) {
- ALOGE("BnAAudioService::onTransact getStreamDescription() returns %d", result);
- parcelable.dump();
- return AAudioConvert_aaudioToAndroidStatus(result);
+ status = reply->writeInt32(result);
+ if (status != NO_ERROR) {
+ return status;
}
- reply->writeInt32(result);
- parcelable.writeToParcel(reply);
- return NO_ERROR;
+ return parcelable.writeToParcel(reply);
} break;
case START_STREAM: {
diff --git a/media/libaaudio/src/binding/RingBufferParcelable.cpp b/media/libaaudio/src/binding/RingBufferParcelable.cpp
index 2babbff..4996b3f 100644
--- a/media/libaaudio/src/binding/RingBufferParcelable.cpp
+++ b/media/libaaudio/src/binding/RingBufferParcelable.cpp
@@ -21,6 +21,7 @@
#include <stdint.h>
#include <binder/Parcelable.h>
+#include <utility/AAudioUtilities.h>
#include "binding/AAudioServiceDefinitions.h"
#include "binding/SharedRegionParcelable.h"
@@ -79,7 +80,10 @@
* The read and write must be symmetric.
*/
status_t RingBufferParcelable::writeToParcel(Parcel* parcel) const {
- status_t status = parcel->writeInt32(mCapacityInFrames);
+ status_t status = AAudioConvert_aaudioToAndroidStatus(validate());
+ if (status != NO_ERROR) goto error;
+
+ status = parcel->writeInt32(mCapacityInFrames);
if (status != NO_ERROR) goto error;
if (mCapacityInFrames > 0) {
status = parcel->writeInt32(mBytesPerFrame);
@@ -97,7 +101,7 @@
}
return NO_ERROR;
error:
- ALOGE("writeToParcel() error = %d", status);
+ ALOGE("%s returning %d", __func__, status);
return status;
}
@@ -118,9 +122,9 @@
status = mDataParcelable.readFromParcel(parcel);
if (status != NO_ERROR) goto error;
}
- return NO_ERROR;
+ return AAudioConvert_aaudioToAndroidStatus(validate());
error:
- ALOGE("readFromParcel() error = %d", status);
+ ALOGE("%s returning %d", __func__, status);
return status;
}
@@ -151,8 +155,7 @@
return AAUDIO_OK;
}
-aaudio_result_t RingBufferParcelable::validate() {
- aaudio_result_t result;
+aaudio_result_t RingBufferParcelable::validate() const {
if (mCapacityInFrames < 0 || mCapacityInFrames >= 32 * 1024) {
ALOGE("invalid mCapacityInFrames = %d", mCapacityInFrames);
return AAUDIO_ERROR_INTERNAL;
@@ -165,18 +168,6 @@
ALOGE("invalid mFramesPerBurst = %d", mFramesPerBurst);
return AAUDIO_ERROR_INTERNAL;
}
- if ((result = mReadCounterParcelable.validate()) != AAUDIO_OK) {
- ALOGE("invalid mReadCounterParcelable = %d", result);
- return result;
- }
- if ((result = mWriteCounterParcelable.validate()) != AAUDIO_OK) {
- ALOGE("invalid mWriteCounterParcelable = %d", result);
- return result;
- }
- if ((result = mDataParcelable.validate()) != AAUDIO_OK) {
- ALOGE("invalid mDataParcelable = %d", result);
- return result;
- }
return AAUDIO_OK;
}
diff --git a/media/libaaudio/src/binding/RingBufferParcelable.h b/media/libaaudio/src/binding/RingBufferParcelable.h
index bd562f2..1dbcf07 100644
--- a/media/libaaudio/src/binding/RingBufferParcelable.h
+++ b/media/libaaudio/src/binding/RingBufferParcelable.h
@@ -66,11 +66,12 @@
aaudio_result_t resolve(SharedMemoryParcelable *memoryParcels, RingBufferDescriptor *descriptor);
- aaudio_result_t validate();
-
void dump();
private:
+
+ aaudio_result_t validate() const;
+
SharedRegionParcelable mReadCounterParcelable;
SharedRegionParcelable mWriteCounterParcelable;
SharedRegionParcelable mDataParcelable;
diff --git a/media/libaaudio/src/binding/SharedMemoryParcelable.cpp b/media/libaaudio/src/binding/SharedMemoryParcelable.cpp
index 4e3e5d1..0b0cf77 100644
--- a/media/libaaudio/src/binding/SharedMemoryParcelable.cpp
+++ b/media/libaaudio/src/binding/SharedMemoryParcelable.cpp
@@ -48,7 +48,10 @@
}
status_t SharedMemoryParcelable::writeToParcel(Parcel* parcel) const {
- status_t status = parcel->writeInt32(mSizeInBytes);
+ status_t status = AAudioConvert_aaudioToAndroidStatus(validate());
+ if (status != NO_ERROR) return status;
+
+ status = parcel->writeInt32(mSizeInBytes);
if (status != NO_ERROR) return status;
if (mSizeInBytes > 0) {
ALOGV("writeToParcel() mFd = %d, this = %p\n", mFd.get(), this);
@@ -61,21 +64,27 @@
status_t SharedMemoryParcelable::readFromParcel(const Parcel* parcel) {
status_t status = parcel->readInt32(&mSizeInBytes);
- if (status != NO_ERROR) {
- return status;
- }
+ if (status != NO_ERROR) goto error;
+
if (mSizeInBytes > 0) {
// The Parcel owns the file descriptor and will close it later.
unique_fd mmapFd;
status = parcel->readUniqueFileDescriptor(&mmapFd);
if (status != NO_ERROR) {
ALOGE("readFromParcel() readUniqueFileDescriptor() failed : %d", status);
- } else {
- // Resolve the memory now while we still have the FD from the Parcel.
- // Closing the FD will not affect the shared memory once mmap() has been called.
- status = AAudioConvert_androidToAAudioResult(resolveSharedMemory(mmapFd));
+ goto error;
}
+
+ // Resolve the memory now while we still have the FD from the Parcel.
+ // Closing the FD will not affect the shared memory once mmap() has been called.
+ aaudio_result_t result = resolveSharedMemory(mmapFd);
+ status = AAudioConvert_aaudioToAndroidStatus(result);
+ if (status != NO_ERROR) goto error;
}
+
+ return AAudioConvert_aaudioToAndroidStatus(validate());
+
+error:
return status;
}
@@ -136,7 +145,7 @@
return mSizeInBytes;
}
-aaudio_result_t SharedMemoryParcelable::validate() {
+aaudio_result_t SharedMemoryParcelable::validate() const {
if (mSizeInBytes < 0 || mSizeInBytes >= MAX_MMAP_SIZE_BYTES) {
ALOGE("invalid mSizeInBytes = %d", mSizeInBytes);
return AAUDIO_ERROR_OUT_OF_RANGE;
diff --git a/media/libaaudio/src/binding/SharedMemoryParcelable.h b/media/libaaudio/src/binding/SharedMemoryParcelable.h
index 2a634e0..82c2240 100644
--- a/media/libaaudio/src/binding/SharedMemoryParcelable.h
+++ b/media/libaaudio/src/binding/SharedMemoryParcelable.h
@@ -61,8 +61,6 @@
int32_t getSizeInBytes();
- aaudio_result_t validate();
-
void dump();
protected:
@@ -74,6 +72,11 @@
android::base::unique_fd mFd;
int32_t mSizeInBytes = 0;
uint8_t *mResolvedAddress = MMAP_UNRESOLVED_ADDRESS;
+
+private:
+
+ aaudio_result_t validate() const;
+
};
} /* namespace aaudio */
diff --git a/media/libaaudio/src/binding/SharedRegionParcelable.cpp b/media/libaaudio/src/binding/SharedRegionParcelable.cpp
index 7aa80bf..c776116 100644
--- a/media/libaaudio/src/binding/SharedRegionParcelable.cpp
+++ b/media/libaaudio/src/binding/SharedRegionParcelable.cpp
@@ -24,6 +24,7 @@
#include <binder/Parcelable.h>
#include <aaudio/AAudio.h>
+#include <utility/AAudioUtilities.h>
#include "binding/SharedMemoryParcelable.h"
#include "binding/SharedRegionParcelable.h"
@@ -47,21 +48,38 @@
}
status_t SharedRegionParcelable::writeToParcel(Parcel* parcel) const {
- parcel->writeInt32(mSizeInBytes);
+ status_t status = AAudioConvert_aaudioToAndroidStatus(validate());
+ if (status != NO_ERROR) goto error;
+
+ status = parcel->writeInt32(mSizeInBytes);
+ if (status != NO_ERROR) goto error;
if (mSizeInBytes > 0) {
- parcel->writeInt32(mSharedMemoryIndex);
- parcel->writeInt32(mOffsetInBytes);
+ status = parcel->writeInt32(mSharedMemoryIndex);
+ if (status != NO_ERROR) goto error;
+ status = parcel->writeInt32(mOffsetInBytes);
+ if (status != NO_ERROR) goto error;
}
- return NO_ERROR; // TODO check for errors above
+ return NO_ERROR;
+
+error:
+ ALOGE("%s returning %d", __func__, status);
+ return status;
}
status_t SharedRegionParcelable::readFromParcel(const Parcel* parcel) {
- parcel->readInt32(&mSizeInBytes);
+ status_t status = parcel->readInt32(&mSizeInBytes);
+ if (status != NO_ERROR) goto error;
if (mSizeInBytes > 0) {
- parcel->readInt32(&mSharedMemoryIndex);
- parcel->readInt32(&mOffsetInBytes);
+ status = parcel->readInt32(&mSharedMemoryIndex);
+ if (status != NO_ERROR) goto error;
+ status = parcel->readInt32(&mOffsetInBytes);
+ if (status != NO_ERROR) goto error;
}
- return NO_ERROR; // TODO check for errors above
+ return AAudioConvert_aaudioToAndroidStatus(validate());
+
+error:
+ ALOGE("%s returning %d", __func__, status);
+ return status;
}
aaudio_result_t SharedRegionParcelable::resolve(SharedMemoryParcelable *memoryParcels,
@@ -78,7 +96,7 @@
return memoryParcel->resolve(mOffsetInBytes, mSizeInBytes, regionAddressPtr);
}
-aaudio_result_t SharedRegionParcelable::validate() {
+aaudio_result_t SharedRegionParcelable::validate() const {
if (mSizeInBytes < 0 || mSizeInBytes >= MAX_MMAP_SIZE_BYTES) {
ALOGE("invalid mSizeInBytes = %d", mSizeInBytes);
return AAUDIO_ERROR_OUT_OF_RANGE;
diff --git a/media/libaaudio/src/binding/SharedRegionParcelable.h b/media/libaaudio/src/binding/SharedRegionParcelable.h
index f6babfd..0cd8c04 100644
--- a/media/libaaudio/src/binding/SharedRegionParcelable.h
+++ b/media/libaaudio/src/binding/SharedRegionParcelable.h
@@ -47,14 +47,15 @@
bool isFileDescriptorSafe(SharedMemoryParcelable *memoryParcels);
- aaudio_result_t validate();
-
void dump();
protected:
int32_t mSharedMemoryIndex = -1;
int32_t mOffsetInBytes = 0;
int32_t mSizeInBytes = 0;
+
+private:
+ aaudio_result_t validate() const;
};
} /* namespace aaudio */