From 0699f2a0f7ba8ab3c9bba58d254ad05f2b57f896 Mon Sep 17 00:00:00 2001 From: Yurii Zubrytskyi Date: Wed, 11 May 2022 11:52:58 -0700 Subject: [PATCH] SELinux labels bug logging and workaround Some apps don't get their dir/file labels correctly updated after installation, and fail to launch because of it This CL adds additional logging for the applying, and introduces some extra verifications during initial app scanning at boot, and right at the final steps of the installation Should be reverted after the root cause gets fixed Bug: 231951809 Test: manual device boot Change-Id: Ifb68ba1203726db24a7791b284e31fc74df25e96 --- .../android/server/pm/FileInstallArgs.java | 19 +++++++++++--- .../server/pm/InstallPackageHelper.java | 5 ++++ .../server/pm/PackageManagerServiceUtils.java | 25 +++++++++++++++++++ 3 files changed, 46 insertions(+), 3 deletions(-) diff --git a/services/core/java/com/android/server/pm/FileInstallArgs.java b/services/core/java/com/android/server/pm/FileInstallArgs.java index 85c3cc91ecf06..e3ceccd1abb86 100644 --- a/services/core/java/com/android/server/pm/FileInstallArgs.java +++ b/services/core/java/com/android/server/pm/FileInstallArgs.java @@ -172,9 +172,22 @@ class FileInstallArgs extends InstallArgs { return false; } - if (!onIncremental && !SELinux.restoreconRecursive(afterCodeFile)) { - Slog.w(TAG, "Failed to restorecon"); - return false; + if (onIncremental) { + Slog.i(TAG, PackageManagerServiceUtils.SELINUX_BUG + + ": Skipping restorecon for Incremental install of " + beforeCodeFile); + } else { + try { + if (!SELinux.restoreconRecursive(afterCodeFile)) { + Slog.w(TAG, "Failed to restorecon"); + return false; + } + PackageManagerServiceUtils.verifySelinuxLabels(afterCodeFile.getAbsolutePath()); + } catch (Exception e) { + Slog.e(TAG, + PackageManagerServiceUtils.SELINUX_BUG + ": Exception from restorecon on " + + beforeCodeFile, e); + throw e; + } } // Reflect the rename internally diff --git a/services/core/java/com/android/server/pm/InstallPackageHelper.java b/services/core/java/com/android/server/pm/InstallPackageHelper.java index 57a1fe04b6901..f909feed80453 100644 --- a/services/core/java/com/android/server/pm/InstallPackageHelper.java +++ b/services/core/java/com/android/server/pm/InstallPackageHelper.java @@ -648,6 +648,10 @@ final class InstallPackageHelper { Log.v(TAG, "restoreAndPostInstall userId=" + userId + " package=" + res.mPkg); } + if (res.mPkg != null) { + PackageManagerServiceUtils.verifySelinuxLabels(res.mPkg.getPath()); + } + // A restore should be requested at this point if (a) the install // succeeded, (b) the operation is not an update. final boolean update = res.mRemovedInfo != null @@ -3566,6 +3570,7 @@ final class InstallPackageHelper { @ParsingPackageUtils.ParseFlags int parseFlags, @PackageManagerService.ScanFlags int scanFlags, @Nullable UserHandle user) throws PackageManagerException { + PackageManagerServiceUtils.verifySelinuxLabels(parsedPackage.getPath()); final Pair scanResultPair = scanSystemPackageLI( parsedPackage, parseFlags, scanFlags, user); diff --git a/services/core/java/com/android/server/pm/PackageManagerServiceUtils.java b/services/core/java/com/android/server/pm/PackageManagerServiceUtils.java index 4d11b13510e99..d0aa6c2b87264 100644 --- a/services/core/java/com/android/server/pm/PackageManagerServiceUtils.java +++ b/services/core/java/com/android/server/pm/PackageManagerServiceUtils.java @@ -60,6 +60,7 @@ import android.os.Debug; import android.os.Environment; import android.os.FileUtils; import android.os.Process; +import android.os.SELinux; import android.os.SystemProperties; import android.os.incremental.IncrementalManager; import android.os.incremental.IncrementalStorage; @@ -1388,4 +1389,28 @@ public class PackageManagerServiceUtils { } } } + + // TODO(b/231951809): remove this workaround after figuring out why apk_tmp_file labels stay + // on the installed apps instead of the correct apk_data_file ones + + public static final String SELINUX_BUG = "b/231951809"; + + /** + * A workaround for b/231951809: + * Verifies the SELinux labels of the passed path, and tries to correct them if detects them + * wrong or missing. + */ + public static void verifySelinuxLabels(String path) { + final String expectedCon = SELinux.fileSelabelLookup(path); + final String actualCon = SELinux.getFileContext(path); + Slog.i(TAG, SELINUX_BUG + ": checking selinux labels for " + path + " expected / actual: " + + expectedCon + " / " + actualCon); + if (expectedCon == null || !expectedCon.equals(actualCon)) { + Slog.w(TAG, SELINUX_BUG + ": labels don't match, reapplying for " + path); + if (!SELinux.restoreconRecursive(new File(path))) { + Slog.w(TAG, SELINUX_BUG + ": Failed to reapply restorecon"); + } + // well, if it didn't work now after not working at first, not much else can be done + } + } }