Limit read log reporting to 2hrs for non-system DLs (non-adb).

Bug: 182477087
Test: atest IncrementalServiceTest
Change-Id: I98c9ed3a2e8a91d26bcb879ab7073903ff7bb2c5

3 files changed, 216 insertions, 17 deletions

diff --git a/services/incremental/IncrementalService.cpp b/services/incremental/IncrementalService.cpp
index c38d0b3cc7db..e3fbeddc3a5f 100644
--- a/services/incremental/IncrementalService.cpp
+++ b/services/incremental/IncrementalService.cpp
@@ -86,6 +86,9 @@ struct Constants {
     static constexpr auto maxBindDelay = 10000s;
     static constexpr auto bindDelayMultiplier = 10;
     static constexpr auto bindDelayJitterDivider = 10;
+
+    // Max interval after system invoked the DL when readlog collection can be enabled.
+    static constexpr auto readLogsMaxInterval = 2h;
 };
 
 static const Constants& constants() {
@@ -290,6 +293,14 @@ void IncrementalService::IncFsMount::cleanupFilesystem(std::string_view root) {
     ::rmdir(path::c_str(root));
 }
 
+void IncrementalService::IncFsMount::setReadLogsEnabled(bool value) {
+    if (value) {
+        flags |= StorageFlags::ReadLogsEnabled;
+    } else {
+        flags &= ~StorageFlags::ReadLogsEnabled;
+    }
+}
+
 IncrementalService::IncrementalService(ServiceManagerWrapper&& sm, std::string_view rootDir)
       : mVold(sm.getVoldService()),
         mDataLoaderManager(sm.getDataLoaderManager()),
@@ -406,7 +417,7 @@ void IncrementalService::onDump(int fd) {
 }
 
 bool IncrementalService::needStartDataLoaderLocked(IncFsMount& ifs) {
-    if (ifs.dataLoaderStub->params().packageName == Constants::systemPackage) {
+    if (ifs.dataLoaderStub->isSystemDataLoader()) {
         return true;
     }
 
@@ -658,7 +669,7 @@ StorageId IncrementalService::createLinkedStorage(std::string_view mountPoint,
     return storageId;
 }
 
-bool IncrementalService::startLoading(StorageId storage,
+bool IncrementalService::startLoading(StorageId storageId,
                                       content::pm::DataLoaderParamsParcel&& dataLoaderParams,
                                       const DataLoaderStatusListener& statusListener,
                                       StorageHealthCheckParams&& healthCheckParams,
@@ -666,12 +677,12 @@ bool IncrementalService::startLoading(StorageId storage,
                                       const std::vector<PerUidReadTimeouts>& perUidReadTimeouts) {
     // Per Uid timeouts.
     if (!perUidReadTimeouts.empty()) {
-        setUidReadTimeouts(storage, perUidReadTimeouts);
+        setUidReadTimeouts(storageId, perUidReadTimeouts);
     }
 
     // Re-initialize DataLoader.
     std::unique_lock l(mLock);
-    const auto ifs = getIfsLocked(storage);
+    const auto ifs = getIfsLocked(storageId);
     if (!ifs) {
         return false;
     }
@@ -686,6 +697,32 @@ bool IncrementalService::startLoading(StorageId storage,
                                             std::move(healthCheckParams), &healthListener);
     CHECK(dataLoaderStub);
 
+    if (dataLoaderStub->isSystemDataLoader()) {
+        // Readlogs from system dataloader (adb) can always be collected.
+        ifs->startLoadingTs = TimePoint::max();
+    } else {
+        // Assign time when installation wants the DL to start streaming.
+        const auto startLoadingTs = mClock->now();
+        ifs->startLoadingTs = startLoadingTs;
+        // Setup a callback to disable the readlogs after max interval.
+        addTimedJob(*mTimedQueue, storageId, Constants::readLogsMaxInterval,
+                    [this, storageId, startLoadingTs]() {
+                        const auto ifs = getIfs(storageId);
+                        if (!ifs) {
+                            LOG(WARNING) << "Can't disable the readlogs, invalid storageId: "
+                                         << storageId;
+                            return;
+                        }
+                        if (ifs->startLoadingTs != startLoadingTs) {
+                            LOG(INFO) << "Can't disable the readlogs, timestamp mismatch (new "
+                                         "installation?): "
+                                      << storageId;
+                            return;
+                        }
+                        setStorageParams(*ifs, storageId, /*enableReadLogs=*/false);
+                    });
+    }
+
     return dataLoaderStub->requestStart();
 }
 
@@ -735,11 +772,16 @@ int IncrementalService::setStorageParams(StorageId storageId, bool enableReadLog
         LOG(ERROR) << "setStorageParams failed, invalid storageId: " << storageId;
         return -EINVAL;
     }
+    return setStorageParams(*ifs, storageId, enableReadLogs);
+}
 
-    const auto& params = ifs->dataLoaderStub->params();
+int IncrementalService::setStorageParams(IncFsMount& ifs, StorageId storageId,
+                                         bool enableReadLogs) {
+    const auto& params = ifs.dataLoaderStub->params();
     if (enableReadLogs) {
-        if (!ifs->readLogsAllowed()) {
-            LOG(ERROR) << "setStorageParams failed, readlogs disabled for storageId: " << storageId;
+        if (!ifs.readLogsAllowed()) {
+            LOG(ERROR) << "setStorageParams failed, readlogs disallowed for storageId: "
+                       << storageId;
             return -EPERM;
         }
 
@@ -760,9 +802,19 @@ int IncrementalService::setStorageParams(StorageId storageId, bool enableReadLog
                        << " check failed: " << status.toString8();
             return fromBinderStatus(status);
         }
+
+        // Check installation time.
+        const auto now = mClock->now();
+        const auto startLoadingTs = ifs.startLoadingTs;
+        if (startLoadingTs <= now && now - startLoadingTs > Constants::readLogsMaxInterval) {
+            LOG(ERROR) << "setStorageParams failed, readlogs can't be enabled at this time, "
+                          "storageId: "
+                       << storageId;
+            return -EPERM;
+        }
     }
 
-    if (auto status = applyStorageParams(*ifs, enableReadLogs); !status.isOk()) {
+    if (auto status = applyStorageParams(ifs, enableReadLogs); !status.isOk()) {
         LOG(ERROR) << "applyStorageParams failed: " << status.toString8();
         return fromBinderStatus(status);
     }
@@ -2222,6 +2274,10 @@ sp<content::pm::IDataLoader> IncrementalService::DataLoaderStub::getDataLoader()
     return dataloader;
 }
 
+bool IncrementalService::DataLoaderStub::isSystemDataLoader() const {
+    return (params().packageName == Constants::systemPackage);
+}
+
 bool IncrementalService::DataLoaderStub::requestCreate() {
     return setTargetStatus(IDataLoaderStatusListener::DATA_LOADER_CREATED);
 }
diff --git a/services/incremental/IncrementalService.h b/services/incremental/IncrementalService.h
index 4eb513808342..bc441c792084 100644
--- a/services/incremental/IncrementalService.h
+++ b/services/incremental/IncrementalService.h
@@ -231,6 +231,7 @@ private:
 
         MountId id() const { return mId.load(std::memory_order_relaxed); }
         const content::pm::DataLoaderParamsParcel& params() const { return mParams; }
+        bool isSystemDataLoader() const;
         void setHealthListener(StorageHealthCheckParams&& healthCheckParams,
                                const StorageHealthListener* healthListener);
         long elapsedMsSinceOldestPendingRead();
@@ -330,6 +331,7 @@ private:
         StorageMap storages;
         BindMap bindPoints;
         DataLoaderStubPtr dataLoaderStub;
+        TimePoint startLoadingTs = {};
         std::atomic<int> nextStorageDirNo{0};
         const IncrementalService& incrementalService;
 
@@ -348,12 +350,7 @@ private:
         void disallowReadLogs() { flags &= ~StorageFlags::ReadLogsAllowed; }
         int32_t readLogsAllowed() const { return (flags & StorageFlags::ReadLogsAllowed); }
 
-        void setReadLogsEnabled(bool value) {
-            if (value)
-                flags |= StorageFlags::ReadLogsEnabled;
-            else
-                flags &= ~StorageFlags::ReadLogsEnabled;
-        }
+        void setReadLogsEnabled(bool value);
         int32_t readLogsEnabled() const { return (flags & StorageFlags::ReadLogsEnabled); }
 
         static void cleanupFilesystem(std::string_view root);
@@ -411,6 +408,8 @@ private:
                                              IncFsMount::StorageMap::const_iterator storageIt,
                                              std::string_view path) const;
     int makeDirs(const IncFsMount& ifs, StorageId storageId, std::string_view path, int mode);
+
+    int setStorageParams(IncFsMount& ifs, StorageId storageId, bool enableReadLogs);
     binder::Status applyStorageParams(IncFsMount& ifs, bool enableReadLogs);
 
     int isFileFullyLoadedFromPath(const IncFsMount& ifs, std::string_view filePath) const;
diff --git a/services/incremental/test/IncrementalServiceTest.cpp b/services/incremental/test/IncrementalServiceTest.cpp
index 25b34b5669b8..bf798273a8a9 100644
--- a/services/incremental/test/IncrementalServiceTest.cpp
+++ b/services/incremental/test/IncrementalServiceTest.cpp
@@ -908,7 +908,7 @@ TEST_F(IncrementalServiceTest, testDataLoaderOnRestart) {
     EXPECT_CALL(*mDataLoader, start(_)).Times(6);
     EXPECT_CALL(*mDataLoader, destroy(_)).Times(1);
     EXPECT_CALL(*mVold, unmountIncFs(_)).Times(2);
-    EXPECT_CALL(*mTimedQueue, addJob(_, _, _)).Times(2);
+    EXPECT_CALL(*mTimedQueue, addJob(_, _, _)).Times(3);
     TemporaryDir tempDir;
     int storageId =
             mIncrementalService->createStorage(tempDir.path, mDataLoaderParcel,
@@ -1119,7 +1119,7 @@ TEST_F(IncrementalServiceTest, testStartDataLoaderUnhealthyStorage) {
     EXPECT_CALL(*mVold, unmountIncFs(_)).Times(2);
     EXPECT_CALL(*mLooper, addFd(MockIncFs::kPendingReadsFd, _, _, _, _)).Times(2);
     EXPECT_CALL(*mLooper, removeFd(MockIncFs::kPendingReadsFd)).Times(2);
-    EXPECT_CALL(*mTimedQueue, addJob(_, _, _)).Times(4);
+    EXPECT_CALL(*mTimedQueue, addJob(_, _, _)).Times(5);
 
     sp<NiceMock<MockStorageHealthListener>> listener{new NiceMock<MockStorageHealthListener>};
     NiceMock<MockStorageHealthListener>* listenerMock = listener.get();
@@ -1292,6 +1292,147 @@ TEST_F(IncrementalServiceTest, testSetIncFsMountOptionsSuccessAndDisabled) {
     ASSERT_EQ(mDataLoader->setStorageParams(true), -EPERM);
 }
 
+TEST_F(IncrementalServiceTest, testSetIncFsMountOptionsSuccessAndTimedOut) {
+    mVold->setIncFsMountOptionsSuccess();
+    mAppOpsManager->checkPermissionSuccess();
+
+    const auto readLogsMaxInterval = 2h;
+
+    EXPECT_CALL(*mDataLoaderManager, unbindFromDataLoader(_));
+    EXPECT_CALL(*mVold, unmountIncFs(_)).Times(2);
+    // Enabling and then disabling readlogs.
+    EXPECT_CALL(*mVold, setIncFsMountOptions(_, true)).Times(2);
+    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);
+    EXPECT_CALL(*mTimedQueue, addJob(_, _, _)).Times(1);
+    TemporaryDir tempDir;
+    int storageId =
+            mIncrementalService->createStorage(tempDir.path, mDataLoaderParcel,
+                                               IncrementalService::CreateOptions::CreateNew);
+    ASSERT_GE(storageId, 0);
+    ASSERT_TRUE(mIncrementalService->startLoading(storageId, std::move(mDataLoaderParcel), {}, {},
+                                                  {}, {}));
+
+    // Disable readlogs callback present.
+    ASSERT_EQ(storageId, mTimedQueue->mId);
+    ASSERT_EQ(mTimedQueue->mAfter, readLogsMaxInterval);
+    auto callback = mTimedQueue->mWhat;
+    mTimedQueue->clearJob(storageId);
+
+    ASSERT_GE(mDataLoader->setStorageParams(true), 0);
+    // Now advance clock for 1hr.
+    mClock->advance(1h);
+    ASSERT_GE(mDataLoader->setStorageParams(true), 0);
+    // Now call the timed callback, it should turn off the readlogs.
+    callback();
+    // Now advance clock for 2hrs.
+    mClock->advance(readLogsMaxInterval);
+    ASSERT_EQ(mDataLoader->setStorageParams(true), -EPERM);
+}
+
+TEST_F(IncrementalServiceTest, testSetIncFsMountOptionsSuccessAndNoTimedOutForSystem) {
+    mVold->setIncFsMountOptionsSuccess();
+    mAppOpsManager->checkPermissionSuccess();
+
+    const auto readLogsMaxInterval = 2h;
+
+    EXPECT_CALL(*mDataLoaderManager, unbindFromDataLoader(_));
+    EXPECT_CALL(*mVold, unmountIncFs(_)).Times(2);
+    // Enabling and then disabling readlogs.
+    EXPECT_CALL(*mVold, setIncFsMountOptions(_, true)).Times(3);
+    EXPECT_CALL(*mVold, setIncFsMountOptions(_, false)).Times(0);
+    // After setIncFsMountOptions succeeded expecting to start watching.
+    EXPECT_CALL(*mAppOpsManager, startWatchingMode(_, _, _)).Times(1);
+    // Not expecting callback removal.
+    EXPECT_CALL(*mAppOpsManager, stopWatchingMode(_)).Times(0);
+    EXPECT_CALL(*mTimedQueue, addJob(_, _, _)).Times(0);
+    // System data loader.
+    mDataLoaderParcel.packageName = "android";
+    TemporaryDir tempDir;
+    int storageId =
+            mIncrementalService->createStorage(tempDir.path, mDataLoaderParcel,
+                                               IncrementalService::CreateOptions::CreateNew);
+    ASSERT_GE(storageId, 0);
+    ASSERT_TRUE(mIncrementalService->startLoading(storageId, std::move(mDataLoaderParcel), {}, {},
+                                                  {}, {}));
+
+    // No readlogs callback.
+    ASSERT_EQ(mTimedQueue->mAfter, 0ms);
+    ASSERT_EQ(mTimedQueue->mWhat, nullptr);
+
+    ASSERT_GE(mDataLoader->setStorageParams(true), 0);
+    // Now advance clock for 1hr.
+    mClock->advance(1h);
+    ASSERT_GE(mDataLoader->setStorageParams(true), 0);
+    // Now advance clock for 2hrs.
+    mClock->advance(readLogsMaxInterval);
+    ASSERT_EQ(mDataLoader->setStorageParams(true), 0);
+}
+
+TEST_F(IncrementalServiceTest, testSetIncFsMountOptionsSuccessAndNewInstall) {
+    mVold->setIncFsMountOptionsSuccess();
+    mAppOpsManager->checkPermissionSuccess();
+
+    const auto readLogsMaxInterval = 2h;
+
+    EXPECT_CALL(*mDataLoaderManager, unbindFromDataLoader(_)).Times(2);
+    EXPECT_CALL(*mVold, unmountIncFs(_)).Times(2);
+    // Enabling and then disabling readlogs.
+    EXPECT_CALL(*mVold, setIncFsMountOptions(_, true)).Times(3);
+    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);
+    EXPECT_CALL(*mTimedQueue, addJob(_, _, _)).Times(2);
+    TemporaryDir tempDir;
+    int storageId =
+            mIncrementalService->createStorage(tempDir.path, mDataLoaderParcel,
+                                               IncrementalService::CreateOptions::CreateNew);
+    ASSERT_GE(storageId, 0);
+
+    auto dataLoaderParcel = mDataLoaderParcel;
+    ASSERT_TRUE(mIncrementalService->startLoading(storageId, std::move(dataLoaderParcel), {}, {},
+                                                  {}, {}));
+
+    // Disable readlogs callback present.
+    ASSERT_EQ(storageId, mTimedQueue->mId);
+    ASSERT_EQ(mTimedQueue->mAfter, readLogsMaxInterval);
+    auto callback = mTimedQueue->mWhat;
+    mTimedQueue->clearJob(storageId);
+
+    ASSERT_GE(mDataLoader->setStorageParams(true), 0);
+    // Now advance clock for 1.5hrs.
+    mClock->advance(90min);
+    ASSERT_GE(mDataLoader->setStorageParams(true), 0);
+
+    // New installation.
+    ASSERT_TRUE(mIncrementalService->startLoading(storageId, std::move(mDataLoaderParcel), {}, {},
+                                                  {}, {}));
+
+    // New callback present.
+    ASSERT_EQ(storageId, mTimedQueue->mId);
+    ASSERT_EQ(mTimedQueue->mAfter, readLogsMaxInterval);
+    auto callback2 = mTimedQueue->mWhat;
+    mTimedQueue->clearJob(storageId);
+
+    // Old callback should not disable readlogs (setIncFsMountOptions should be called only once).
+    callback();
+    // Advance clock for another 1.5hrs.
+    mClock->advance(90min);
+    // Still success even it's 3hrs past first install.
+    ASSERT_GE(mDataLoader->setStorageParams(true), 0);
+
+    // New one should disable.
+    callback2();
+    // And timeout.
+    mClock->advance(90min);
+    ASSERT_EQ(mDataLoader->setStorageParams(true), -EPERM);
+}
+
 TEST_F(IncrementalServiceTest, testSetIncFsMountOptionsSuccessAndPermissionChanged) {
     mVold->setIncFsMountOptionsSuccess();
     mAppOpsManager->checkPermissionSuccess();
@@ -1675,7 +1816,7 @@ TEST_F(IncrementalServiceTest, testPerUidTimeoutsTooShort) {
     EXPECT_CALL(*mDataLoader, start(_)).Times(1);
     EXPECT_CALL(*mDataLoader, destroy(_)).Times(1);
     EXPECT_CALL(*mIncFs, setUidReadTimeouts(_, _)).Times(0);
-    EXPECT_CALL(*mTimedQueue, addJob(_, _, _)).Times(0);
+    EXPECT_CALL(*mTimedQueue, addJob(_, _, _)).Times(1);
     EXPECT_CALL(*mVold, unmountIncFs(_)).Times(2);
     TemporaryDir tempDir;
     int storageId =
@@ -1702,6 +1843,9 @@ TEST_F(IncrementalServiceTest, testPerUidTimeoutsSuccess) {
     // Empty storage.
     mIncFs->countFilledBlocksEmpty();
 
+    // Mark DataLoader as 'system' so that readlogs don't pollute the timed queue.
+    mDataLoaderParcel.packageName = "android";
+
     TemporaryDir tempDir;
     int storageId =
             mIncrementalService->createStorage(tempDir.path, mDataLoaderParcel,