Appops permission monitoring for GET_USAGE_STATS.

This makes sure DataLoader won't be able to obtain read logs once user
denies access.

Bug: b/152633648
Test: atest PackageManagerShellCommandTest PackageManagerShellCommandIncrementalTest IncrementalServiceTest
Test: adb shell appops set 1000 GET_USAGE_STATS deny
Change-Id: Ibbb74933b4ef0dd8f5fe27732743e5820b8ee4dc

7 files changed, 181 insertions, 74 deletions

diff --git a/PREUPLOAD.cfg b/PREUPLOAD.cfg
index 9abb308534df..2fd2e33bbc37 100644
--- a/PREUPLOAD.cfg
+++ b/PREUPLOAD.cfg
@@ -9,6 +9,7 @@ clang_format = --commit ${PREUPLOAD_COMMIT} --style file --extensions c,h,cc,cpp
                core/jni/
                libs/input/
                services/core/jni/
+               services/incremental/
 
 [Hook Scripts]
 checkstyle_hook = ${REPO_ROOT}/prebuilts/checkstyle/checkstyle.py --sha ${PREUPLOAD_COMMIT}
diff --git a/services/incremental/Android.bp b/services/incremental/Android.bp
index 02bb0bc3e49c..b13d33054e19 100644
--- a/services/incremental/Android.bp
+++ b/services/incremental/Android.bp
@@ -50,7 +50,6 @@ cc_defaults {
         "libbinder",
         "libcrypto",
         "libcutils",
-        "libdataloader",
         "libincfs",
         "liblog",
         "libz",
diff --git a/services/incremental/IncrementalService.cpp b/services/incremental/IncrementalService.cpp
index 0da167303ccd..d36eae89c1ff 100644
--- a/services/incremental/IncrementalService.cpp
+++ b/services/incremental/IncrementalService.cpp
@@ -17,7 +17,6 @@
 #define LOG_TAG "IncrementalService"
 
 #include "IncrementalService.h"
-#include "IncrementalServiceValidation.h"
 
 #include <android-base/file.h>
 #include <android-base/logging.h>
@@ -582,25 +581,29 @@ int IncrementalService::setStorageParams(StorageId storageId, bool enableReadLog
         return -EINVAL;
     }
 
-    ifs->dataLoaderFilesystemParams.readLogsEnabled = enableReadLogs;
     if (enableReadLogs) {
-        // We never unregister the callbacks, but given a restricted number of data loaders and even fewer asking for read log access, should be ok.
-        registerAppOpsCallback(ifs->dataLoaderParams.packageName);
+        if (auto status =
+                    mAppOpsManager->checkPermission(kDataUsageStats, kOpUsage,
+                                                    ifs->dataLoaderParams.packageName.c_str());
+            !status.isOk()) {
+            LOG(ERROR) << "checkPermission failed: " << status.toString8();
+            return fromBinderStatus(status);
+        }
     }
 
-    return applyStorageParams(*ifs);
-}
+    if (auto status = applyStorageParams(*ifs, enableReadLogs); !status.isOk()) {
+        LOG(ERROR) << "applyStorageParams failed: " << status.toString8();
+        return fromBinderStatus(status);
+    }
 
