transcoding: use NDK methods for permission and binder
- Change permission checks to use NDK
- Replace MediaProvider uid check with permission check of
MANAGE_EXTERNAL_STORAGE (which is replacing the deprecated
WRITE_MEDIA_STORAGE).
- Use ABinderProcess_* for thread pool.
bug: 159172726
bug: 171393067
test: MediaTranscodingService unit tests; dumpsys to check if
MediaProvider can still regiserClient successfully.
Change-Id: I4fed7bb23a2d68c92e5cd1aecd5a6ef340dc2b61
diff --git a/media/libmediatranscoding/Android.bp b/media/libmediatranscoding/Android.bp
index 7329c63..763a73e 100644
--- a/media/libmediatranscoding/Android.bp
+++ b/media/libmediatranscoding/Android.bp
@@ -60,6 +60,7 @@
],
shared_libs: [
+ "libandroid",
"libbinder_ndk",
"libcutils",
"liblog",
diff --git a/media/libmediatranscoding/TranscodingClientManager.cpp b/media/libmediatranscoding/TranscodingClientManager.cpp
index ae1f7a5..d94c468 100644
--- a/media/libmediatranscoding/TranscodingClientManager.cpp
+++ b/media/libmediatranscoding/TranscodingClientManager.cpp
@@ -20,6 +20,7 @@
#include <aidl/android/media/BnTranscodingClient.h>
#include <aidl/android/media/IMediaTranscodingService.h>
#include <android/binder_ibinder.h>
+#include <android/permission_manager.h>
#include <inttypes.h>
#include <media/TranscodingClientManager.h>
#include <media/TranscodingRequest.h>
@@ -27,15 +28,11 @@
#include <private/android_filesystem_config.h>
#include <utils/Log.h>
#include <utils/String16.h>
+
namespace android {
static_assert(sizeof(ClientIdType) == sizeof(void*), "ClientIdType should be pointer-sized");
-static constexpr const char* MEDIA_PROVIDER_PKG_NAMES[] = {
- "com.android.providers.media.module",
- "com.google.android.providers.media.module",
-};
-
using ::aidl::android::media::BnTranscodingClient;
using ::aidl::android::media::IMediaTranscodingService; // For service error codes
using ::aidl::android::media::TranscodingRequestParcel;
@@ -137,7 +134,7 @@
in_clientUid = callingUid;
} else if (in_clientUid < 0) {
return Status::ok();
- } else if (in_clientUid != callingUid && !owner->isTrustedCallingUid(callingUid)) {
+ } else if (in_clientUid != callingUid && !owner->isTrustedCaller(callingPid, callingUid)) {
ALOGE("MediaTranscodingService::registerClient rejected (clientPid %d, clientUid %d) "
"(don't trust callingUid %d)",
in_clientPid, in_clientUid, callingUid);
@@ -154,7 +151,7 @@
in_clientPid = callingPid;
} else if (in_clientPid < 0) {
return Status::ok();
- } else if (in_clientPid != callingPid && !owner->isTrustedCallingUid(callingUid)) {
+ } else if (in_clientPid != callingPid && !owner->isTrustedCaller(callingPid, callingUid)) {
ALOGE("MediaTranscodingService::registerClient rejected (clientPid %d, clientUid %d) "
"(don't trust callingUid %d)",
in_clientPid, in_clientUid, callingUid);
@@ -266,14 +263,8 @@
: mDeathRecipient(AIBinder_DeathRecipient_new(BinderDiedCallback)),
mSessionController(controller) {
ALOGD("TranscodingClientManager started");
- uid_t mpuid;
- for (const char* pkgName : MEDIA_PROVIDER_PKG_NAMES) {
- if (TranscodingUidPolicy::getUidForPackage(String16(pkgName), mpuid) == NO_ERROR) {
- ALOGI("Found %s's uid: %d", pkgName, mpuid);
- mMediaProviderUid.insert(mpuid);
- } else {
- ALOGW("Couldn't get uid for %s.", pkgName);
- }
+ for (uid_t uid : {AID_ROOT, AID_SYSTEM, AID_SHELL, AID_MEDIA}) {
+ mTrustedUids.insert(uid);
}
}
@@ -305,20 +296,20 @@
write(fd, result.string(), result.size());
}
-bool TranscodingClientManager::isTrustedCallingUid(uid_t uid) {
- if (uid > 0 && mMediaProviderUid.count(uid) > 0) {
+bool TranscodingClientManager::isTrustedCaller(pid_t pid, uid_t uid) {
+ if (uid > 0 && mTrustedUids.count(uid) > 0) {
return true;
}
- switch (uid) {
- case AID_ROOT: // root user
- case AID_SYSTEM:
- case AID_SHELL:
- case AID_MEDIA: // mediaserver
+ int32_t result;
+ if (APermissionManager_checkPermission("android.permission.MANAGE_EXTERNAL_STORAGE", pid, uid,
+ &result) == PERMISSION_MANAGER_STATUS_OK &&
+ result == PERMISSION_MANAGER_PERMISSION_GRANTED) {
+ mTrustedUids.insert(uid);
return true;
- default:
- return false;
}
+
+ return false;
}
status_t TranscodingClientManager::addClient(
diff --git a/media/libmediatranscoding/TranscodingUidPolicy.cpp b/media/libmediatranscoding/TranscodingUidPolicy.cpp
index 084a871..fdda327 100644
--- a/media/libmediatranscoding/TranscodingUidPolicy.cpp
+++ b/media/libmediatranscoding/TranscodingUidPolicy.cpp
@@ -21,10 +21,7 @@
#include <aidl/android/media/IResourceManagerService.h>
#include <android/binder_manager.h>
#include <android/binder_process.h>
-#include <android/content/pm/IPackageManagerNative.h>
#include <binder/ActivityManager.h>
-#include <binder/IServiceManager.h>
-#include <binder/PermissionController.h>
#include <cutils/misc.h> // FIRST_APPLICATION_UID
#include <cutils/multiuser.h>
#include <inttypes.h>
@@ -113,19 +110,6 @@
////////////////////////////////////////////////////////////////////////////
-//static
-status_t TranscodingUidPolicy::getUidForPackage(String16 packageName, /*inout*/ uid_t& uid) {
- PermissionController pc;
- uid = pc.getPackageUid(packageName, 0);
- if (uid <= 0) {
- ALOGE("Unknown package: '%s'", String8(packageName).string());
- return BAD_VALUE;
- }
-
- uid = multiuser_get_uid(0 /*userId*/, uid);
- return NO_ERROR;
-}
-
TranscodingUidPolicy::TranscodingUidPolicy()
: mAm(std::make_shared<ActivityManager>()),
mUidObserver(new UidObserver(this)),
diff --git a/media/libmediatranscoding/include/media/TranscodingClientManager.h b/media/libmediatranscoding/include/media/TranscodingClientManager.h
index 451f993..be55c78 100644
--- a/media/libmediatranscoding/include/media/TranscodingClientManager.h
+++ b/media/libmediatranscoding/include/media/TranscodingClientManager.h
@@ -87,7 +87,7 @@
TranscodingClientManager(const std::shared_ptr<ControllerClientInterface>& controller);
// Checks if a user is trusted (and allowed to submit sessions on behalf of other uids)
- bool isTrustedCallingUid(uid_t uid);
+ bool isTrustedCaller(pid_t pid, uid_t uid);
/**
* Removes an existing client from the manager.
@@ -109,7 +109,7 @@
::ndk::ScopedAIBinder_DeathRecipient mDeathRecipient;
std::shared_ptr<ControllerClientInterface> mSessionController;
- std::unordered_set<uid_t> mMediaProviderUid;
+ std::unordered_set<uid_t> mTrustedUids;
static std::atomic<ClientIdType> sCookieCounter;
static std::mutex sCookie2ClientLock;
diff --git a/media/libmediatranscoding/include/media/TranscodingUidPolicy.h b/media/libmediatranscoding/include/media/TranscodingUidPolicy.h
index 4c642de..dec67b9 100644
--- a/media/libmediatranscoding/include/media/TranscodingUidPolicy.h
+++ b/media/libmediatranscoding/include/media/TranscodingUidPolicy.h
@@ -49,8 +49,6 @@
void setCallback(const std::shared_ptr<UidPolicyCallbackInterface>& cb) override;
// ~UidPolicyInterface
- static status_t getUidForPackage(String16 packageName, /*inout*/ uid_t& uid);
-
private:
void onUidStateChanged(uid_t uid, int32_t procState);
void setUidObserverRegistered(bool registerd);
diff --git a/services/mediatranscoding/Android.bp b/services/mediatranscoding/Android.bp
index 2dbcf5a..3d6821b 100644
--- a/services/mediatranscoding/Android.bp
+++ b/services/mediatranscoding/Android.bp
@@ -8,8 +8,8 @@
],
shared_libs: [
+ "libandroid",
"libbase",
- "libbinder",
"libbinder_ndk",
"libcutils",
"liblog",
@@ -40,8 +40,7 @@
shared_libs: [
"libbase",
- // TODO(hkuang): Use libbinder_ndk
- "libbinder",
+ "libbinder_ndk",
"libutils",
"liblog",
"libbase",
diff --git a/services/mediatranscoding/MediaTranscodingService.cpp b/services/mediatranscoding/MediaTranscodingService.cpp
index 56f327e..64def5e 100644
--- a/services/mediatranscoding/MediaTranscodingService.cpp
+++ b/services/mediatranscoding/MediaTranscodingService.cpp
@@ -20,7 +20,7 @@
#include <android/binder_manager.h>
#include <android/binder_process.h>
-#include <binder/IServiceManager.h>
+#include <android/permission_manager.h>
#include <cutils/properties.h>
#include <media/TranscoderWrapper.h>
#include <media/TranscodingClientManager.h>
@@ -60,8 +60,12 @@
binder_status_t MediaTranscodingService::dump(int fd, const char** /*args*/, uint32_t /*numArgs*/) {
String8 result;
- // TODO(b/161549994): Remove libbinder dependencies for mainline.
- if (checkCallingPermission(String16("android.permission.DUMP")) == false) {
+ uid_t callingUid = AIBinder_getCallingUid();
+ pid_t callingPid = AIBinder_getCallingPid();
+ int32_t permissionResult;
+ if (APermissionManager_checkPermission("android.permission.DUMP", callingPid, callingUid,
+ &permissionResult) != PERMISSION_MANAGER_STATUS_OK ||
+ permissionResult != PERMISSION_MANAGER_PERMISSION_GRANTED) {
result.format(
"Permission Denial: "
"can't dump MediaTranscodingService from pid=%d, uid=%d\n",
diff --git a/services/mediatranscoding/main_mediatranscodingservice.cpp b/services/mediatranscoding/main_mediatranscodingservice.cpp
index 7d862e6..14c568e 100644
--- a/services/mediatranscoding/main_mediatranscodingservice.cpp
+++ b/services/mediatranscoding/main_mediatranscodingservice.cpp
@@ -15,8 +15,7 @@
*/
#include <android-base/logging.h>
-#include <binder/IPCThreadState.h>
-#include <binder/ProcessState.h>
+#include <android/binder_process.h>
#include "MediaTranscodingService.h"
@@ -25,12 +24,9 @@
int main(int argc __unused, char** argv) {
LOG(INFO) << "media transcoding service starting";
- // TODO(hkuang): Start the service with libbinder_ndk.
strcpy(argv[0], "media.transcoding");
- sp<ProcessState> proc(ProcessState::self());
- sp<IServiceManager> sm = defaultServiceManager();
android::MediaTranscodingService::instantiate();
- ProcessState::self()->startThreadPool();
- IPCThreadState::self()->joinThreadPool();
+ ABinderProcess_startThreadPool();
+ ABinderProcess_joinThreadPool();
}