Checking LOADER_USAGE_STATS before enabling read logs.

Bug: b/152633648
Test: atest PackageManagerShellCommandTest PackageManagerShellCommandIncrementalTest IncrementalServiceTest
Change-Id: Ic747a51b97b785c627c95bddecc6834ef602ff30

10 files changed, 139 insertions, 0 deletions

diff --git a/core/java/android/os/incremental/IIncrementalService.aidl b/core/java/android/os/incremental/IIncrementalService.aidl
index 2dbaea860e2a..d8308c7c3362 100644
--- a/core/java/android/os/incremental/IIncrementalService.aidl
+++ b/core/java/android/os/incremental/IIncrementalService.aidl
@@ -38,6 +38,13 @@ interface IIncrementalService {
     int createLinkedStorage(in @utf8InCpp String path, int otherStorageId, int createMode);
 
     /**
+     * Changes storage params. Returns 0 on success, and -errno on failure.
+     * Use enableReadLogs to switch pages read logs reporting on and off.
+     * Returns 0 on success, and - errno on failure: permission check or remount.
+     */
+    int setStorageParams(int storageId, boolean enableReadLogs);
+
+    /**
      * Bind-mounts a path under a storage to a full path. Can be permanent or temporary.
      */
     const int BIND_TEMPORARY = 0;
diff --git a/core/java/android/os/incremental/IncrementalManager.java b/core/java/android/os/incremental/IncrementalManager.java
index 35518db32829..5f01408944e8 100644
--- a/core/java/android/os/incremental/IncrementalManager.java
+++ b/core/java/android/os/incremental/IncrementalManager.java
@@ -19,11 +19,13 @@ package android.os.incremental;
 import android.annotation.IntDef;
 import android.annotation.NonNull;
 import android.annotation.Nullable;
+import android.annotation.RequiresPermission;
 import android.annotation.SystemService;
 import android.content.Context;
 import android.content.pm.DataLoaderParams;
 import android.content.pm.IDataLoaderStatusListener;
 import android.os.RemoteException;
+import android.system.ErrnoException;
 import android.util.SparseArray;
 
 import com.android.internal.annotations.GuardedBy;
@@ -319,6 +321,23 @@ public final class IncrementalManager {
         return nativeUnsafeGetFileSignature(path);
     }
 
+    /**
+     * Sets storage parameters.
+     *
+     * @param enableReadLogs - enables or disables read logs. Caller has to have a permission.
+     */
+    @RequiresPermission(android.Manifest.permission.LOADER_USAGE_STATS)
+    public void setStorageParams(int storageId, boolean enableReadLogs) throws ErrnoException {
+        try {
+            int res = mService.setStorageParams(storageId, enableReadLogs);
+            if (res < 0) {
+                throw new ErrnoException("setStorageParams", -res);
+            }
+        } catch (RemoteException e) {
+            e.rethrowFromSystemServer();
+        }
+    }
+
     /* Native methods */
     private static native boolean nativeIsEnabled();
     private static native boolean nativeIsIncrementalPath(@NonNull String path);
diff --git a/core/java/android/service/dataloader/DataLoaderService.java b/core/java/android/service/dataloader/DataLoaderService.java
index c047dc0d07c7..05877a59368a 100644
--- a/core/java/android/service/dataloader/DataLoaderService.java
+++ b/core/java/android/service/dataloader/DataLoaderService.java
@@ -21,6 +21,7 @@ import android.annotation.Nullable;
 import android.annotation.RequiresPermission;
 import android.annotation.SystemApi;
 import android.app.Service;
+import android.content.Context;
 import android.content.Intent;
 import android.content.pm.DataLoaderParams;
 import android.content.pm.DataLoaderParamsParcel;
@@ -31,6 +32,8 @@ import android.content.pm.InstallationFile;
 import android.content.pm.InstallationFileParcel;
 import android.os.IBinder;
 import android.os.ParcelFileDescriptor;
+import android.os.incremental.IncrementalManager;
+import android.system.ErrnoException;
 import android.util.ExceptionUtils;
 import android.util.Slog;
 
@@ -208,6 +211,25 @@ public abstract class DataLoaderService extends Service {
         private final long mNativeInstance;
     }
 
+    /* Used by native FileSystemConnector. */
+    private boolean setStorageParams(int storageId, boolean enableReadLogs) {
+        IncrementalManager incrementalManager = (IncrementalManager) getSystemService(
+                Context.INCREMENTAL_SERVICE);
+        if (incrementalManager == null) {
+            Slog.e(TAG, "Failed to obtain incrementalManager: " + storageId);
+            return false;
+        }
+        try {
+            // This has to be done directly in incrementalManager as the storage
+            // might be missing still.
+            incrementalManager.setStorageParams(storageId, enableReadLogs);
+        } catch (ErrnoException e) {
+            Slog.e(TAG, "Failed to set params for storage: " + storageId, e);
+            return false;
+        }
+        return true;
+    }
+
     /* Native methods */
     private native boolean nativeCreateDataLoader(int storageId,
             @NonNull FileSystemControlParcel control,
diff --git a/services/core/jni/com_android_server_pm_PackageManagerShellCommandDataLoader.cpp b/services/core/jni/com_android_server_pm_PackageManagerShellCommandDataLoader.cpp
index e9a5e58e49d1..606c547386b8 100644
--- a/services/core/jni/com_android_server_pm_PackageManagerShellCommandDataLoader.cpp
+++ b/services/core/jni/com_android_server_pm_PackageManagerShellCommandDataLoader.cpp
@@ -392,6 +392,7 @@ private:
         mArgs = params.arguments();
         mIfs = ifs;
         mStatusListener = statusListener;
+        mIfs->setParams({.readLogsEnabled = true});
         return true;
     }
     bool onStart() final { return true; }
diff --git a/services/incremental/BinderIncrementalService.cpp b/services/incremental/BinderIncrementalService.cpp
index 2dbbc5ac6806..97de1800cae2 100644
--- a/services/incremental/BinderIncrementalService.cpp
+++ b/services/incremental/BinderIncrementalService.cpp
@@ -155,6 +155,11 @@ binder::Status BinderIncrementalService::deleteStorage(int32_t storageId) {
     return ok();
 }
 
+binder::Status BinderIncrementalService::setStorageParams(int32_t storage, bool enableReadLogs, int32_t* _aidl_return) {
+    *_aidl_return = mImpl.setStorageParams(storage, enableReadLogs);
+    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 28613e101b7c..d0357d924586 100644
--- a/services/incremental/BinderIncrementalService.h
+++ b/services/incremental/BinderIncrementalService.h
@@ -71,6 +71,7 @@ public:
     binder::Status configureNativeBinaries(int32_t storageId, const std::string& apkFullPath,
                                            const std::string& libDirRelativePath,
                                            const std::string& abi, bool* _aidl_return) final;
+    binder::Status setStorageParams(int32_t storage, bool enableReadLogs, int32_t* _aidl_return) final;
 
 private:
     android::incremental::IncrementalService mImpl;
diff --git a/services/incremental/IncrementalService.cpp b/services/incremental/IncrementalService.cpp
index 25da8fe4a2e8..90df240233f4 100644
--- a/services/incremental/IncrementalService.cpp
+++ b/services/incremental/IncrementalService.cpp
@@ -563,6 +563,36 @@ StorageId IncrementalService::findStorageId(std::string_view path) const {
     return it->second->second.storage;
 }
 
+int IncrementalService::setStorageParams(StorageId storageId, bool enableReadLogs) {
+    const auto ifs = getIfs(storageId);
+    if (!ifs) {
+        return -EINVAL;
+    }
+
+    using unique_fd = ::android::base::unique_fd;
+    ::android::os::incremental::IncrementalFileSystemControlParcel control;
+    control.cmd.reset(unique_fd(dup(ifs->control.cmd())));
+    control.pendingReads.reset(unique_fd(dup(ifs->control.pendingReads())));
+    auto logsFd = ifs->control.logs();
+    if (logsFd >= 0) {
+        control.log.reset(unique_fd(dup(logsFd)));
+    }
+
+    std::lock_guard l(mMountOperationLock);
+    const auto status = mVold->setIncFsMountOptions(control, enableReadLogs);
+    if (!status.isOk()) {
+        LOG(ERROR) << "Calling Vold::setIncFsMountOptions() failed: " << status.toString8();
+        return status.exceptionCode() == binder::Status::EX_SERVICE_SPECIFIC
+                ? status.serviceSpecificErrorCode() > 0 ? -status.serviceSpecificErrorCode()
+                                                        : status.serviceSpecificErrorCode() == 0
+                                ? -EFAULT
+                                : status.serviceSpecificErrorCode()
+                : -EIO;
+    }
+
+    return 0;
+}
+
 void IncrementalService::deleteStorage(StorageId storageId) {
     const auto ifs = getIfs(storageId);
     if (!ifs) {
@@ -737,10 +767,12 @@ int IncrementalService::makeFile(StorageId storage, std::string_view path, int m
     if (auto ifs = getIfs(storage)) {
         std::string normPath = normalizePathToStorage(ifs, storage, path);
         if (normPath.empty()) {
+            LOG(ERROR) << "Internal error: storageId " << storage << " failed to normalize: " << path;
             return -EINVAL;
         }
         auto err = mIncFs->makeFile(ifs->control, normPath, mode, id, params);
         if (err) {
+            LOG(ERROR) << "Internal error: storageId " << storage << " failed to makeFile: " << err;
             return err;
         }
         std::vector<uint8_t> metadataBytes;
diff --git a/services/incremental/IncrementalService.h b/services/incremental/IncrementalService.h
index 406b32e51044..90d58a7adcf0 100644
--- a/services/incremental/IncrementalService.h
+++ b/services/incremental/IncrementalService.h
@@ -111,6 +111,8 @@ public:
     int unbind(StorageId storage, std::string_view target);
     void deleteStorage(StorageId storage);
 
+    int setStorageParams(StorageId storage, bool enableReadLogs);
+
     int makeFile(StorageId storage, std::string_view path, int mode, FileId id,
                  incfs::NewFileParams params);
     int makeDir(StorageId storage, std::string_view path, int mode = 0755);
diff --git a/services/incremental/ServiceWrappers.h b/services/incremental/ServiceWrappers.h
index c70a47d40c4e..d8c96e83ffb4 100644
--- a/services/incremental/ServiceWrappers.h
+++ b/services/incremental/ServiceWrappers.h
@@ -49,6 +49,7 @@ public:
     virtual binder::Status unmountIncFs(const std::string& dir) const = 0;
     virtual binder::Status bindMount(const std::string& sourceDir,
                                      const std::string& targetDir) const = 0;
+    virtual binder::Status setIncFsMountOptions(const ::android::os::incremental::IncrementalFileSystemControlParcel& control, bool enableReadLogs) const = 0;
 };
 
 class DataLoaderManagerWrapper {
@@ -106,6 +107,9 @@ public:
                              const std::string& targetDir) const override {
         return mInterface->bindMount(sourceDir, targetDir);
     }
+    binder::Status setIncFsMountOptions(const ::android::os::incremental::IncrementalFileSystemControlParcel& control, bool enableReadLogs) const override {
+        return mInterface->setIncFsMountOptions(control, enableReadLogs);
+    }
 
 private:
     sp<os::IVold> mInterface;
diff --git a/services/incremental/test/IncrementalServiceTest.cpp b/services/incremental/test/IncrementalServiceTest.cpp
index c4b4d1746cbe..d2e97977b686 100644
--- a/services/incremental/test/IncrementalServiceTest.cpp
+++ b/services/incremental/test/IncrementalServiceTest.cpp
@@ -52,6 +52,8 @@ public:
     MOCK_CONST_METHOD1(unmountIncFs, binder::Status(const std::string& dir));
     MOCK_CONST_METHOD2(bindMount,
                        binder::Status(const std::string& sourceDir, const std::string& argetDir));
+    MOCK_CONST_METHOD2(setIncFsMountOptions,
+                       binder::Status(const ::android::os::incremental::IncrementalFileSystemControlParcel&, bool));
 
     void mountIncFsFails() {
         ON_CALL(*this, mountIncFs(_, _, _, _))
@@ -74,6 +76,14 @@ public:
     void bindMountSuccess() {
         ON_CALL(*this, bindMount(_, _)).WillByDefault(Return(binder::Status::ok()));
     }
+    void setIncFsMountOptionsFails() const {
+        ON_CALL(*this, setIncFsMountOptions(_, _))
+                .WillByDefault(
+                        Return(binder::Status::fromExceptionCode(1, String8("failed to set options"))));
+    }
+    void setIncFsMountOptionsSuccess() {
+        ON_CALL(*this, setIncFsMountOptions(_, _)).WillByDefault(Return(binder::Status::ok()));
+    }
     binder::Status getInvalidControlParcel(const std::string& imagePath,
                                            const std::string& targetDir, int32_t flags,
                                            IncrementalFileSystemControlParcel* _aidl_return) {
@@ -390,6 +400,42 @@ TEST_F(IncrementalServiceTest, testStartDataLoaderSuccess) {
     ASSERT_TRUE(mIncrementalService->startLoading(storageId));
 }
 
+TEST_F(IncrementalServiceTest, testSetIncFsMountOptionsSuccess) {
+    mVold->mountIncFsSuccess();
+    mIncFs->makeFileSuccess();
+    mVold->bindMountSuccess();
+    mVold->setIncFsMountOptionsSuccess();
+    mDataLoaderManager->initializeDataLoaderSuccess();
+    mDataLoaderManager->getDataLoaderSuccess();
+    EXPECT_CALL(*mDataLoaderManager, destroyDataLoader(_));
+    EXPECT_CALL(*mVold, unmountIncFs(_)).Times(2);
+    EXPECT_CALL(*mVold, setIncFsMountOptions(_, _));
+    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);
+}
+
+TEST_F(IncrementalServiceTest, testSetIncFsMountOptionsFails) {
+    mVold->mountIncFsSuccess();
+    mIncFs->makeFileSuccess();
+    mVold->bindMountSuccess();
+    mVold->setIncFsMountOptionsFails();
+    mDataLoaderManager->initializeDataLoaderSuccess();
+    mDataLoaderManager->getDataLoaderSuccess();
+    EXPECT_CALL(*mDataLoaderManager, destroyDataLoader(_));
+    EXPECT_CALL(*mVold, unmountIncFs(_)).Times(2);
+    EXPECT_CALL(*mVold, setIncFsMountOptions(_, _));
+    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, testMakeDirectory) {
     mVold->mountIncFsSuccess();
     mIncFs->makeFileSuccess();