Skip to content

Commit 5c392a9

Browse files
Treehugger RobotGerrit Code Review
Treehugger Robot
authored and
Gerrit Code Review
committed
Merge "Support IAccessor in libbinder for RPC services" into main
2 parents 454e2fe + 8578f13 commit 5c392a9

13 files changed

Lines changed: 277 additions & 49 deletions

‎cmds/servicemanager/ServiceManager.cpp‎

Lines changed: 80 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,25 @@ static std::vector<std::string> getVintfUpdatableNames(const std::string& apexNa
249249
return names;
250250
}
251251

252+
static std::optional<std::string> getVintfAccessorName(const std::string& name) {
253+
AidlName aname;
254+
if (!AidlName::fill(name, &aname)) return std::nullopt;
255+
256+
std::optional<std::string> accessor;
257+
forEachManifest([&](const ManifestWithDescription& mwd) {
258+
mwd.manifest->forEachInstance([&](const auto& manifestInstance) {
259+
if (manifestInstance.format() != vintf::HalFormat::AIDL) return true;
260+
if (manifestInstance.package() != aname.package) return true;
261+
if (manifestInstance.interface() != aname.iface) return true;
262+
if (manifestInstance.instance() != aname.instance) return true;
263+
accessor = manifestInstance.accessor();
264+
return false; // break (libvintf uses opposite convention)
265+
});
266+
return false; // continue
267+
});
268+
return accessor;
269+
}
270+
252271
static std::optional<ConnectionInfo> getVintfConnectionInfo(const std::string& name) {
253272
AidlName aname;
254273
if (!AidlName::fill(name, &aname)) return std::nullopt;
@@ -364,23 +383,40 @@ ServiceManager::~ServiceManager() {
364383
}
365384
}
366385

