From 79157a35cce54fb222d9461d5325a0ddae92a242 Mon Sep 17 00:00:00 2001 From: Mohammad Samiul Islam Date: Thu, 10 Mar 2022 11:32:23 +0000 Subject: Ensure subdirectories of sdk data inherit group id of parent When sandbox process creates files under sdk data directory, the files get their owner id and group id set to sandbox uid. This is the default behavior. This causes problems for storage calculation. These files should inherit their parent group id, which is AID_NOBODY. Bug: 215506889 Test: atest installd_service_test Ignore-AOSP-First: Some of the cls are missing in AOSP. Will cherry-pick this with rest of them together next week. Change-Id: I4a2e97a033d1faece97600d3188c81e094a02c21 Merged-In: I4a2e97a033d1faece97600d3188c81e094a02c21 (cherry picked from commit 431d92181847a7da3679a8cdd7d545441803d101) --- cmds/installd/InstalldNativeService.cpp | 4 ++-- cmds/installd/tests/installd_service_test.cpp | 22 ++++++++++++---------- 2 files changed, 14 insertions(+), 12 deletions(-) diff --git a/cmds/installd/InstalldNativeService.cpp b/cmds/installd/InstalldNativeService.cpp index baf62782e5..03fbd5d62f 100644 --- a/cmds/installd/InstalldNativeService.cpp +++ b/cmds/installd/InstalldNativeService.cpp @@ -813,7 +813,7 @@ binder::Status InstalldNativeService::createSdkSandboxDataPackageDirectory( StringPrintf("cacheGid cannot be -1 for sdksandbox data")); } auto status = createAppDataDirs(sharedPath, sdkSandboxUid, AID_NOBODY, - &previousSdkSandboxUid, cacheGid, seInfo, 0700); + &previousSdkSandboxUid, cacheGid, seInfo, 0700 | S_ISGID); if (!status.isOk()) { return status; } @@ -993,7 +993,7 @@ binder::Status InstalldNativeService::reconcileSdkData( const int32_t sandboxUid = multiuser_get_sdk_sandbox_uid(userId, appId); int32_t previousSandboxUid = multiuser_get_sdk_sandbox_uid(userId, previousAppId); auto status = createAppDataDirs(path, sandboxUid, AID_NOBODY, &previousSandboxUid, - cacheGid, seInfo, 0700); + cacheGid, seInfo, 0700 | S_ISGID); if (!status.isOk()) { res = status; continue; diff --git a/cmds/installd/tests/installd_service_test.cpp b/cmds/installd/tests/installd_service_test.cpp index 912b088159..65edc802b9 100644 --- a/cmds/installd/tests/installd_service_test.cpp +++ b/cmds/installd/tests/installd_service_test.cpp @@ -1126,7 +1126,8 @@ TEST_F(SdkSandboxDataTest, CreateAppData_CreatesSdkPackageData) { const std::string fooCePath = "misc_ce/0/sdksandbox/com.foo"; CheckFileAccess(fooCePath, kSystemUid, kSystemUid, S_IFDIR | 0751); - CheckFileAccess(fooCePath + "/shared", kTestSdkSandboxUid, kNobodyUid, S_IFDIR | 0700); + CheckFileAccess(fooCePath + "/shared", kTestSdkSandboxUid, kNobodyUid, + S_IFDIR | S_ISGID | 0700); CheckFileAccess(fooCePath + "/shared/cache", kTestSdkSandboxUid, kTestCacheGid, S_IFDIR | S_ISGID | 0771); CheckFileAccess(fooCePath + "/shared/code_cache", kTestSdkSandboxUid, kTestCacheGid, @@ -1134,7 +1135,8 @@ TEST_F(SdkSandboxDataTest, CreateAppData_CreatesSdkPackageData) { const std::string fooDePath = "misc_de/0/sdksandbox/com.foo"; CheckFileAccess(fooDePath, kSystemUid, kSystemUid, S_IFDIR | 0751); - CheckFileAccess(fooDePath + "/shared", kTestSdkSandboxUid, kNobodyUid, S_IFDIR | 0700); + CheckFileAccess(fooDePath + "/shared", kTestSdkSandboxUid, kNobodyUid, + S_IFDIR | S_ISGID | 0700); CheckFileAccess(fooDePath + "/shared/cache", kTestSdkSandboxUid, kTestCacheGid, S_IFDIR | S_ISGID | 0771); CheckFileAccess(fooDePath + "/shared/code_cache", kTestSdkSandboxUid, kTestCacheGid, @@ -1205,28 +1207,28 @@ TEST_F(SdkSandboxDataTest, ReconcileSdkData) { ASSERT_BINDER_SUCCESS(service->reconcileSdkData(args)); const std::string barCePath = "misc_ce/0/sdksandbox/com.foo/bar@random1"; - CheckFileAccess(barCePath, kTestSdkSandboxUid, kNobodyUid, S_IFDIR | 0700); + CheckFileAccess(barCePath, kTestSdkSandboxUid, kNobodyUid, S_IFDIR | S_ISGID | 0700); CheckFileAccess(barCePath + "/cache", kTestSdkSandboxUid, kTestCacheGid, S_IFDIR | S_ISGID | 0771); CheckFileAccess(barCePath + "/code_cache", kTestSdkSandboxUid, kTestCacheGid, S_IFDIR | S_ISGID | 0771); const std::string bazCePath = "misc_ce/0/sdksandbox/com.foo/baz@random2"; - CheckFileAccess(bazCePath, kTestSdkSandboxUid, kNobodyUid, S_IFDIR | 0700); + CheckFileAccess(bazCePath, kTestSdkSandboxUid, kNobodyUid, S_IFDIR | S_ISGID | 0700); CheckFileAccess(bazCePath + "/cache", kTestSdkSandboxUid, kTestCacheGid, S_IFDIR | S_ISGID | 0771); CheckFileAccess(bazCePath + "/code_cache", kTestSdkSandboxUid, kTestCacheGid, S_IFDIR | S_ISGID | 0771); const std::string barDePath = "misc_de/0/sdksandbox/com.foo/bar@random1"; - CheckFileAccess(barDePath, kTestSdkSandboxUid, kNobodyUid, S_IFDIR | 0700); + CheckFileAccess(barDePath, kTestSdkSandboxUid, kNobodyUid, S_IFDIR | S_ISGID | 0700); CheckFileAccess(barDePath + "/cache", kTestSdkSandboxUid, kTestCacheGid, S_IFDIR | S_ISGID | 0771); CheckFileAccess(barDePath + "/code_cache", kTestSdkSandboxUid, kTestCacheGid, S_IFDIR | S_ISGID | 0771); const std::string bazDePath = "misc_de/0/sdksandbox/com.foo/baz@random2"; - CheckFileAccess(bazDePath, kTestSdkSandboxUid, kNobodyUid, S_IFDIR | 0700); + CheckFileAccess(bazDePath, kTestSdkSandboxUid, kNobodyUid, S_IFDIR | S_ISGID | 0700); CheckFileAccess(bazDePath + "/cache", kTestSdkSandboxUid, kTestCacheGid, S_IFDIR | S_ISGID | 0771); CheckFileAccess(bazDePath + "/code_cache", kTestSdkSandboxUid, kTestCacheGid, @@ -1269,9 +1271,9 @@ TEST_F(SdkSandboxDataTest, ReconcileSdkData_DirectoryNotCreatedIfAlreadyExistsIg // Previous directories from first attempt should exist CheckFileAccess("misc_ce/0/sdksandbox/com.foo/bar@random1", kTestSdkSandboxUid, kNobodyUid, - S_IFDIR | 0700); + S_IFDIR | S_ISGID | 0700); CheckFileAccess("misc_ce/0/sdksandbox/com.foo/baz@random2", kTestSdkSandboxUid, kNobodyUid, - S_IFDIR | 0700); + S_IFDIR | S_ISGID | 0700); // No new directories should be created on second attempt ASSERT_FALSE(exists("/data/local/tmp/misc_ce/0/sdksandbox/com.foo/bar@r10")); ASSERT_FALSE(exists("/data/local/tmp/misc_de/0/sdksandbox/com.foo/bar@r20")); @@ -1292,9 +1294,9 @@ TEST_F(SdkSandboxDataTest, ReconcileSdkData_ExtraCodeDirectoriesAreDeleted) { // New directoris should exist CheckFileAccess("misc_ce/0/sdksandbox/com.foo/bar.diff@random1", kTestSdkSandboxUid, kNobodyUid, - S_IFDIR | 0700); + S_IFDIR | S_ISGID | 0700); CheckFileAccess("misc_ce/0/sdksandbox/com.foo/baz@random2", kTestSdkSandboxUid, kNobodyUid, - S_IFDIR | 0700); + S_IFDIR | S_ISGID | 0700); // Directory for old unreferred sdksandbox package name should be removed ASSERT_FALSE(exists("/data/local/tmp/misc_ce/0/sdksandbox/com.foo/bar@random1")); } -- cgit v1.2.3-59-g8ed1b