Fix GraphicsBufferProducer recreation

We need to use a shared_ptr to ensure that the present callback doesn't do any UAFs, also unlocks the GBP during presentation as if the queue is full a deadlock could a rise where the present callback wouldn't be able to run due to the (waiting) DequeueBuffer thread holding the lock.
This commit is contained in:
Billy Laws 2022-10-30 16:30:04 +00:00
parent 29e89a3950
commit 001064b7bf
4 changed files with 21 additions and 15 deletions

View File

@ -264,7 +264,7 @@ namespace skyline::service::hosbinder {
return AndroidStatus::BadValue; return AndroidStatus::BadValue;
} }
std::scoped_lock lock(mutex); std::unique_lock lock(mutex);
if (slot < 0 || slot >= queue.size()) [[unlikely]] { if (slot < 0 || slot >= queue.size()) [[unlikely]] {
Logger::Warn("#{} was out of range", slot); Logger::Warn("#{} was out of range", slot);
return AndroidStatus::BadValue; return AndroidStatus::BadValue;
@ -383,15 +383,6 @@ namespace skyline::service::hosbinder {
throw exception("Application attempting to perform unknown sticky transformation: {:#b}", static_cast<u32>(stickyTransform)); throw exception("Application attempting to perform unknown sticky transformation: {:#b}", static_cast<u32>(stickyTransform));
} }
state.gpu->presentation.Present(buffer.texture, isAutoTimestamp ? 0 : timestamp, swapInterval, crop, scalingMode, transform, fence, [this, &buffer] {
std::unique_lock lock{mutex};
buffer.state = BufferState::Free;
bufferEvent->Signal();
freeCondition.notify_all();
});
buffer.state = BufferState::Queued; buffer.state = BufferState::Queued;
buffer.frameNumber = ++frameNumber; buffer.frameNumber = ++frameNumber;
@ -401,6 +392,20 @@ namespace skyline::service::hosbinder {
pendingBufferCount = GetPendingBufferCount(); pendingBufferCount = GetPendingBufferCount();
Logger::Debug("#{} - {}Timestamp: {}, Crop: ({}-{})x({}-{}), Scale Mode: {}, Transform: {} [Sticky: {}], Swap Interval: {}, Is Async: {}", slot, isAutoTimestamp ? "Auto " : "", timestamp, crop.left, crop.right, crop.top, crop.bottom, ToString(scalingMode), ToString(transform), ToString(stickyTransform), swapInterval, async); Logger::Debug("#{} - {}Timestamp: {}, Crop: ({}-{})x({}-{}), Scale Mode: {}, Transform: {} [Sticky: {}], Swap Interval: {}, Is Async: {}", slot, isAutoTimestamp ? "Auto " : "", timestamp, crop.left, crop.right, crop.top, crop.bottom, ToString(scalingMode), ToString(transform), ToString(stickyTransform), swapInterval, async);
// We can present with the mutex locked as if the queue ends up waiting for free space then the lock will never be released as the dequeue callback also locks the lock
lock.unlock();
std::weak_ptr<GraphicBufferProducer> weakThis{shared_from_this()};
state.gpu->presentation.Present(buffer.texture, isAutoTimestamp ? 0 : timestamp, swapInterval, crop, scalingMode, transform, fence, [weakThis, &buffer] {
if (auto gbp{weakThis.lock()}) {
std::scoped_lock lock{gbp->mutex};
buffer.state = BufferState::Free;
gbp->bufferEvent->Signal();
gbp->freeCondition.notify_all();
}
});
return AndroidStatus::Ok; return AndroidStatus::Ok;
} }

View File

@ -57,7 +57,7 @@ namespace skyline::service::hosbinder {
* @url https://cs.android.com/android/platform/superproject/+/android-5.1.1_r38:frameworks/native/include/gui/BufferQueueCore.h * @url https://cs.android.com/android/platform/superproject/+/android-5.1.1_r38:frameworks/native/include/gui/BufferQueueCore.h
* @url https://cs.android.com/android/platform/superproject/+/android-5.1.1_r38:frameworks/native/libs/gui/BufferQueueCore.cpp * @url https://cs.android.com/android/platform/superproject/+/android-5.1.1_r38:frameworks/native/libs/gui/BufferQueueCore.cpp
*/ */
class GraphicBufferProducer { class GraphicBufferProducer : public std::enable_shared_from_this<GraphicBufferProducer> {
private: private:
const DeviceState &state; const DeviceState &state;
std::mutex mutex; //!< Synchronizes access to the buffer queue std::mutex mutex; //!< Synchronizes access to the buffer queue

View File

@ -117,12 +117,13 @@ namespace skyline::service::hosbinder {
u64 IHOSBinderDriver::CreateLayer(DisplayId pDisplayId) { u64 IHOSBinderDriver::CreateLayer(DisplayId pDisplayId) {
if (pDisplayId != displayId) if (pDisplayId != displayId)
throw exception("Creating layer on unopened display: '{}'", ToString(pDisplayId)); throw exception("Creating layer on unopened display: '{}'", ToString(pDisplayId));
else if (layer) else if (layer) // Fallback to replacing previous layer
throw exception("Creation of multiple layers is not supported"); Logger::Warn("Creation of multiple layers is not supported");
layerStrongReferenceCount = InitialStrongReferenceCount; layerStrongReferenceCount = InitialStrongReferenceCount;
layerWeakReferenceCount = 0; layerWeakReferenceCount = 0;
layer.emplace(state, nvMap); layer = std::make_shared<GraphicBufferProducer>(state, nvMap);
return DefaultLayerId; return DefaultLayerId;
} }

View File

@ -40,7 +40,7 @@ namespace skyline::service::hosbinder {
constexpr static u64 DefaultLayerId{1}; //!< The VI ID of the default (and only) layer in our surface stack constexpr static u64 DefaultLayerId{1}; //!< The VI ID of the default (and only) layer in our surface stack
constexpr static u32 DefaultBinderLayerHandle{1}; //!< The handle as assigned by SurfaceFlinger of the default layer constexpr static u32 DefaultBinderLayerHandle{1}; //!< The handle as assigned by SurfaceFlinger of the default layer
std::optional<GraphicBufferProducer> layer; //!< The IGraphicBufferProducer backing the layer (NativeWindow) std::shared_ptr<GraphicBufferProducer> layer{}; //!< The IGraphicBufferProducer backing the layer (NativeWindow)
nvdrv::core::NvMap &nvMap; nvdrv::core::NvMap &nvMap;