summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMathias Agopian <mathias@google.com>2010-04-30 12:59:21 -0700
committerMathias Agopian <mathias@google.com>2010-04-30 13:05:36 -0700
commit3b91e13ec7cdf7d607a221feda2eb0afc1e02050 (patch)
tree3aeeb3c837519b517f370add6b09cd60c64e9831
parent1bb8b670f99c1029def72ec408142077abd66cc4 (diff)
downloadframeworks_base-3b91e13ec7cdf7d607a221feda2eb0afc1e02050.zip
frameworks_base-3b91e13ec7cdf7d607a221feda2eb0afc1e02050.tar.gz
frameworks_base-3b91e13ec7cdf7d607a221feda2eb0afc1e02050.tar.bz2
make sure the server-side validates pointers/indices visible on the client side
Change-Id: I604f58d3fcd2d09ec7998123c627401081345cd6
-rw-r--r--libs/surfaceflinger_client/SharedBufferStack.cpp16
1 files changed, 15 insertions, 1 deletions
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::LockCondition::LockCondition(
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 @@ SharedBufferServer::ReallocateCondition::ReallocateCondition(
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 @@ SharedBufferServer::RetireUpdate::RetireUpdate(
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 @@ ssize_t SharedBufferServer::retireAndLock()
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",