graphics: fix use-after-free in mapper 2.0 passthrough

The mapper 2.0 gralloc passthrough keeps a set of the native_handle_t
pointers of all imported buffers. This change ensures that the
underlying HAL wrapper freeing the native_handle_t is performed
atomically with respect to accesses to the passthrough's native_handle_t
pointer set.

This fixes a race where a native_handle_t could be reallocated between
being freed by the HAL wrapper and removed from the passthrough's set,
which could then cause the import which happened to reallocate that
native_handle_t to spuriously fail.

Bug: 170798776
Test: Manually very no more spurious import failures in ARCVM's decoder.
Change-Id: I654a44e94adb319f54fb462f4484db414ca1b154
This commit is contained in:
David Stevens
2020-10-15 10:35:46 +09:00
parent cdf176e17b
commit be8f52e8b0
3 changed files with 16 additions and 19 deletions

View File

@@ -85,11 +85,7 @@ class MapperImpl : public Interface {
return Error::BAD_BUFFER;
}
Error error = mHal->freeBuffer(bufferHandle);
if (error == Error::NONE) {
removeImportedBuffer(buffer);
}
return error;
return freeImportedBuffer(bufferHandle);
}
Return<void> lock(void* buffer, uint64_t cpuUsage, const V2_0::IMapper::Rect& accessRegion,
@@ -160,8 +156,8 @@ class MapperImpl : public Interface {
return static_cast<void*>(bufferHandle);
}
virtual native_handle_t* removeImportedBuffer(void* buffer) {
return static_cast<native_handle_t*>(buffer);
virtual Error freeImportedBuffer(native_handle_t* bufferHandle) {
return mHal->freeBuffer(bufferHandle);
}
virtual native_handle_t* getImportedBuffer(void* buffer) const {

View File

@@ -56,17 +56,14 @@ class GrallocImportedBufferPool {
return *singleton;
}
std::mutex* getMutex() { return &mMutex; }
void* add(native_handle_t* bufferHandle) {
std::lock_guard<std::mutex> lock(mMutex);
return mBufferHandles.insert(bufferHandle).second ? bufferHandle : nullptr;
}
native_handle_t* remove(void* buffer) {
auto bufferHandle = static_cast<native_handle_t*>(buffer);
std::lock_guard<std::mutex> lock(mMutex);
return mBufferHandles.erase(bufferHandle) == 1 ? bufferHandle : nullptr;
}
void removeLocked(native_handle* bufferHandle) { mBufferHandles.erase(bufferHandle); }
native_handle_t* get(void* buffer) {
auto bufferHandle = static_cast<native_handle_t*>(buffer);
@@ -95,8 +92,13 @@ class GrallocMapper : public T {
return GrallocImportedBufferPool::getInstance().add(bufferHandle);
}
native_handle_t* removeImportedBuffer(void* buffer) override {
return GrallocImportedBufferPool::getInstance().remove(buffer);
Error freeImportedBuffer(native_handle_t* bufferHandle) override {
std::lock_guard<std::mutex> lock(*GrallocImportedBufferPool::getInstance().getMutex());
Error error = this->mHal->freeBuffer(bufferHandle);
if (error == Error::NONE) {
GrallocImportedBufferPool::getInstance().removeLocked(bufferHandle);
}
return error;
}
native_handle_t* getImportedBuffer(void* buffer) const override {

View File

@@ -46,7 +46,7 @@ class MapperImpl : public V2_0::hal::detail::MapperImpl<Interface, Hal> {
return Error::BAD_BUFFER;
}
return mHal->validateBufferSize(bufferHandle, descriptorInfo, stride);
return this->mHal->validateBufferSize(bufferHandle, descriptorInfo, stride);
}
Return<void> getTransportSize(void* buffer, IMapper::getTransportSize_cb hidl_cb) {
@@ -58,7 +58,7 @@ class MapperImpl : public V2_0::hal::detail::MapperImpl<Interface, Hal> {
uint32_t numFds = 0;
uint32_t numInts = 0;
Error error = mHal->getTransportSize(bufferHandle, &numFds, &numInts);
Error error = this->mHal->getTransportSize(bufferHandle, &numFds, &numInts);
hidl_cb(error, numFds, numInts);
return Void();
}
@@ -66,7 +66,7 @@ class MapperImpl : public V2_0::hal::detail::MapperImpl<Interface, Hal> {
Return<void> createDescriptor_2_1(const IMapper::BufferDescriptorInfo& descriptorInfo,
IMapper::createDescriptor_2_1_cb hidl_cb) override {
BufferDescriptor descriptor;
Error error = mHal->createDescriptor_2_1(descriptorInfo, &descriptor);
Error error = this->mHal->createDescriptor_2_1(descriptorInfo, &descriptor);
hidl_cb(error, descriptor);
return Void();
}
@@ -74,7 +74,6 @@ class MapperImpl : public V2_0::hal::detail::MapperImpl<Interface, Hal> {
private:
using BaseType2_0 = V2_0::hal::detail::MapperImpl<Interface, Hal>;
using BaseType2_0::getImportedBuffer;
using BaseType2_0::mHal;
};
} // namespace detail