Don't provide read logs for shell-initiated installations.

Only if the application is profileable.

Bug: 158238023
Fixes: 158238023
Test: atest PackageManagerShellCommandTest PackageManagerShellCommandIncrementalTest IncrementalServiceTest PackageParserTest
Change-Id: I8575830ec3f29850297fdbfbaa157072d6350a28
Merged-In: I8575830ec3f29850297fdbfbaa157072d6350a28

5 files changed, 92 insertions, 1 deletions

diff --git a/services/incremental/BinderIncrementalService.cpp b/services/incremental/BinderIncrementalService.cpp
index e790a196ad64..7132706c4ef1 100644
--- a/services/incremental/BinderIncrementalService.cpp
+++ b/services/incremental/BinderIncrementalService.cpp
@@ -165,6 +165,11 @@ binder::Status BinderIncrementalService::deleteStorage(int32_t storageId) {
     return ok();
 }
 
+binder::Status BinderIncrementalService::disableReadLogs(int32_t storageId) {
+    mImpl.disableReadLogs(storageId);
+    return ok();
+}
+
 binder::Status BinderIncrementalService::makeDirectory(int32_t storageId, const std::string& path,
                                                        int32_t* _aidl_return) {
     *_aidl_return = mImpl.makeDir(storageId, path);
diff --git a/services/incremental/BinderIncrementalService.h b/services/incremental/BinderIncrementalService.h
index 68549f5a8ff8..10154946d3ee 100644
--- a/services/incremental/BinderIncrementalService.h
+++ b/services/incremental/BinderIncrementalService.h
@@ -74,7 +74,7 @@ public:
                                    std::vector<uint8_t>* _aidl_return) final;
     binder::Status startLoading(int32_t storageId, bool* _aidl_return) final;
     binder::Status deleteStorage(int32_t storageId) final;
