Merge changes from topic 'svcmgr_shutdown_critical' into oc-dr1-dev
* changes:
servicemanager: Mark servicemanager as shutdown critical
servicemanager: Mark vndservicemanager as shutdown critical
diff --git a/libs/vr/libbufferhub/bufferhub_tests.cpp b/libs/vr/libbufferhub/bufferhub_tests.cpp
index fa61c4a..1daa5d6 100644
--- a/libs/vr/libbufferhub/bufferhub_tests.cpp
+++ b/libs/vr/libbufferhub/bufferhub_tests.cpp
@@ -62,6 +62,57 @@
EXPECT_GE(0, RETRY_EINTR(p->Poll(0)));
}
+TEST_F(LibBufferHubTest, TestStateTransitions) {
+ std::unique_ptr<BufferProducer> p = BufferProducer::Create(
+ kWidth, kHeight, kFormat, kUsage, sizeof(uint64_t));
+ ASSERT_TRUE(p.get() != nullptr);
+ std::unique_ptr<BufferConsumer> c =
+ BufferConsumer::Import(p->CreateConsumer());
+ ASSERT_TRUE(c.get() != nullptr);
+
+ uint64_t context;
+ LocalHandle fence;
+
+ // The producer buffer starts in gained state.
+
+ // Acquire, release, and gain in gained state should fail.
+ EXPECT_EQ(-EBUSY, c->Acquire(&fence, &context));
+ EXPECT_EQ(-EBUSY, c->Release(LocalHandle()));
+ EXPECT_EQ(-EALREADY, p->Gain(&fence));
+
+ // Post in gained state should succeed.
+ EXPECT_EQ(0, p->Post(LocalHandle(), kContext));
+
+ // Post, release, and gain in posted state should fail.
+ EXPECT_EQ(-EBUSY, p->Post(LocalHandle(), kContext));
+ EXPECT_EQ(-EBUSY, c->Release(LocalHandle()));
+ EXPECT_EQ(-EBUSY, p->Gain(&fence));
+
+ // Acquire in posted state should succeed.
+ EXPECT_LE(0, c->Acquire(&fence, &context));
+
+ // Acquire, post, and gain in acquired state should fail.
+ EXPECT_EQ(-EBUSY, c->Acquire(&fence, &context));
+ EXPECT_EQ(-EBUSY, p->Post(LocalHandle(), kContext));
+ EXPECT_EQ(-EBUSY, p->Gain(&fence));
+
+ // Release in acquired state should succeed.
+ EXPECT_EQ(0, c->Release(LocalHandle()));
+
+ // Release, acquire, and post in released state should fail.
+ EXPECT_EQ(-EBUSY, c->Release(LocalHandle()));
+ EXPECT_EQ(-EBUSY, c->Acquire(&fence, &context));
+ EXPECT_EQ(-EBUSY, p->Post(LocalHandle(), kContext));
+
+ // Gain in released state should succeed.
+ EXPECT_EQ(0, p->Gain(&fence));
+
+ // Acquire, release, and gain in gained state should fail.
+ EXPECT_EQ(-EBUSY, c->Acquire(&fence, &context));
+ EXPECT_EQ(-EBUSY, c->Release(LocalHandle()));
+ EXPECT_EQ(-EALREADY, p->Gain(&fence));
+}
+
TEST_F(LibBufferHubTest, TestWithCustomMetadata) {
struct Metadata {
int64_t field1;
diff --git a/libs/vr/libvrflinger/display_surface.cpp b/libs/vr/libvrflinger/display_surface.cpp
index 6e18781..04e3d5f 100644
--- a/libs/vr/libvrflinger/display_surface.cpp
+++ b/libs/vr/libvrflinger/display_surface.cpp
@@ -248,11 +248,12 @@
"ApplicationDisplaySurface::OnQueueEvent: queue_id=%d events=%x",
consumer_queue->id(), events);
+ std::lock_guard<std::mutex> autolock(lock_);
+
// Always give the queue a chance to handle its internal bookkeeping.
consumer_queue->HandleQueueEvents();
// Check for hangup and remove a queue that is no longer needed.
- std::lock_guard<std::mutex> autolock(lock_);
if (consumer_queue->hung_up()) {
ALOGD_IF(TRACE, "ApplicationDisplaySurface::OnQueueEvent: Removing queue.");
UnregisterQueue(consumer_queue);
@@ -317,11 +318,12 @@
ALOGD_IF(TRACE, "DirectDisplaySurface::OnQueueEvent: queue_id=%d events=%x",
consumer_queue->id(), events);
+ std::lock_guard<std::mutex> autolock(lock_);
+
// Always give the queue a chance to handle its internal bookkeeping.
consumer_queue->HandleQueueEvents();
// Check for hangup and remove a queue that is no longer needed.
- std::lock_guard<std::mutex> autolock(lock_);
if (consumer_queue->hung_up()) {
ALOGD_IF(TRACE, "DirectDisplaySurface::OnQueueEvent: Removing queue.");
UnregisterQueue(consumer_queue);
diff --git a/services/vr/bufferhubd/consumer_channel.cpp b/services/vr/bufferhubd/consumer_channel.cpp
index 08b2790..ac6896a 100644
--- a/services/vr/bufferhubd/consumer_channel.cpp
+++ b/services/vr/bufferhubd/consumer_channel.cpp
@@ -8,9 +8,9 @@
#include <private/dvr/bufferhub_rpc.h>
#include "producer_channel.h"
-using android::pdx::ErrorStatus;
using android::pdx::BorrowedHandle;
using android::pdx::Channel;
+using android::pdx::ErrorStatus;
using android::pdx::Message;
using android::pdx::Status;
using android::pdx::rpc::DispatchRemoteMethod;
@@ -22,8 +22,6 @@
int channel_id,
const std::shared_ptr<Channel> producer)
: BufferHubChannel(service, buffer_id, channel_id, kConsumerType),
- handled_(true),
- ignored_(false),
producer_(producer) {
GetProducer()->AddConsumer(this);
}
@@ -34,7 +32,7 @@
channel_id(), buffer_id());
if (auto producer = GetProducer()) {
- if (!handled_) // Producer is waiting for our Release.
+ if (!released_) // Producer is waiting for our Release.
producer->OnConsumerIgnored();
producer->RemoveConsumer(this);
}
@@ -108,15 +106,20 @@
if (!producer)
return ErrorStatus(EPIPE);
- if (ignored_ || handled_) {
+ if (acquired_ || released_) {
ALOGE(
"ConsumerChannel::OnConsumerAcquire: Acquire when not posted: "
- "ignored=%d handled=%d channel_id=%d buffer_id=%d",
- ignored_, handled_, message.GetChannelId(), producer->buffer_id());
+ "ignored=%d acquired=%d released=%d channel_id=%d buffer_id=%d",
+ ignored_, acquired_, released_, message.GetChannelId(),
+ producer->buffer_id());
return ErrorStatus(EBUSY);
} else {
- ClearAvailable();
- return producer->OnConsumerAcquire(message, metadata_size);
+ auto status = producer->OnConsumerAcquire(message, metadata_size);
+ if (status) {
+ ClearAvailable();
+ acquired_ = true;
+ }
+ return status;
}
}
@@ -127,17 +130,21 @@
if (!producer)
return ErrorStatus(EPIPE);
- if (ignored_ || handled_) {
+ if (!acquired_ || released_) {
ALOGE(
"ConsumerChannel::OnConsumerRelease: Release when not acquired: "
- "ignored=%d handled=%d channel_id=%d buffer_id=%d",
- ignored_, handled_, message.GetChannelId(), producer->buffer_id());
+ "ignored=%d acquired=%d released=%d channel_id=%d buffer_id=%d",
+ ignored_, acquired_, released_, message.GetChannelId(),
+ producer->buffer_id());
return ErrorStatus(EBUSY);
} else {
- ClearAvailable();
auto status =
producer->OnConsumerRelease(message, std::move(release_fence));
- handled_ = !!status;
+ if (status) {
+ ClearAvailable();
+ acquired_ = false;
+ released_ = true;
+ }
return status;
}
}
@@ -149,12 +156,13 @@
return ErrorStatus(EPIPE);
ignored_ = ignored;
- if (ignored_ && !handled_) {
+ if (ignored_ && acquired_) {
// Update the producer if ignore is set after the consumer acquires the
// buffer.
ClearAvailable();
producer->OnConsumerIgnored();
- handled_ = false;
+ acquired_ = false;
+ released_ = true;
}
return {};
@@ -162,10 +170,12 @@
bool ConsumerChannel::OnProducerPosted() {
if (ignored_) {
- handled_ = true;
+ acquired_ = false;
+ released_ = true;
return false;
} else {
- handled_ = false;
+ acquired_ = false;
+ released_ = false;
SignalAvailable();
return true;
}
diff --git a/services/vr/bufferhubd/consumer_channel.h b/services/vr/bufferhubd/consumer_channel.h
index d84055c..208a002 100644
--- a/services/vr/bufferhubd/consumer_channel.h
+++ b/services/vr/bufferhubd/consumer_channel.h
@@ -38,8 +38,9 @@
LocalFence release_fence);
pdx::Status<void> OnConsumerSetIgnore(Message& message, bool ignore);
- bool handled_; // True if we have processed RELEASE.
- bool ignored_; // True if we are ignoring events.
+ bool acquired_{false};
+ bool released_{true};
+ bool ignored_{false}; // True if we are ignoring events.
std::weak_ptr<Channel> producer_;
ConsumerChannel(const ConsumerChannel&) = delete;
diff --git a/services/vr/bufferhubd/producer_channel.cpp b/services/vr/bufferhubd/producer_channel.cpp
index b9984a0..b2db795 100644
--- a/services/vr/bufferhubd/producer_channel.cpp
+++ b/services/vr/bufferhubd/producer_channel.cpp
@@ -122,7 +122,7 @@
}
Status<NativeBufferHandle<BorrowedHandle>> ProducerChannel::OnGetBuffer(
- Message& message) {
+ Message& /*message*/) {
ATRACE_NAME("ProducerChannel::OnGetBuffer");
ALOGD_IF(TRACE, "ProducerChannel::OnGetBuffer: buffer=%d", buffer_id());
return {NativeBufferHandle<BorrowedHandle>(buffer_, buffer_id())};
@@ -204,7 +204,7 @@
return {};
}
-Status<LocalFence> ProducerChannel::OnProducerGain(Message& message) {
+Status<LocalFence> ProducerChannel::OnProducerGain(Message& /*message*/) {
ATRACE_NAME("ProducerChannel::OnGain");
ALOGD_IF(TRACE, "ProducerChannel::OnGain: buffer_id=%d", buffer_id());
if (producer_owns_) {
@@ -224,7 +224,7 @@
}
Status<std::pair<BorrowedFence, BufferWrapper<std::uint8_t*>>>
-ProducerChannel::OnConsumerAcquire(Message& message,
+ProducerChannel::OnConsumerAcquire(Message& /*message*/,
std::size_t metadata_size) {
ATRACE_NAME("ProducerChannel::OnConsumerAcquire");
ALOGD_IF(TRACE, "ProducerChannel::OnConsumerAcquire: buffer_id=%d",
diff --git a/services/vr/performanced/performance_service.cpp b/services/vr/performanced/performance_service.cpp
index 3f7009a..4b9fbe0 100644
--- a/services/vr/performanced/performance_service.cpp
+++ b/services/vr/performanced/performance_service.cpp
@@ -63,7 +63,7 @@
// Returns true if the sender's euid is trusted according to VR manager service.
struct Trusted {
static bool Check(const Message& sender, const Task&) {
- return IsTrustedUid(sender.GetEffectiveUserId(), false);
+ return IsTrustedUid(sender.GetEffectiveUserId());
}
};
diff --git a/services/vr/performanced/performance_service.h b/services/vr/performanced/performance_service.h
index b812535..b28d94a 100644
--- a/services/vr/performanced/performance_service.h
+++ b/services/vr/performanced/performance_service.h
@@ -53,10 +53,13 @@
permission_check;
// Check the permisison of the given task to use this scheduler class. If a
- // permission check function is not set then all tasks are allowed.
- bool IsAllowed(const pdx::Message& message, const Task& task) const {
+ // permission check function is not set then operations are only allowed on
+ // tasks in the sender's process.
+ bool IsAllowed(const pdx::Message& sender, const Task& task) const {
if (permission_check)
- return permission_check(message, task);
+ return permission_check(sender, task);
+ else if (!task || task.thread_group_id() != sender.GetProcessId())
+ return false;
else
return true;
}
diff --git a/services/vr/performanced/performance_service_tests.cpp b/services/vr/performanced/performance_service_tests.cpp
index 7de1f08..274a1b3 100644
--- a/services/vr/performanced/performance_service_tests.cpp
+++ b/services/vr/performanced/performance_service_tests.cpp
@@ -183,6 +183,17 @@
ASSERT_EQ(AID_ROOT, original_uid)
<< "This test must run as root to function correctly!";
+ // Test unprivileged policies on a task that does not belong to this process.
+ // Use the init process (task_id=1) as the target.
+ error = dvrSetSchedulerPolicy(1, "batch");
+ EXPECT_EQ(-EINVAL, error);
+ error = dvrSetSchedulerPolicy(1, "background");
+ EXPECT_EQ(-EINVAL, error);
+ error = dvrSetSchedulerPolicy(1, "foreground");
+ EXPECT_EQ(-EINVAL, error);
+ error = dvrSetSchedulerPolicy(1, "normal");
+ EXPECT_EQ(-EINVAL, error);
+
// Switch the uid/gid to an id that should not have permission to access any
// privileged actions.
ASSERT_EQ(0, setresgid(AID_NOBODY, AID_NOBODY, -1))