From b27bbd18bb65b3744ae066fcd6826285dec8b469 Mon Sep 17 00:00:00 2001 From: Nick Kralevich Date: Thu, 5 Mar 2015 10:58:40 -0800 Subject: [PATCH] service_manager: reorder permission checks for find Reorder the find permission checks. This avoids generating misleading SELinux denials when a service doesn't exist, or when a service is prohibited to isolated apps. The original reason for structuring the code this way is explained in https://android-review.googlesource.com/#/c/100530/4/cmds/servicemanager/service_manager.c@172 The concern at the time was to avoid leaking a situation where a caller could probe for the existance of a service. This turns out to be unnecessary. The same return value is used for both a permission denied and a service not found. The only side effect is the generation of an SELinux audit log, which likely won't be accessible to the calling application. Change-Id: I9760e1821ed16102fa5f9bec07f8c34944565be9 --- cmds/servicemanager/service_manager.c | 34 +++++++++++++-------------- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/cmds/servicemanager/service_manager.c b/cmds/servicemanager/service_manager.c index f37427a0b..df46a6087 100644 --- a/cmds/servicemanager/service_manager.c +++ b/cmds/servicemanager/service_manager.c @@ -169,28 +169,26 @@ uint16_t svcmgr_id[] = { uint32_t do_find_service(struct binder_state *bs, const uint16_t *s, size_t len, uid_t uid, pid_t spid) { - struct svcinfo *si; + struct svcinfo *si = find_svc(s, len); + + if (!si || !si->handle) { + return 0; + } + + if (!si->allow_isolated) { + // If this service doesn't allow access from isolated processes, + // then check the uid to see if it is isolated. + uid_t appid = uid % AID_USER; + if (appid >= AID_ISOLATED_START && appid <= AID_ISOLATED_END) { + return 0; + } + } if (!svc_can_find(s, len, spid)) { - ALOGE("find_service('%s') uid=%d - PERMISSION DENIED\n", - str8(s, len), uid); - return 0; - } - si = find_svc(s, len); - //ALOGI("check_service('%s') handle = %x\n", str8(s, len), si ? si->handle : 0); - if (si && si->handle) { - if (!si->allow_isolated) { - // If this service doesn't allow access from isolated processes, - // then check the uid to see if it is isolated. - uid_t appid = uid % AID_USER; - if (appid >= AID_ISOLATED_START && appid <= AID_ISOLATED_END) { - return 0; - } - } - return si->handle; - } else { return 0; } + + return si->handle; } int do_add_service(struct binder_state *bs,