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
diff --git a/core/java/android/content/pm/PackageParser.java b/core/java/android/content/pm/PackageParser.java
index c8dd4d9..885ffba 100644
--- a/core/java/android/content/pm/PackageParser.java
+++ b/core/java/android/content/pm/PackageParser.java
@@ -205,6 +205,7 @@
public static final String TAG_USES_PERMISSION_SDK_M = "uses-permission-sdk-m";
public static final String TAG_USES_SDK = "uses-sdk";
public static final String TAG_USES_SPLIT = "uses-split";
+ public static final String TAG_PROFILEABLE = "profileable";
public static final String METADATA_MAX_ASPECT_RATIO = "android.max_aspect";
public static final String METADATA_SUPPORTS_SIZE_CHANGES = "android.supports_size_changes";
@@ -459,6 +460,9 @@
public final SigningDetails signingDetails;
public final boolean coreApp;
public final boolean debuggable;
+ // This does not represent the actual manifest structure since the 'profilable' tag
+ // could be used with attributes other than 'shell'. Extend if necessary.
+ public final boolean profilableByShell;
public final boolean multiArch;
public final boolean use32bitAbi;
public final boolean extractNativeLibs;
@@ -470,15 +474,13 @@
public final int overlayPriority;
public ApkLite(String codePath, String packageName, String splitName,
- boolean isFeatureSplit,
- String configForSplit, String usesSplitName, boolean isSplitRequired,
- int versionCode, int versionCodeMajor,
- int revisionCode, int installLocation, List<VerifierInfo> verifiers,
- SigningDetails signingDetails, boolean coreApp,
- boolean debuggable, boolean multiArch, boolean use32bitAbi,
- boolean useEmbeddedDex, boolean extractNativeLibs, boolean isolatedSplits,
- String targetPackageName, boolean overlayIsStatic, int overlayPriority,
- int minSdkVersion, int targetSdkVersion) {
+ boolean isFeatureSplit, String configForSplit, String usesSplitName,
+ boolean isSplitRequired, int versionCode, int versionCodeMajor, int revisionCode,
+ int installLocation, List<VerifierInfo> verifiers, SigningDetails signingDetails,
+ boolean coreApp, boolean debuggable, boolean profilableByShell, boolean multiArch,
+ boolean use32bitAbi, boolean useEmbeddedDex, boolean extractNativeLibs,
+ boolean isolatedSplits, String targetPackageName, boolean overlayIsStatic,
+ int overlayPriority, int minSdkVersion, int targetSdkVersion) {
this.codePath = codePath;
this.packageName = packageName;
this.splitName = splitName;
@@ -493,6 +495,7 @@
this.verifiers = verifiers.toArray(new VerifierInfo[verifiers.size()]);
this.coreApp = coreApp;
this.debuggable = debuggable;
+ this.profilableByShell = profilableByShell;
this.multiArch = multiArch;
this.use32bitAbi = use32bitAbi;
this.useEmbeddedDex = useEmbeddedDex;
@@ -1573,6 +1576,7 @@
int revisionCode = 0;
boolean coreApp = false;
boolean debuggable = false;
+ boolean profilableByShell = false;
boolean multiArch = false;
boolean use32bitAbi = false;
boolean extractNativeLibs = true;
@@ -1638,6 +1642,10 @@
final String attr = attrs.getAttributeName(i);
if ("debuggable".equals(attr)) {
debuggable = attrs.getAttributeBooleanValue(i, false);
+ if (debuggable) {
+ // Debuggable implies profileable
+ profilableByShell = true;
+ }
}
if ("multiArch".equals(attr)) {
multiArch = attrs.getAttributeBooleanValue(i, false);
@@ -1690,6 +1698,13 @@
minSdkVersion = attrs.getAttributeIntValue(i, DEFAULT_MIN_SDK_VERSION);
}
}
+ } else if (TAG_PROFILEABLE.equals(parser.getName())) {
+ for (int i = 0; i < attrs.getAttributeCount(); ++i) {
+ final String attr = attrs.getAttributeName(i);
+ if ("shell".equals(attr)) {
+ profilableByShell = attrs.getAttributeBooleanValue(i, profilableByShell);
+ }
+ }
}
}
@@ -1707,8 +1722,9 @@
return new ApkLite(codePath, packageSplit.first, packageSplit.second, isFeatureSplit,
configForSplit, usesSplitName, isSplitRequired, versionCode, versionCodeMajor,
revisionCode, installLocation, verifiers, signingDetails, coreApp, debuggable,
- multiArch, use32bitAbi, useEmbeddedDex, extractNativeLibs, isolatedSplits,
- targetPackage, overlayIsStatic, overlayPriority, minSdkVersion, targetSdkVersion);
+ profilableByShell, multiArch, use32bitAbi, useEmbeddedDex, extractNativeLibs,
+ isolatedSplits, targetPackage, overlayIsStatic, overlayPriority, minSdkVersion,
+ targetSdkVersion);
}
/**
diff --git a/core/java/android/content/pm/parsing/ApkLiteParseUtils.java b/core/java/android/content/pm/parsing/ApkLiteParseUtils.java
index d2172d3..c3e9402 100644
--- a/core/java/android/content/pm/parsing/ApkLiteParseUtils.java
+++ b/core/java/android/content/pm/parsing/ApkLiteParseUtils.java
@@ -20,7 +20,6 @@
import static android.content.pm.PackageManager.INSTALL_PARSE_FAILED_MANIFEST_MALFORMED;
import static android.os.Trace.TRACE_TAG_PACKAGE_MANAGER;
-import android.compat.annotation.UnsupportedAppUsage;
import android.content.pm.PackageInfo;
import android.content.pm.PackageManager;
import android.content.pm.PackageParser;
@@ -303,6 +302,7 @@
int revisionCode = 0;
boolean coreApp = false;
boolean debuggable = false;
+ boolean profilableByShell = false;
boolean multiArch = false;
boolean use32bitAbi = false;
boolean extractNativeLibs = true;
@@ -379,6 +379,10 @@
switch (attr) {
case "debuggable":
debuggable = attrs.getAttributeBooleanValue(i, false);
+ if (debuggable) {
+ // Debuggable implies profileable
+ profilableByShell = true;
+ }
break;
case "multiArch":
multiArch = attrs.getAttributeBooleanValue(i, false);
@@ -431,6 +435,13 @@
minSdkVersion = attrs.getAttributeIntValue(i, DEFAULT_MIN_SDK_VERSION);
}
}
+ } else if (PackageParser.TAG_PROFILEABLE.equals(parser.getName())) {
+ for (int i = 0; i < attrs.getAttributeCount(); ++i) {
+ final String attr = attrs.getAttributeName(i);
+ if ("shell".equals(attr)) {
+ profilableByShell = attrs.getAttributeBooleanValue(i, profilableByShell);
+ }
+ }
}
}
@@ -445,12 +456,13 @@
overlayPriority = 0;
}
- return input.success(new PackageParser.ApkLite(codePath, packageSplit.first,
- packageSplit.second, isFeatureSplit, configForSplit, usesSplitName, isSplitRequired,
- versionCode, versionCodeMajor, revisionCode, installLocation, verifiers,
- signingDetails, coreApp, debuggable, multiArch, use32bitAbi, useEmbeddedDex,
- extractNativeLibs, isolatedSplits, targetPackage, overlayIsStatic, overlayPriority,
- minSdkVersion, targetSdkVersion));
+ return input.success(
+ new PackageParser.ApkLite(codePath, packageSplit.first, packageSplit.second,
+ isFeatureSplit, configForSplit, usesSplitName, isSplitRequired, versionCode,
+ versionCodeMajor, revisionCode, installLocation, verifiers, signingDetails,
+ coreApp, debuggable, profilableByShell, multiArch, use32bitAbi,
+ useEmbeddedDex, extractNativeLibs, isolatedSplits, targetPackage,
+ overlayIsStatic, overlayPriority, minSdkVersion, targetSdkVersion));
}
public static ParseResult<Pair<String, String>> parsePackageSplitNames(ParseInput input,
diff --git a/core/java/android/os/incremental/IIncrementalService.aidl b/core/java/android/os/incremental/IIncrementalService.aidl
index 220ce22..61e6a05 100644
--- a/core/java/android/os/incremental/IIncrementalService.aidl
+++ b/core/java/android/os/incremental/IIncrementalService.aidl
@@ -111,6 +111,11 @@
void deleteStorage(int storageId);
/**
+ * Permanently disable readlogs reporting for a storage given its ID.
+ */
+ void disableReadLogs(int storageId);
+
+ /**
* Setting up native library directories and extract native libs onto a storage if needed.
*/
boolean configureNativeBinaries(int storageId, in @utf8InCpp String apkFullPath, in @utf8InCpp String libDirRelativePath, in @utf8InCpp String abi, boolean extractNativeLibs);
diff --git a/core/java/android/os/incremental/IncrementalFileStorages.java b/core/java/android/os/incremental/IncrementalFileStorages.java
index 863d86ef..31ccf95 100644
--- a/core/java/android/os/incremental/IncrementalFileStorages.java
+++ b/core/java/android/os/incremental/IncrementalFileStorages.java
@@ -153,6 +153,13 @@
}
/**
+ * Permanently disables readlogs.
+ */
+ public void disableReadLogs() {
+ mDefaultStorage.disableReadLogs();
+ }
+
+ /**
* Resets the states and unbinds storage instances for an installation session.
* TODO(b/136132412): make sure unnecessary binds are removed but useful storages are kept
*/
diff --git a/core/java/android/os/incremental/IncrementalStorage.java b/core/java/android/os/incremental/IncrementalStorage.java
index 6200a38..ca6114f 100644
--- a/core/java/android/os/incremental/IncrementalStorage.java
+++ b/core/java/android/os/incremental/IncrementalStorage.java
@@ -418,6 +418,17 @@
private static final int INCFS_MAX_ADD_DATA_SIZE = 128;
/**
+ * Permanently disable readlogs collection.
+ */
+ public void disableReadLogs() {
+ try {
+ mService.disableReadLogs(mId);
+ } catch (RemoteException e) {
+ e.rethrowFromSystemServer();
+ }
+ }
+
+ /**
* Deserialize and validate v4 signature bytes.
*/
private static void validateV4Signature(@Nullable byte[] v4signatureBytes)
diff --git a/services/core/java/com/android/server/pm/PackageInstallerSession.java b/services/core/java/com/android/server/pm/PackageInstallerSession.java
index 7ab05c4..de8ad6b 100644
--- a/services/core/java/com/android/server/pm/PackageInstallerSession.java
+++ b/services/core/java/com/android/server/pm/PackageInstallerSession.java
@@ -2108,6 +2108,7 @@
baseApk = apk;
}
+ // Validate and add Dex Metadata (.dm).
final File dexMetadataFile = DexMetadataHelper.findDexMetadataForFile(addedFile);
if (dexMetadataFile != null) {
if (!FileUtils.isValidExtFilename(dexMetadataFile.getName())) {
@@ -2295,6 +2296,13 @@
throw new PackageManagerException(INSTALL_FAILED_MISSING_SPLIT,
"Missing split for " + mPackageName);
}
+
+ final boolean isInstallerShell = (mInstallerUid == Process.SHELL_UID);
+ if (isInstallerShell && isIncrementalInstallation() && mIncrementalFileStorages != null) {
+ if (!baseApk.debuggable && !baseApk.profilableByShell) {
+ mIncrementalFileStorages.disableReadLogs();
+ }
+ }
}
private void resolveAndStageFile(File origFile, File targetFile)
diff --git a/services/incremental/BinderIncrementalService.cpp b/services/incremental/BinderIncrementalService.cpp
index e790a19..7132706 100644
--- a/services/incremental/BinderIncrementalService.cpp
+++ b/services/incremental/BinderIncrementalService.cpp
@@ -165,6 +165,11 @@
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 68549f5..1015494 100644
--- a/services/incremental/BinderIncrementalService.h
+++ b/services/incremental/BinderIncrementalService.h
@@ -74,7 +74,7 @@
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 885f4d2..3450c3a 100644
--- a/services/incremental/IncrementalService.cpp
+++ b/services/incremental/IncrementalService.cpp
@@ -60,6 +60,7 @@
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 @@
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 @@
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 @@
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::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 @@
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 918531b..a6cc946 100644
--- a/services/incremental/IncrementalService.h
+++ b/services/incremental/IncrementalService.h
@@ -94,6 +94,10 @@
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 @@
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 @@
const std::string root;
Control control;
/*const*/ MountId mountId;
+ int32_t flags = StorageFlags::ReadLogsEnabled;
StorageMap storages;
BindMap bindPoints;
DataLoaderStubPtr dataLoaderStub;
@@ -282,6 +288,9 @@
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 26b5094..1ae9e25 100644
--- a/services/incremental/test/IncrementalServiceTest.cpp
+++ b/services/incremental/test/IncrementalServiceTest.cpp
@@ -929,6 +929,34 @@
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();