make sure the server-side validates pointers/indices visible on the client side
Change-Id: I604f58d3fcd2d09ec7998123c627401081345cd6
diff --git a/libs/surfaceflinger_client/SharedBufferStack.cpp b/libs/surfaceflinger_client/SharedBufferStack.cpp
index cea8bdc..c42cd53 100644
--- a/libs/surfaceflinger_client/SharedBufferStack.cpp
+++ b/libs/surfaceflinger_client/SharedBufferStack.cpp
@@ -246,6 +246,9 @@
SharedBufferClient* sbc, int buf) : ConditionBase(sbc), buf(buf) {
}
bool SharedBufferClient::LockCondition::operator()() const {
+ // NOTE: if stack.head is messed up, we could crash the client
+ // or cause some drawing artifacts. This is okay, as long as it is
+ // limited to the client.
return (buf != stack.index[stack.head] ||
(stack.queued > 0 && stack.inUse != buf));
}
@@ -254,8 +257,15 @@
SharedBufferBase* sbb, int buf) : ConditionBase(sbb), buf(buf) {
}
bool SharedBufferServer::ReallocateCondition::operator()() const {
+ int32_t head = stack.head;
+ if (uint32_t(head) >= NUM_BUFFER_MAX) {
+ // if stack.head is messed up, we cannot allow the server to
+ // crash (since stack.head is mapped on the client side)
+ stack.status = BAD_VALUE;
+ return false;
+ }
// TODO: we should also check that buf has been dequeued
- return (buf != stack.index[stack.head]);
+ return (buf != stack.index[head]);
}
// ----------------------------------------------------------------------------
@@ -297,6 +307,8 @@
ssize_t SharedBufferServer::RetireUpdate::operator()() {
// head is only written in this function, which is single-thread.
int32_t head = stack.head;
+ if (uint32_t(head) >= NUM_BUFFER_MAX)
+ return BAD_VALUE;
// Preventively lock the current buffer before updating queued.
android_atomic_write(stack.index[head], &stack.inUse);
@@ -460,6 +472,8 @@
RetireUpdate update(this, mNumBuffers);
ssize_t buf = updateCondition( update );
if (buf >= 0) {
+ if (uint32_t(buf) >= NUM_BUFFER_MAX)
+ return BAD_VALUE;
SharedBufferStack& stack( *mSharedStack );
buf = stack.index[buf];
LOGD_IF(DEBUG_ATOMICS && buf>=0, "retire=%d, %s",
