Change the way how we call setStorageParams.
Now it's unified with callback FS connector - we are passing the
callback pointer directly to dataloader. This restricts access only
to methods we want and only by someone we want.
Bug: b/153468113
Test: atest PackageManagerShellCommandTest PackageManagerShellCommandIncrementalTest IncrementalServiceTest
Change-Id: Ib557ebbe7c6c5ce92140eb20534a3626b3ac96d3
diff --git a/Android.bp b/Android.bp
index 874d76f..d4ca706 100644
--- a/Android.bp
+++ b/Android.bp
@@ -911,6 +911,7 @@
filegroup {
name: "incremental_aidl",
srcs: [
+ "core/java/android/os/incremental/IIncrementalServiceConnector.aidl",
"core/java/android/os/incremental/IncrementalFileSystemControlParcel.aidl",
],
path: "core/java",
diff --git a/core/java/android/content/pm/FileSystemControlParcel.aidl b/core/java/android/content/pm/FileSystemControlParcel.aidl
index f00feae..92df16c 100644
--- a/core/java/android/content/pm/FileSystemControlParcel.aidl
+++ b/core/java/android/content/pm/FileSystemControlParcel.aidl
@@ -17,6 +17,7 @@
package android.content.pm;
import android.content.pm.IPackageInstallerSessionFileSystemConnector;
+import android.os.incremental.IIncrementalServiceConnector;
import android.os.incremental.IncrementalFileSystemControlParcel;
/**
@@ -26,6 +27,8 @@
parcelable FileSystemControlParcel {
// Incremental FS control descriptors.
@nullable IncrementalFileSystemControlParcel incremental;
+ // Incremental FS service.
+ @nullable IIncrementalServiceConnector service;
// Callback-based installation connector.
@nullable IPackageInstallerSessionFileSystemConnector callback;
}
diff --git a/core/java/android/os/incremental/IIncrementalService.aidl b/core/java/android/os/incremental/IIncrementalService.aidl
index d8308c7..2dbaea8 100644
--- a/core/java/android/os/incremental/IIncrementalService.aidl
+++ b/core/java/android/os/incremental/IIncrementalService.aidl
@@ -38,13 +38,6 @@
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/IIncrementalServiceConnector.aidl b/core/java/android/os/incremental/IIncrementalServiceConnector.aidl
new file mode 100644
index 0000000..5800ecf
--- /dev/null
+++ b/core/java/android/os/incremental/IIncrementalServiceConnector.aidl
@@ -0,0 +1,27 @@
+/*
+ * Copyright (C) 2019 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package android.os.incremental;
+
+/** @hide */
+interface IIncrementalServiceConnector {
+ /**
+ * 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(boolean enableReadLogs);
+}
diff --git a/core/java/android/os/incremental/IncrementalManager.java b/core/java/android/os/incremental/IncrementalManager.java
index 5f01408..35518db 100644
--- a/core/java/android/os/incremental/IncrementalManager.java
+++ b/core/java/android/os/incremental/IncrementalManager.java
@@ -19,13 +19,11 @@
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;
@@ -321,23 +319,6 @@
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 05877a5..c047dc0 100644
--- a/core/java/android/service/dataloader/DataLoaderService.java
+++ b/core/java/android/service/dataloader/DataLoaderService.java
@@ -21,7 +21,6 @@
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;
@@ -32,8 +31,6 @@
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;
@@ -211,25 +208,6 @@
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/incremental/BinderIncrementalService.cpp b/services/incremental/BinderIncrementalService.cpp
index 97de1800..2dbbc5a 100644
--- a/services/incremental/BinderIncrementalService.cpp
+++ b/services/incremental/BinderIncrementalService.cpp
@@ -155,11 +155,6 @@
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 d0357d9..28613e1 100644
--- a/services/incremental/BinderIncrementalService.h
+++ b/services/incremental/BinderIncrementalService.h
@@ -71,7 +71,6 @@
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 d36eae8..d6ce529 100644
--- a/services/incremental/IncrementalService.cpp
+++ b/services/incremental/IncrementalService.cpp
@@ -1143,6 +1143,7 @@
fsControlParcel.incremental->pendingReads.reset(
base::unique_fd(::dup(ifs.control.pendingReads())));
fsControlParcel.incremental->log.reset(base::unique_fd(::dup(ifs.control.logs())));
+ fsControlParcel.service = new IncrementalServiceConnector(*this, ifs.mountId);
sp<IncrementalDataLoaderListener> listener =
new IncrementalDataLoaderListener(*this,
externalListener ? *externalListener
@@ -1394,4 +1395,10 @@
incrementalService.onAppOpChanged(packageName);
}
+binder::Status IncrementalService::IncrementalServiceConnector::setStorageParams(
+ bool enableReadLogs, int32_t* _aidl_return) {
+ *_aidl_return = incrementalService.setStorageParams(storage, enableReadLogs);
+ return binder::Status::ok();
+}
+
} // namespace android::incremental
diff --git a/services/incremental/IncrementalService.h b/services/incremental/IncrementalService.h
index 5800297..c2a550b 100644
--- a/services/incremental/IncrementalService.h
+++ b/services/incremental/IncrementalService.h
@@ -39,6 +39,7 @@
#include "ServiceWrappers.h"
#include "android/content/pm/BnDataLoaderStatusListener.h"
+#include "android/os/incremental/BnIncrementalServiceConnector.h"
#include "incfs.h"
#include "path.h"
@@ -139,7 +140,7 @@
DataLoaderStatusListener externalListener)
: incrementalService(incrementalService), externalListener(externalListener) {}
// Callbacks interface
- binder::Status onStatusChanged(MountId mount, int newStatus) override;
+ binder::Status onStatusChanged(MountId mount, int newStatus) final;
private:
IncrementalService& incrementalService;
@@ -149,13 +150,24 @@
class AppOpsListener : public android::BnAppOpsCallback {
public:
AppOpsListener(IncrementalService& incrementalService, std::string packageName) : incrementalService(incrementalService), packageName(std::move(packageName)) {}
- void opChanged(int32_t op, const String16& packageName) override;
+ void opChanged(int32_t op, const String16& packageName) final;
private:
IncrementalService& incrementalService;
const std::string packageName;
};
+ class IncrementalServiceConnector : public BnIncrementalServiceConnector {
+ public:
+ IncrementalServiceConnector(IncrementalService& incrementalService, int32_t storage)
+ : incrementalService(incrementalService) {}
+ binder::Status setStorageParams(bool enableReadLogs, int32_t* _aidl_return) final;
+
+ private:
+ IncrementalService& incrementalService;
+ int32_t storage;
+ };
+
private:
struct IncFsMount {
struct Bind {