-
+    binder::Status disableReadLogs(int32_t storageId) final;
     binder::Status configureNativeBinaries(int32_t storageId, const std::string& apkFullPath,
                                            const std::string& libDirRelativePath,
                                            const std::string& abi, bool extractNativeLibs,
diff --git a/services/incremental/IncrementalService.cpp b/services/incremental/IncrementalService.cpp
index 885f4d2d34d7..3450c3ae9fb3 100644
--- a/services/incremental/IncrementalService.cpp
+++ b/services/incremental/IncrementalService.cpp
@@ -60,6 +60,7 @@ struct Constants {
     static constexpr auto storagePrefix = "st"sv;
     static constexpr auto mountpointMdPrefix = ".mountpoint."sv;
     static constexpr auto infoMdName = ".info"sv;
+    static constexpr auto readLogsDisabledMarkerName = ".readlogs_disabled"sv;
     static constexpr auto libDir = "lib"sv;
     static constexpr auto libSuffix = ".so"sv;
     static constexpr auto blockSize = 4096;
@@ -172,6 +173,13 @@ std::string makeBindMdName() {
 
     return name;
 }
+
+static bool checkReadLogsDisabledMarker(std::string_view root) {
+    const auto markerPath = path::c_str(path::join(root, constants().readLogsDisabledMarkerName));
+    struct stat st;
+    return (::stat(markerPath, &st) == 0);
+}
+
 } // namespace
 
 IncrementalService::IncFsMount::~IncFsMount() {
@@ -618,6 +626,32 @@ StorageId IncrementalService::findStorageId(std::string_view path) const {
     return it->second->second.storage;
 }
 
+void IncrementalService::disableReadLogs(StorageId storageId) {
+    std::unique_lock l(mLock);
+    const auto ifs = getIfsLocked(storageId);
+    if (!ifs) {
+        LOG(ERROR) << "disableReadLogs failed, invalid storageId: " << storageId;
+        return;
+    }
+    if (!ifs->readLogsEnabled()) {
+        return;
+    }
+    ifs->disableReadLogs();
+    l.unlock();
+
+    const auto metadata = constants().readLogsDisabledMarkerName;
+    if (auto err = mIncFs->makeFile(ifs->control,
+                                    path::join(ifs->root, constants().mount,
+                                               constants().readLogsDisabledMarkerName),
+                                    0777, idFromMetadata(metadata), {})) {
+        //{.metadata = {metadata.data(), (IncFsSize)metadata.size()}})) {
+        LOG(ERROR) << "Failed to make marker file for storageId: " << storageId;
+        return;
+    }
+
+    setStorageParams(storageId, /*enableReadLogs=*/false);
+}
+
 int IncrementalService::setStorageParams(StorageId storageId, bool enableReadLogs) {
     const auto ifs = getIfs(storageId);
     if (!ifs) {
@@ -627,6 +661,11 @@ int IncrementalService::setStorageParams(StorageId storageId, bool enableReadLog
 
     const auto& params = ifs->dataLoaderStub->params();
     if (enableReadLogs) {
+        if (!ifs->readLogsEnabled()) {
+            LOG(ERROR) << "setStorageParams failed, readlogs disabled for storageId: " << storageId;
+            return -EPERM;
+        }
+
         if (auto status = mAppOpsManager->checkPermission(kDataUsageStats, kOpUsage,
                                                           params.packageName.c_str());
             !status.isOk()) {
@@ -1072,6 +1111,11 @@ std::unordered_set<std::string_view> IncrementalService::adoptMountedInstances()
                                                 std::move(control), *this);
         cleanupFiles.release(); // ifs will take care of that now
 
+        // Check if marker file present.
+        if (checkReadLogsDisabledMarker(root)) {
+            ifs->disableReadLogs();
+        }
+
         std::vector<std::pair<std::string, metadata::BindPoint>> permanentBindPoints;
         auto d = openDir(root);
         while (auto e = ::readdir(d.get())) {
@@ -1243,6 +1287,11 @@ bool IncrementalService::mountExistingImage(std::string_view root) {
     ifs->mountId = mount.storage().id();
     mNextId = std::max(mNextId, ifs->mountId + 1);
 
+    // Check if marker file present.
+    if (checkReadLogsDisabledMarker(mountTarget)) {
+        ifs->disableReadLogs();
+    }
+
     // DataLoader params
     DataLoaderParamsParcel dataLoaderParams;
     {
diff --git a/services/incremental/IncrementalService.h b/services/incremental/IncrementalService.h
index 918531b7921c..a6cc94639c8a 100644
--- a/services/incremental/IncrementalService.h
+++ b/services/incremental/IncrementalService.h
@@ -94,6 +94,10 @@ public:
         Permanent = 1,
     };
 
+    enum StorageFlags {
+        ReadLogsEnabled = 1,
+    };
+
     static FileId idFromMetadata(std::span<const uint8_t> metadata);
     static inline FileId idFromMetadata(std::span<const char> metadata) {
         return idFromMetadata({(const uint8_t*)metadata.data(), metadata.size()});
@@ -116,6 +120,7 @@ public:
     int unbind(StorageId storage, std::string_view target);
     void deleteStorage(StorageId storage);
 
+    void disableReadLogs(StorageId storage);
     int setStorageParams(StorageId storage, bool enableReadLogs);
 
     int makeFile(StorageId storage, std::string_view path, int mode, FileId id,
@@ -264,6 +269,7 @@ private:
         const std::string root;
         Control control;
         /*const*/ MountId mountId;
+        int32_t flags = StorageFlags::ReadLogsEnabled;
         StorageMap storages;
         BindMap bindPoints;
         DataLoaderStubPtr dataLoaderStub;
@@ -282,6 +288,9 @@ private:
 
         StorageMap::iterator makeStorage(StorageId id);
 
+        void disableReadLogs() { flags &= ~StorageFlags::ReadLogsEnabled; }
+        int32_t readLogsEnabled() const { return (flags & StorageFlags::ReadLogsEnabled); }
+
         static void cleanupFilesystem(std::string_view root);
     };
 
diff --git a/services/incremental/test/IncrementalServiceTest.cpp b/services/incremental/test/IncrementalServiceTest.cpp
index 26b5094a795a..1ae9e256c9f4 100644
--- a/services/incremental/test/IncrementalServiceTest.cpp
+++ b/services/incremental/test/IncrementalServiceTest.cpp
@@ -929,6 +929,34 @@ TEST_F(IncrementalServiceTest, testSetIncFsMountOptionsSuccess) {
     ASSERT_GE(mDataLoader->setStorageParams(true), 0);
 }
 
+TEST_F(IncrementalServiceTest, testSetIncFsMountOptionsSuccessAndDisabled) {
+    mVold->mountIncFsSuccess();
+    mIncFs->makeFileSuccess();
+    mVold->bindMountSuccess();
+    mVold->setIncFsMountOptionsSuccess();
+    mDataLoaderManager->bindToDataLoaderSuccess();
+    mDataLoaderManager->getDataLoaderSuccess();
+    mAppOpsManager->checkPermissionSuccess();
+    EXPECT_CALL(*mDataLoaderManager, unbindFromDataLoader(_));
+    EXPECT_CALL(*mVold, unmountIncFs(_)).Times(2);
+    // Enabling and then disabling readlogs.
+    EXPECT_CALL(*mVold, setIncFsMountOptions(_, true)).Times(1);
+    EXPECT_CALL(*mVold, setIncFsMountOptions(_, false)).Times(1);
+    // After setIncFsMountOptions succeeded expecting to start watching.
+    EXPECT_CALL(*mAppOpsManager, startWatchingMode(_, _, _)).Times(1);
+    // Not expecting callback removal.
+    EXPECT_CALL(*mAppOpsManager, stopWatchingMode(_)).Times(0);
+    TemporaryDir tempDir;
+    int storageId = mIncrementalService->createStorage(tempDir.path, std::move(mDataLoaderParcel),
+                                                       IncrementalService::CreateOptions::CreateNew,
+                                                       {}, {}, {});
+    ASSERT_GE(storageId, 0);
+    ASSERT_GE(mDataLoader->setStorageParams(true), 0);
+    // Now disable.
+    mIncrementalService->disableReadLogs(storageId);
+    ASSERT_EQ(mDataLoader->setStorageParams(true), -EPERM);
+}
+
 TEST_F(IncrementalServiceTest, testSetIncFsMountOptionsSuccessAndPermissionChanged) {
     mVold->mountIncFsSuccess();
     mIncFs->makeFileSuccess();