-int IncrementalService::applyStorageParams(IncFsMount& ifs) {
-    const bool enableReadLogs = ifs.dataLoaderFilesystemParams.readLogsEnabled;
     if (enableReadLogs) {
-        if (auto status = CheckPermissionForDataDelivery(kDataUsageStats, kOpUsage);
-            !status.isOk()) {
-            LOG(ERROR) << "CheckPermissionForDataDelivery failed: " << status.toString8();
-            return fromBinderStatus(status);
-        }
+        registerAppOpsCallback(ifs->dataLoaderParams.packageName);
     }
 
+    return 0;
+}
+
+binder::Status IncrementalService::applyStorageParams(IncFsMount& ifs, bool enableReadLogs) {
     using unique_fd = ::android::base::unique_fd;
     ::android::os::incremental::IncrementalFileSystemControlParcel control;
     control.cmd.reset(unique_fd(dup(ifs.control.cmd())));
@@ -611,13 +614,7 @@ int IncrementalService::applyStorageParams(IncFsMount& ifs) {
     }
 
     std::lock_guard l(mMountOperationLock);
-    const auto status = mVold->setIncFsMountOptions(control, enableReadLogs);
-    if (!status.isOk()) {
-        LOG(ERROR) << "Calling Vold::setIncFsMountOptions() failed: " << status.toString8();
-        return fromBinderStatus(status);
-    }
-
-    return 0;
+    return mVold->setIncFsMountOptions(control, enableReadLogs);
 }
 
 void IncrementalService::deleteStorage(StorageId storageId) {
@@ -1280,39 +1277,54 @@ bool IncrementalService::configureNativeBinaries(StorageId storage, std::string_
 }
 
 void IncrementalService::registerAppOpsCallback(const std::string& packageName) {
-    if (packageName.empty()) {
-        return;
-    }
-
+    sp<IAppOpsCallback> listener;
     {
         std::unique_lock lock{mCallbacksLock};
-        if (!mCallbackRegistered.insert(packageName).second) {
+        auto& cb = mCallbackRegistered[packageName];
+        if (cb) {
             return;
         }
+        cb = new AppOpsListener(*this, packageName);
+        listener = cb;
     }
 
-    /* TODO(b/152633648): restore callback after it's not crashing Binder anymore.
-    sp<AppOpsListener> listener = new AppOpsListener(*this, packageName);
     mAppOpsManager->startWatchingMode(AppOpsManager::OP_GET_USAGE_STATS, String16(packageName.c_str()), listener);
-    */
 }
 
-void IncrementalService::onAppOppChanged(const std::string& packageName) {
+bool IncrementalService::unregisterAppOpsCallback(const std::string& packageName) {
+    sp<IAppOpsCallback> listener;
+    {
+        std::unique_lock lock{mCallbacksLock};
+        auto found = mCallbackRegistered.find(packageName);
+        if (found == mCallbackRegistered.end()) {
+            return false;
+        }
+        listener = found->second;
+        mCallbackRegistered.erase(found);
+    }
+
+    mAppOpsManager->stopWatchingMode(listener);
+    return true;
+}
+
+void IncrementalService::onAppOpChanged(const std::string& packageName) {
+    if (!unregisterAppOpsCallback(packageName)) {
+        return;
+    }
+
     std::vector<IfsMountPtr> affected;
     {
         std::lock_guard l(mLock);
         affected.reserve(mMounts.size());
         for (auto&& [id, ifs] : mMounts) {
-            if (ifs->dataLoaderFilesystemParams.readLogsEnabled && ifs->dataLoaderParams.packageName == packageName) {
+            if (ifs->mountId == id && ifs->dataLoaderParams.packageName == packageName) {
                 affected.push_back(ifs);
             }
         }
     }
-    /* TODO(b/152633648): restore callback after it's not crashing Kernel anymore.
     for (auto&& ifs : affected) {
-        applyStorageParams(*ifs);
+        applyStorageParams(*ifs, false);
     }
-    */
 }
 
 binder::Status IncrementalService::IncrementalDataLoaderListener::onStatusChanged(MountId mountId,
@@ -1378,8 +1390,8 @@ binder::Status IncrementalService::IncrementalDataLoaderListener::onStatusChange
     return binder::Status::ok();
 }
 
-void IncrementalService::AppOpsListener::opChanged(int32_t op, const String16&) {
-    incrementalService.onAppOppChanged(packageName);
+void IncrementalService::AppOpsListener::opChanged(int32_t, const String16&) {
+    incrementalService.onAppOpChanged(packageName);
 }
 
 } // namespace android::incremental
diff --git a/services/incremental/IncrementalService.h b/services/incremental/IncrementalService.h
index ff69633e185b..58002974e180 100644
--- a/services/incremental/IncrementalService.h
+++ b/services/incremental/IncrementalService.h
@@ -40,7 +40,6 @@
 #include "ServiceWrappers.h"
 #include "android/content/pm/BnDataLoaderStatusListener.h"
 #include "incfs.h"
-#include "dataloader_ndk.h"
 #include "path.h"
 
 using namespace android::os::incremental;
@@ -182,7 +181,6 @@ private:
         StorageMap storages;
         BindMap bindPoints;
         DataLoaderParamsParcel dataLoaderParams;
-        DataLoaderFilesystemParams dataLoaderFilesystemParams;
         std::atomic<int> nextStorageDirNo{0};
         std::atomic<int> dataLoaderStatus = -1;
         bool dataLoaderStartRequested = false;
@@ -193,9 +191,7 @@ private:
               : root(std::move(root)),
                 control(std::move(control)),
                 mountId(mountId),
-                incrementalService(incrementalService) {
-            dataLoaderFilesystemParams.readLogsEnabled = false;
-        }
+                incrementalService(incrementalService) {}
         IncFsMount(IncFsMount&&) = delete;
         IncFsMount& operator=(IncFsMount&&) = delete;
         ~IncFsMount();
@@ -234,10 +230,11 @@ private:
     std::string normalizePathToStorage(const IfsMountPtr incfs, StorageId storage,
                                        std::string_view path);
 
-    int applyStorageParams(IncFsMount& ifs);
+    binder::Status applyStorageParams(IncFsMount& ifs, bool enableReadLogs);
 
     void registerAppOpsCallback(const std::string& packageName);
-    void onAppOppChanged(const std::string& packageName);
+    bool unregisterAppOpsCallback(const std::string& packageName);
+    void onAppOpChanged(const std::string& packageName);
 
     // Member variables
     std::unique_ptr<VoldServiceWrapper> const mVold;
@@ -252,7 +249,7 @@ private:
     BindPathMap mBindsByPath;
 
     std::mutex mCallbacksLock;
-    std::set<std::string> mCallbackRegistered;
+    std::map<std::string, sp<AppOpsListener>> mCallbackRegistered;
 
     std::atomic_bool mSystemReady = false;
     StorageId mNextId = 0;
diff --git a/services/incremental/IncrementalServiceValidation.h b/services/incremental/IncrementalServiceValidation.h
index 24f9f7f94dfd..48894c6926c8 100644
--- a/services/incremental/IncrementalServiceValidation.h
+++ b/services/incremental/IncrementalServiceValidation.h
@@ -41,7 +41,8 @@ inline int fromBinderStatus(const binder::Status& status) {
             : -EIO;
 }
 
-inline binder::Status CheckPermissionForDataDelivery(const char* permission, const char* operation) {
+inline binder::Status CheckPermissionForDataDelivery(const char* permission, const char* operation,
+                                                     const char* package) {
     using android::base::StringPrintf;
 
     int32_t pid;
@@ -52,23 +53,23 @@ inline binder::Status CheckPermissionForDataDelivery(const char* permission, con
                          StringPrintf("UID %d / PID %d lacks permission %s", uid, pid, permission));
     }
 
+    String16 packageName{package};
+
     // Caller must also have op granted.
     PermissionController pc;
-    // Package is a required parameter. Need to obtain one.
-    Vector<String16> packages;
-    pc.getPackagesForUid(uid, packages);
-    if (packages.empty()) {
+    if (auto packageUid = pc.getPackageUid(packageName, 0); packageUid != uid) {
         return Exception(binder::Status::EX_SECURITY,
-                         StringPrintf("UID %d / PID %d has no packages", uid, pid));
+                         StringPrintf("UID %d / PID %d does not own package %s", uid, pid,
+                                      package));
     }
-    switch (auto result = pc.noteOp(String16(operation), uid, packages[0]); result) {
+    switch (auto result = pc.noteOp(String16(operation), uid, packageName); result) {
         case PermissionController::MODE_ALLOWED:
         case PermissionController::MODE_DEFAULT:
             return binder::Status::ok();
         default:
             return Exception(binder::Status::EX_SECURITY,
-                             StringPrintf("UID %d / PID %d lacks app-op %s, error %d", uid, pid,
-                                          operation, result));
+                             StringPrintf("UID %d / PID %d / package %s lacks app-op %s, error %d",
+                                          uid, pid, package, operation, result));
     }
 }
 
diff --git a/services/incremental/ServiceWrappers.h b/services/incremental/ServiceWrappers.h
index 449b457045c6..84bf1ffaf45c 100644
--- a/services/incremental/ServiceWrappers.h
+++ b/services/incremental/ServiceWrappers.h
@@ -16,6 +16,8 @@
 
 #pragma once
 
+#include "IncrementalServiceValidation.h"
+
 #include <android-base/strings.h>
 #include <android-base/unique_fd.h>
 #include <android/content/pm/DataLoaderParamsParcel.h>
@@ -85,7 +87,10 @@ public:
 class AppOpsManagerWrapper {
 public:
     virtual ~AppOpsManagerWrapper() = default;
+    virtual binder::Status checkPermission(const char* permission, const char* operation,
+                                           const char* package) const = 0;
     virtual void startWatchingMode(int32_t op, const String16& packageName, const sp<IAppOpsCallback>& callback) = 0;
+    virtual void stopWatchingMode(const sp<IAppOpsCallback>& callback) = 0;
 };
 
 class ServiceManagerWrapper {
@@ -105,17 +110,19 @@ public:
     ~RealVoldService() = default;
     binder::Status mountIncFs(const std::string& backingPath, const std::string& targetDir,
                               int32_t flags,
-                              IncrementalFileSystemControlParcel* _aidl_return) const override {
+                              IncrementalFileSystemControlParcel* _aidl_return) const final {
         return mInterface->mountIncFs(backingPath, targetDir, flags, _aidl_return);
     }
-    binder::Status unmountIncFs(const std::string& dir) const override {
+    binder::Status unmountIncFs(const std::string& dir) const final {
         return mInterface->unmountIncFs(dir);
     }
     binder::Status bindMount(const std::string& sourceDir,
-                             const std::string& targetDir) const override {
+                             const std::string& targetDir) const final {
         return mInterface->bindMount(sourceDir, targetDir);
     }
-    binder::Status setIncFsMountOptions(const ::android::os::incremental::IncrementalFileSystemControlParcel& control, bool enableReadLogs) const override {
+    binder::Status setIncFsMountOptions(
+            const ::android::os::incremental::IncrementalFileSystemControlParcel& control,
+            bool enableReadLogs) const final {
         return mInterface->setIncFsMountOptions(control, enableReadLogs);
     }
 
@@ -131,13 +138,13 @@ public:
     binder::Status initializeDataLoader(MountId mountId, const DataLoaderParamsParcel& params,
                                         const FileSystemControlParcel& control,
                                         const sp<IDataLoaderStatusListener>& listener,
-                                        bool* _aidl_return) const override {
+                                        bool* _aidl_return) const final {
         return mInterface->initializeDataLoader(mountId, params, control, listener, _aidl_return);
     }
-    binder::Status getDataLoader(MountId mountId, sp<IDataLoader>* _aidl_return) const override {
+    binder::Status getDataLoader(MountId mountId, sp<IDataLoader>* _aidl_return) const final {
         return mInterface->getDataLoader(mountId, _aidl_return);
     }
-    binder::Status destroyDataLoader(MountId mountId) const override {
+    binder::Status destroyDataLoader(MountId mountId) const final {
         return mInterface->destroyDataLoader(mountId);
     }
 
@@ -148,9 +155,18 @@ private:
 class RealAppOpsManager : public AppOpsManagerWrapper {
 public:
     ~RealAppOpsManager() = default;
-    void startWatchingMode(int32_t op, const String16& packageName, const sp<IAppOpsCallback>& callback) override {
+    binder::Status checkPermission(const char* permission, const char* operation,
+                                   const char* package) const final {
+        return android::incremental::CheckPermissionForDataDelivery(permission, operation, package);
+    }
+    void startWatchingMode(int32_t op, const String16& packageName,
+                           const sp<IAppOpsCallback>& callback) final {
         mAppOpsManager.startWatchingMode(op, packageName, callback);
     }
+    void stopWatchingMode(const sp<IAppOpsCallback>& callback) final {
+        mAppOpsManager.stopWatchingMode(callback);
+    }
+
 private:
     android::AppOpsManager mAppOpsManager;
 };
@@ -174,36 +190,35 @@ class RealIncFs : public IncFsWrapper {
 public:
     RealIncFs() = default;
     ~RealIncFs() = default;
-    Control createControl(IncFsFd cmd, IncFsFd pendingReads, IncFsFd logs) const override {
+    Control createControl(IncFsFd cmd, IncFsFd pendingReads, IncFsFd logs) const final {
         return incfs::createControl(cmd, pendingReads, logs);
     }
     ErrorCode makeFile(const Control& control, std::string_view path, int mode, FileId id,
-                       NewFileParams params) const override {
+                       NewFileParams params) const final {
         return incfs::makeFile(control, path, mode, id, params);
     }
-    ErrorCode makeDir(const Control& control, std::string_view path, int mode) const override {
+    ErrorCode makeDir(const Control& control, std::string_view path, int mode) const final {
         return incfs::makeDir(control, path, mode);
     }
-    RawMetadata getMetadata(const Control& control, FileId fileid) const override {
+    RawMetadata getMetadata(const Control& control, FileId fileid) const final {
         return incfs::getMetadata(control, fileid);
     }
-    RawMetadata getMetadata(const Control& control, std::string_view path) const override {
+    RawMetadata getMetadata(const Control& control, std::string_view path) const final {
         return incfs::getMetadata(control, path);
     }
-    FileId getFileId(const Control& control, std::string_view path) const override {
+    FileId getFileId(const Control& control, std::string_view path) const final {
         return incfs::getFileId(control, path);
     }
-    ErrorCode link(const Control& control, std::string_view from,
-                   std::string_view to) const override {
+    ErrorCode link(const Control& control, std::string_view from, std::string_view to) const final {
         return incfs::link(control, from, to);
     }
-    ErrorCode unlink(const Control& control, std::string_view path) const override {
+    ErrorCode unlink(const Control& control, std::string_view path) const final {
         return incfs::unlink(control, path);
     }
-    base::unique_fd openForSpecialOps(const Control& control, FileId id) const override {
+    base::unique_fd openForSpecialOps(const Control& control, FileId id) const final {
         return base::unique_fd{incfs::openForSpecialOps(control, id).release()};
     }
-    ErrorCode writeBlocks(Span<const DataBlock> blocks) const override {
+    ErrorCode writeBlocks(Span<const DataBlock> blocks) const final {
         return incfs::writeBlocks(blocks);
     }
 };
diff --git a/services/incremental/test/IncrementalServiceTest.cpp b/services/incremental/test/IncrementalServiceTest.cpp
index 5553f688060a..0635ae169281 100644
--- a/services/incremental/test/IncrementalServiceTest.cpp
+++ b/services/incremental/test/IncrementalServiceTest.cpp
@@ -221,7 +221,28 @@ public:
 };
 
 class MockAppOpsManager : public AppOpsManagerWrapper {
+public:
+    MOCK_CONST_METHOD3(checkPermission, binder::Status(const char*, const char*, const char*));
     MOCK_METHOD3(startWatchingMode, void(int32_t, const String16&, const sp<IAppOpsCallback>&));
+    MOCK_METHOD1(stopWatchingMode, void(const sp<IAppOpsCallback>&));
+
+    void checkPermissionSuccess() {
+        ON_CALL(*this, checkPermission(_, _, _)).WillByDefault(Return(android::incremental::Ok()));
+    }
+    void checkPermissionFails() {
+        ON_CALL(*this, checkPermission(_, _, _))
+                .WillByDefault(
+                        Return(android::incremental::Exception(binder::Status::EX_SECURITY, {})));
+    }
+    void initializeStartWatchingMode() {
+        ON_CALL(*this, startWatchingMode(_, _, _))
+                .WillByDefault(Invoke(this, &MockAppOpsManager::storeCallback));
+    }
+    void storeCallback(int32_t, const String16&, const sp<IAppOpsCallback>& cb) {
+        mStoredCallback = cb;
+    }
+
+    sp<IAppOpsCallback> mStoredCallback;
 };
 
 class MockServiceManager : public ServiceManagerWrapper {
@@ -418,9 +439,15 @@ TEST_F(IncrementalServiceTest, testSetIncFsMountOptionsSuccess) {
     mVold->setIncFsMountOptionsSuccess();
     mDataLoaderManager->initializeDataLoaderSuccess();
     mDataLoaderManager->getDataLoaderSuccess();
+    mAppOpsManager->checkPermissionSuccess();
     EXPECT_CALL(*mDataLoaderManager, destroyDataLoader(_));
     EXPECT_CALL(*mVold, unmountIncFs(_)).Times(2);
-    EXPECT_CALL(*mVold, setIncFsMountOptions(_, _));
+    // We are calling setIncFsMountOptions(true).
+    EXPECT_CALL(*mVold, setIncFsMountOptions(_, true)).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), {},
@@ -429,6 +456,56 @@ TEST_F(IncrementalServiceTest, testSetIncFsMountOptionsSuccess) {
     ASSERT_GE(mIncrementalService->setStorageParams(storageId, true), 0);
 }
 
+TEST_F(IncrementalServiceTest, testSetIncFsMountOptionsSuccessAndPermissionChanged) {
+    mVold->mountIncFsSuccess();
+    mIncFs->makeFileSuccess();
+    mVold->bindMountSuccess();
+    mVold->setIncFsMountOptionsSuccess();
+    mDataLoaderManager->initializeDataLoaderSuccess();
+    mDataLoaderManager->getDataLoaderSuccess();
+    mAppOpsManager->checkPermissionSuccess();
+    mAppOpsManager->initializeStartWatchingMode();
+    EXPECT_CALL(*mDataLoaderManager, destroyDataLoader(_));
+    EXPECT_CALL(*mVold, unmountIncFs(_)).Times(2);
+    // We are calling setIncFsMountOptions(true).
+    EXPECT_CALL(*mVold, setIncFsMountOptions(_, true)).Times(1);
+    // setIncFsMountOptions(false) is called on the callback.
+    EXPECT_CALL(*mVold, setIncFsMountOptions(_, false)).Times(1);
+    // After setIncFsMountOptions succeeded expecting to start watching.
+    EXPECT_CALL(*mAppOpsManager, startWatchingMode(_, _, _)).Times(1);
+    // After callback is called, disable read logs and remove callback.
+    EXPECT_CALL(*mAppOpsManager, stopWatchingMode(_)).Times(1);
+    TemporaryDir tempDir;
+    int storageId =
+            mIncrementalService->createStorage(tempDir.path, std::move(mDataLoaderParcel), {},
+                                               IncrementalService::CreateOptions::CreateNew);
+    ASSERT_GE(storageId, 0);
+    ASSERT_GE(mIncrementalService->setStorageParams(storageId, true), 0);
+    ASSERT_NE(nullptr, mAppOpsManager->mStoredCallback.get());
+    mAppOpsManager->mStoredCallback->opChanged(0, {});
+}
+
+TEST_F(IncrementalServiceTest, testSetIncFsMountOptionsCheckPermissionFails) {
+    mVold->mountIncFsSuccess();
+    mIncFs->makeFileSuccess();
+    mVold->bindMountSuccess();
+    mDataLoaderManager->initializeDataLoaderSuccess();
+    mDataLoaderManager->getDataLoaderSuccess();
+    mAppOpsManager->checkPermissionFails();
+    EXPECT_CALL(*mDataLoaderManager, destroyDataLoader(_));
+    EXPECT_CALL(*mVold, unmountIncFs(_)).Times(2);
+    // checkPermission fails, no calls to set opitions,  start or stop WatchingMode.
+    EXPECT_CALL(*mVold, setIncFsMountOptions(_, true)).Times(0);
+    EXPECT_CALL(*mAppOpsManager, startWatchingMode(_, _, _)).Times(0);
+    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_LT(mIncrementalService->setStorageParams(storageId, true), 0);
+}
+
 TEST_F(IncrementalServiceTest, testSetIncFsMountOptionsFails) {
     mVold->mountIncFsSuccess();
     mIncFs->makeFileSuccess();
@@ -436,9 +513,14 @@ TEST_F(IncrementalServiceTest, testSetIncFsMountOptionsFails) {
     mVold->setIncFsMountOptionsFails();
     mDataLoaderManager->initializeDataLoaderSuccess();
     mDataLoaderManager->getDataLoaderSuccess();
+    mAppOpsManager->checkPermissionSuccess();
     EXPECT_CALL(*mDataLoaderManager, destroyDataLoader(_));
     EXPECT_CALL(*mVold, unmountIncFs(_)).Times(2);
-    EXPECT_CALL(*mVold, setIncFsMountOptions(_, _));
+    // We are calling setIncFsMountOptions.
+    EXPECT_CALL(*mVold, setIncFsMountOptions(_, true)).Times(1);
+    // setIncFsMountOptions fails, no calls to start or stop WatchingMode.
+    EXPECT_CALL(*mAppOpsManager, startWatchingMode(_, _, _)).Times(0);
+    EXPECT_CALL(*mAppOpsManager, stopWatchingMode(_)).Times(0);
     TemporaryDir tempDir;
     int storageId =
             mIncrementalService->createStorage(tempDir.path, std::move(mDataLoaderParcel), {},