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/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();