367-
Status ServiceManager::getService(const std::string& name, sp<IBinder>* outBinder) {
386+
Status ServiceManager::getService(const std::string& name, os::Service* outService) {
368387
SM_PERFETTO_TRACE_FUNC(PERFETTO_TE_ARG_STRING("name", name.c_str()));
369388

370-
*outBinder = tryGetService(name, true);
389+
*outService = tryGetService(name, true);
371390
// returns ok regardless of result for legacy reasons
372391
return Status::ok();
373392
}
374393

375-
Status ServiceManager::checkService(const std::string& name, sp<IBinder>* outBinder) {
394+
Status ServiceManager::checkService(const std::string& name, os::Service* outService) {
376395
SM_PERFETTO_TRACE_FUNC(PERFETTO_TE_ARG_STRING("name", name.c_str()));
377396

378-
*outBinder = tryGetService(name, false);
397+
*outService = tryGetService(name, false);
379398
// returns ok regardless of result for legacy reasons
380399
return Status::ok();
381400
}
382401

383-
sp<IBinder> ServiceManager::tryGetService(const std::string& name, bool startIfNotFound) {
402+
os::Service ServiceManager::tryGetService(const std::string& name, bool startIfNotFound) {
403+
std::optional<std::string> accessorName;
404+
#ifndef VENDORSERVICEMANAGER
405+
accessorName = getVintfAccessorName(name);
406+
#endif
407+
if (accessorName.has_value()) {
408+
auto ctx = mAccess->getCallingContext();
409+
if (!mAccess->canFind(ctx, name)) {
410+
return os::Service::make<os::Service::Tag::accessor>(nullptr);
411+
}
412+
return os::Service::make<os::Service::Tag::accessor>(
413+
tryGetBinder(*accessorName, startIfNotFound));
414+
} else {
415+
return os::Service::make<os::Service::Tag::binder>(tryGetBinder(name, startIfNotFound));
416+
}
417+
}
418+
419+
sp<IBinder> ServiceManager::tryGetBinder(const std::string& name, bool startIfNotFound) {
384420
SM_PERFETTO_TRACE_FUNC(PERFETTO_TE_ARG_STRING("name", name.c_str()));
385421

386422
auto ctx = mAccess->getCallingContext();
@@ -565,8 +601,11 @@ Status ServiceManager::registerForNotifications(
565601

566602
auto ctx = mAccess->getCallingContext();
567603

568-
if (!mAccess->canFind(ctx, name)) {
569-
return Status::fromExceptionCode(Status::EX_SECURITY, "SELinux");
604+
// TODO(b/338541373): Implement the notification mechanism for services accessed via
605+
// IAccessor.
606+
std::optional<std::string> accessorName;
607+
if (auto status = canFindService(ctx, name, &accessorName); !status.isOk()) {
608+
return status;
570609
}
571610

572611
// note - we could allow isolated apps to get notifications if we
@@ -613,8 +652,9 @@ Status ServiceManager::unregisterForNotifications(
613652

614653
auto ctx = mAccess->getCallingContext();
615654

616-
if (!mAccess->canFind(ctx, name)) {
617-
return Status::fromExceptionCode(Status::EX_SECURITY, "SELinux denied.");
655+
std::optional<std::string> accessorName;
656+
if (auto status = canFindService(ctx, name, &accessorName); !status.isOk()) {
657+
return status;
618658
}
619659

620660
bool found = false;
@@ -638,8 +678,9 @@ Status ServiceManager::isDeclared(const std::string& name, bool* outReturn) {
638678

639679
auto ctx = mAccess->getCallingContext();
640680

641-
if (!mAccess->canFind(ctx, name)) {
642-
return Status::fromExceptionCode(Status::EX_SECURITY, "SELinux denied.");
681+
std::optional<std::string> accessorName;
682+
if (auto status = canFindService(ctx, name, &accessorName); !status.isOk()) {
683+
return status;
643684
}
644685

645686
*outReturn = false;
@@ -662,8 +703,10 @@ binder::Status ServiceManager::getDeclaredInstances(const std::string& interface
662703

663704
outReturn->clear();
664705

706+
std::optional<std::string> _accessorName;
665707
for (const std::string& instance : allInstances) {
666-
if (mAccess->canFind(ctx, interface + "/" + instance)) {
708+
if (auto status = canFindService(ctx, interface + "/" + instance, &_accessorName);
709+
status.isOk()) {
667710
outReturn->push_back(instance);
668711
}
669712
}
@@ -681,8 +724,9 @@ Status ServiceManager::updatableViaApex(const std::string& name,
681724

682725
auto ctx = mAccess->getCallingContext();
683726

684-
if (!mAccess->canFind(ctx, name)) {
685-
return Status::fromExceptionCode(Status::EX_SECURITY, "SELinux denied.");
727+
std::optional<std::string> _accessorName;
728+
if (auto status = canFindService(ctx, name, &_accessorName); !status.isOk()) {
729+
return status;
686730
}
687731

688732
*outReturn = std::nullopt;
@@ -706,8 +750,9 @@ Status ServiceManager::getUpdatableNames([[maybe_unused]] const std::string& ape
706750

707751
outReturn->clear();
708752

753+
std::optional<std::string> _accessorName;
709754
for (const std::string& name : apexUpdatableNames) {
710-
if (mAccess->canFind(ctx, name)) {
755+
if (auto status = canFindService(ctx, name, &_accessorName); status.isOk()) {
711756
outReturn->push_back(name);
712757
}
713758
}
@@ -724,8 +769,9 @@ Status ServiceManager::getConnectionInfo(const std::string& name,
724769

725770
auto ctx = mAccess->getCallingContext();
726771

727-
if (!mAccess->canFind(ctx, name)) {
728-
return Status::fromExceptionCode(Status::EX_SECURITY, "SELinux denied.");
772+
std::optional<std::string> _accessorName;
773+
if (auto status = canFindService(ctx, name, &_accessorName); !status.isOk()) {
774+
return status;
729775
}
730776

731777
*outReturn = std::nullopt;
@@ -1032,6 +1078,23 @@ Status ServiceManager::tryUnregisterService(const std::string& name, const sp<IB
10321078
return Status::ok();
10331079
}
10341080

1081+
Status ServiceManager::canFindService(const Access::CallingContext& ctx, const std::string& name,
1082+
std::optional<std::string>* accessor) {
1083+
if (!mAccess->canFind(ctx, name)) {
1084+
return Status::fromExceptionCode(Status::EX_SECURITY, "SELinux denied for service.");
1085+
}
1086+
#ifndef VENDORSERVICEMANAGER
1087+
*accessor = getVintfAccessorName(name);
1088+
#endif
1089+
if (accessor->has_value()) {
1090+
if (!mAccess->canFind(ctx, accessor->value())) {
1091+
return Status::fromExceptionCode(Status::EX_SECURITY,
1092+
"SELinux denied for the accessor of the service.");
1093+
}
1094+
}
1095+
return Status::ok();
1096+
}
1097+
10351098
Status ServiceManager::getServiceDebugInfo(std::vector<ServiceDebugInfo>* outReturn) {
10361099
SM_PERFETTO_TRACE_FUNC();
10371100
if (!mAccess->canList(mAccess->getCallingContext())) {

‎cmds/servicemanager/ServiceManager.h‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,8 @@ class ServiceManager : public os::BnServiceManager, public IBinder::DeathRecipie
4444
~ServiceManager();
4545

4646
// getService will try to start any services it cannot find
47-
binder::Status getService(const std::string& name, sp<IBinder>* outBinder) override;
48-
binder::Status checkService(const std::string& name, sp<IBinder>* outBinder) override;
47+
binder::Status getService(const std::string& name, os::Service* outService) override;
48+
binder::Status checkService(const std::string& name, os::Service* outService) override;
4949
binder::Status addService(const std::string& name, const sp<IBinder>& binder,
5050
bool allowIsolated, int32_t dumpPriority) override;
5151
binder::Status listServices(int32_t dumpPriority, std::vector<std::string>* outList) override;
@@ -112,7 +112,10 @@ class ServiceManager : public os::BnServiceManager, public IBinder::DeathRecipie
112112
// this updates the iterator to the next location
113113
void removeClientCallback(const wp<IBinder>& who, ClientCallbackMap::iterator* it);
114114

115-
sp<IBinder> tryGetService(const std::string& name, bool startIfNotFound);
115+
os::Service tryGetService(const std::string& name, bool startIfNotFound);
116+
sp<IBinder> tryGetBinder(const std::string& name, bool startIfNotFound);
117+
binder::Status canFindService(const Access::CallingContext& ctx, const std::string& name,
118+
std::optional<std::string>* accessor);
116119

117120
ServiceMap mNameToService;
118121
ServiceCallbackMap mNameToRegistrationCallback;

‎cmds/servicemanager/test_sm.cpp‎

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ using android::base::StartsWith;
3838
using android::binder::Status;
3939
using android::os::BnServiceCallback;
4040
using android::os::IServiceManager;
41+
using android::os::Service;
4142
using testing::_;
4243
using testing::ElementsAre;
4344
using testing::NiceMock;
@@ -153,18 +154,18 @@ TEST(AddService, OverwriteExistingService) {
153154
EXPECT_TRUE(sm->addService("foo", serviceA, false /*allowIsolated*/,
154155
IServiceManager::DUMP_FLAG_PRIORITY_DEFAULT).isOk());
155156

156-
sp<IBinder> outA;
157+
Service outA;
157158
EXPECT_TRUE(sm->getService("foo", &outA).isOk());
158-
EXPECT_EQ(serviceA, outA);
159+
EXPECT_EQ(serviceA, outA.get<Service::Tag::binder>());
159160

160161
// serviceA should be overwritten by serviceB
161162
sp<IBinder> serviceB = getBinder();
162163
EXPECT_TRUE(sm->addService("foo", serviceB, false /*allowIsolated*/,
163164
IServiceManager::DUMP_FLAG_PRIORITY_DEFAULT).isOk());
164165

165-
sp<IBinder> outB;
166+
Service outB;
166167
EXPECT_TRUE(sm->getService("foo", &outB).isOk());
167-
EXPECT_EQ(serviceB, outB);
168+
EXPECT_EQ(serviceB, outB.get<Service::Tag::binder>());
168169
}
169170

170171
TEST(AddService, NoPermissions) {
@@ -186,17 +187,17 @@ TEST(GetService, HappyHappy) {
186187
EXPECT_TRUE(sm->addService("foo", service, false /*allowIsolated*/,
187188
IServiceManager::DUMP_FLAG_PRIORITY_DEFAULT).isOk());
188189

189-
sp<IBinder> out;
190+
Service out;
190191
EXPECT_TRUE(sm->getService("foo", &out).isOk());
191-
EXPECT_EQ(service, out);
192+
EXPECT_EQ(service, out.get<Service::Tag::binder>());
192193
}
193194

194195
TEST(GetService, NonExistant) {
195196
auto sm = getPermissiveServiceManager();
196197

197-
sp<IBinder> out;
198+
Service out;
198199
EXPECT_TRUE(sm->getService("foo", &out).isOk());
199-
EXPECT_EQ(nullptr, out.get());
200+
EXPECT_EQ(nullptr, out.get<Service::Tag::binder>());
200201
}
201202

202203
TEST(GetService, NoPermissionsForGettingService) {
@@ -211,10 +212,10 @@ TEST(GetService, NoPermissionsForGettingService) {
211212
EXPECT_TRUE(sm->addService("foo", getBinder(), false /*allowIsolated*/,
212213
IServiceManager::DUMP_FLAG_PRIORITY_DEFAULT).isOk());
213214

214-
sp<IBinder> out;
215+
Service out;
215216
// returns nullptr but has OK status for legacy compatibility
216217
EXPECT_TRUE(sm->getService("foo", &out).isOk());
217-
EXPECT_EQ(nullptr, out.get());
218+
EXPECT_EQ(nullptr, out.get<Service::Tag::binder>());
218219
}
219220

220221
TEST(GetService, AllowedFromIsolated) {
@@ -236,9 +237,9 @@ TEST(GetService, AllowedFromIsolated) {
236237
EXPECT_TRUE(sm->addService("foo", service, true /*allowIsolated*/,
237238
IServiceManager::DUMP_FLAG_PRIORITY_DEFAULT).isOk());
238239

239-
sp<IBinder> out;
240+
Service out;
240241
EXPECT_TRUE(sm->getService("foo", &out).isOk());
241-
EXPECT_EQ(service, out.get());
242+
EXPECT_EQ(service, out.get<Service::Tag::binder>());
242243
}
243244

244245
TEST(GetService, NotAllowedFromIsolated) {
@@ -261,10 +262,10 @@ TEST(GetService, NotAllowedFromIsolated) {
261262
EXPECT_TRUE(sm->addService("foo", getBinder(), false /*allowIsolated*/,
262263
IServiceManager::DUMP_FLAG_PRIORITY_DEFAULT).isOk());
263264

264-
sp<IBinder> out;
265+
Service out;
265266
// returns nullptr but has OK status for legacy compatibility
266267
EXPECT_TRUE(sm->getService("foo", &out).isOk());
267-
EXPECT_EQ(nullptr, out.get());
268+
EXPECT_EQ(nullptr, out.get<Service::Tag::binder>());
268269
}
269270

270271
TEST(ListServices, NoPermissions) {

‎libs/binder/Android.bp‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -771,11 +771,41 @@ filegroup {
771771
"aidl/android/os/IClientCallback.aidl",
772772
"aidl/android/os/IServiceCallback.aidl",
773773
"aidl/android/os/IServiceManager.aidl",
774+
"aidl/android/os/Service.aidl",
774775
"aidl/android/os/ServiceDebugInfo.aidl",
776+
":libbinder_accessor_aidl",
775777
],
776778
path: "aidl",
777779
}
778780

781+
filegroup {
782+
name: "libbinder_accessor_aidl",
783+
srcs: [
784+
"aidl/android/os/IAccessor.aidl",
785+
],
786+
path: "aidl",
787+
}
788+
789+
// TODO(b/353492849): Make this interface private to libbinder.
790+
aidl_interface {
791+
name: "android.os.accessor",
792+
srcs: [":libbinder_accessor_aidl"],
793+
unstable: true,
794+
backend: {
795+
rust: {
796+
enabled: true,
797+
apex_available: [
798+
"com.android.virt",
799+
],
800+
},
801+
},
802+
visibility: [
803+
":__subpackages__",
804+
"//system/tools/aidl:__subpackages__",
805+
"//packages/modules/Virtualization:__subpackages__",
806+
],
807+
}
808+
779809
aidl_interface {
780810
name: "packagemanager_aidl",
781811
unstable: true,

0 commit comments

Comments
 (0)